diff --git a/packages/cli-command/README.md b/packages/cli-command/README.md index f8a6ecd3a..28adda567 100644 --- a/packages/cli-command/README.md +++ b/packages/cli-command/README.md @@ -171,9 +171,6 @@ option, if the environment variable `PERCY_ENABLE` is `0`, the callback _will no instance (and can act accordingly). - `percy` — Enables creation of a `@percy/core` instance initialized with provided options. - - `discoveryFlags` — When `false` prevents percy discovery flags from being accepted, but will - still allow other percy flags such as `--config` and `--dry-run`. - - `...` — All other options are passed along to the new percy instance. #### Config diff --git a/packages/cli-command/src/command.js b/packages/cli-command/src/command.js index 59d22ffff..1d9c0f9f5 100644 --- a/packages/cli-command/src/command.js +++ b/packages/cli-command/src/command.js @@ -25,7 +25,7 @@ function withBuiltIns(definition) { builtInFlags.PERCY.forEach(addDedupedFlag); // maybe include percy discovery flags - if (def.percy.discoveryFlags !== false) { + if (def.percy.skipDiscovery !== true) { builtInFlags.DISCOVERY.forEach(addDedupedFlag); } } @@ -67,8 +67,8 @@ async function runCommandWithContext(parsed) { if (def.percy) { let { Percy } = await import('@percy/core'); - // set defaults and prune preconfiguraton options - let conf = del({ server: false, ...def.percy }, 'discoveryFlags'); + // shallow merge with default options + let conf = { server: false, ...def.percy }; if (pkg) conf.clientInfo ||= `${pkg.name}/${pkg.version}`; conf.environmentInfo ||= `node/${process.version}`; diff --git a/packages/cli-command/test/flags.test.js b/packages/cli-command/test/flags.test.js index c52233256..4e72796a0 100644 --- a/packages/cli-command/test/flags.test.js +++ b/packages/cli-command/test/flags.test.js @@ -88,7 +88,7 @@ describe('Built-in flags:', () => { }); it('does not show discovery flags when excluded', async () => { - await command('foo', { percy: { discoveryFlags: false } })(['--help']); + await command('foo', { percy: { skipDiscovery: true } })(['--help']); expect(logger.stdout).toEqual([expectedMinPercyFlags]); expect(logger.stdout).not.toEqual([expectedAllPercyFlags]); }); diff --git a/packages/cli-upload/src/upload.js b/packages/cli-upload/src/upload.js index baed669d0..2437d0a28 100644 --- a/packages/cli-upload/src/upload.js +++ b/packages/cli-upload/src/upload.js @@ -48,8 +48,8 @@ export const upload = command('upload', { ], percy: { - discoveryFlags: false, - deferUploads: true + deferUploads: true, + skipDiscovery: true }, config: { @@ -76,9 +76,7 @@ export const upload = command('upload', { // the internal upload queue shares a concurrency with the snapshot queue percy.setConfig({ discovery: { concurrency: config.concurrency } }); - - // do not launch a browser when starting - yield* percy.yield.start({ browser: false }); + yield* percy.yield.start(); for (let filename of pathnames) { let file = path.parse(filename); diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index b84ff105e..08f93be88 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -45,7 +45,9 @@ export class Percy { deferUploads, // run without uploading anything skipUploads, - // implies `skipUploads` and also skips asset discovery + // run without asset discovery + skipDiscovery, + // implies `skipUploads` and `skipDiscovery` dryRun, // implies `dryRun`, silent logs, and adds extra api endpoints testing, @@ -68,6 +70,7 @@ export class Percy { this.testing = testing ? {} : null; this.dryRun = !!testing || !!dryRun; this.skipUploads = this.dryRun || !!skipUploads; + this.skipDiscovery = this.dryRun || !!skipDiscovery; this.delayUploads = this.skipUploads || !!delayUploads; this.deferUploads = this.delayUploads || !!deferUploads; if (this.deferUploads) this.#uploads.stop(); @@ -152,7 +155,7 @@ export class Percy { // Starts a local API server, a browser process, and queues creating a new Percy build which will run // at a later time when uploads are deferred, or run immediately when not deferred. - async *start(options) { + async *start() { // already starting or started if (this.readyState != null) return; this.readyState = 0; @@ -187,9 +190,7 @@ export class Percy { if (!this.deferUploads) await buildTask; // maybe launch the discovery browser - if (!this.dryRun && options?.browser !== false) { - yield this.browser.launch(); - } + if (!this.skipDiscovery) yield this.browser.launch(); // start the server after everything else is ready yield this.server?.listen(); @@ -231,8 +232,8 @@ export class Percy { if (close) this.#snapshots.close(); yield* this.#snapshots.flush(s => { - // do not log a count when not closing or while dry-running - if (!close || this.dryRun) return; + // do not log a count when not closing or if asset discovery is disabled + if (!close || this.skipDiscovery) return; this.log.progress(`Processing ${s} snapshot${s !== 1 ? 's' : ''}...`, !!s); }); } diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index 5c9c4bd3d..b9b6ba14a 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -376,6 +376,13 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) { .map(resource => [resource.url, resource]) )); + // when no discovery browser is available, do not attempt to discover other resources + if (percy.skipDiscovery && !snapshot.domSnapshot) { + throw new Error('Cannot capture DOM snapshot when asset discovery is disabled'); + } else if (percy.skipDiscovery) { + return handleSnapshotResources(snapshot, resources, callback); + } + // open a new browser page let page = yield percy.browser.page({ enableJavaScript: snapshot.enableJavaScript ?? !snapshot.domSnapshot, diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 08b07c333..0843ec643 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -716,6 +716,45 @@ describe('Discovery', () => { ])); }); + it('can skip asset discovery', async () => { + // stop current instance to create a new one + await percy.stop(); + + percy = await Percy.start({ + token: 'PERCY_TOKEN', + skipDiscovery: true, + clientInfo: 'testing', + environmentInfo: 'testing' + }); + + logger.reset(); + await percy.snapshot([{ + name: 'Snapshot with DOM', + url: 'http://localhost:8000', + domSnapshot: testDOM + }, { + name: 'Snapshot without DOM', + url: 'http://localhost:8000' + }]); + + await percy.idle(); + expect(server.requests).toEqual([]); + + expect(captured).toHaveSize(1); + expect(captured[0].map(r => r.attributes['resource-url'])).toEqual([ + jasmine.stringMatching(/\/percy\.\d+\.log$/), + 'http://localhost:8000/' + ]); + + expect(logger.stdout).toEqual([ + '[percy] Snapshot taken: Snapshot with DOM' + ]); + expect(logger.stderr).toEqual([ + '[percy] Encountered an error taking snapshot: Snapshot without DOM', + '[percy] Error: Cannot capture DOM snapshot when asset discovery is disabled' + ]); + }); + describe('idle timeout', () => { let Network, timeout;