Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add --debug discovery flag #492

Merged
merged 4 commits into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/cli-command/src/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export default class PercyCommand extends Command {

// set config: false to prevent core from reloading config
return Object.assign(config, {
skipUploads: this.flags.debug,
config: false
});
}
Expand Down
9 changes: 6 additions & 3 deletions packages/cli-command/src/flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,25 @@ const logging = {
})
};

// Common asset discovery flags mapped to config options.
// Common asset discovery flags.
const discovery = {
'allowed-hostname': flags.string({
char: 'h',
description: 'allowed hostnames',
description: 'allowed hostnames to capture in asset discovery',
multiple: true,
percyrc: 'discovery.allowedHostnames'
}),
'network-idle-timeout': flags.integer({
char: 't',
description: 'asset discovery idle timeout',
description: 'asset discovery network idle timeout',
percyrc: 'discovery.networkIdleTimeout'
}),
'disable-cache': flags.boolean({
description: 'disable asset discovery caches',
percyrc: 'discovery.disableCache'
}),
debug: flags.boolean({
description: 'debug asset discovery and do not upload snapshots'
})
};

Expand Down
4 changes: 4 additions & 0 deletions packages/cli-command/test/command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ describe('PercyCommand', () => {
expect(logger.loglevel()).toBe('warn');
await TestPercyCommand.run(['--silent']);
expect(logger.loglevel()).toBe('silent');
await TestPercyCommand.run(['--debug']);
expect(logger.loglevel()).toBe('debug');
});

it('logs errors to the logger and exits', async () => {
Expand Down Expand Up @@ -127,12 +129,14 @@ describe('PercyCommand', () => {
'--allowed-hostname', '*.percy.io',
'--network-idle-timeout', '150',
'--disable-cache',
'--debug',
'foo', 'bar'
])).toBeResolved();

expect(results[0].percyrc()).toEqual({
version: 2,
config: false,
skipUploads: true,
snapshot: {
widths: [375, 1280],
minHeight: 1024,
Expand Down
10 changes: 6 additions & 4 deletions packages/cli-exec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ USAGE
OPTIONS
-P, --port=port [default: 5338] server port
-c, --config=config configuration file path
-h, --allowed-hostname=allowed-hostname allowed hostnames
-h, --allowed-hostname=allowed-hostname allowed hostnames to capture in asset discovery
-q, --quiet log errors only
-t, --network-idle-timeout=network-idle-timeout asset discovery idle timeout
-t, --network-idle-timeout=network-idle-timeout asset discovery network idle timeout
-v, --verbose log everything
--debug debug asset discovery and do not upload snapshots
--disable-cache disable asset discovery caches
--parallel marks the build as one of many parallel builds
--partial marks the build as a partial build
Expand Down Expand Up @@ -60,10 +61,11 @@ USAGE
OPTIONS
-P, --port=port [default: 5338] server port
-c, --config=config configuration file path
-h, --allowed-hostname=allowed-hostname allowed hostnames
-h, --allowed-hostname=allowed-hostname allowed hostnames to capture in asset discovery
-q, --quiet log errors only
-t, --network-idle-timeout=network-idle-timeout asset discovery idle timeout
-t, --network-idle-timeout=network-idle-timeout asset discovery network idle timeout
-v, --verbose log everything
--debug debug asset discovery and do not upload snapshots
--disable-cache disable asset discovery caches
--silent log nothing

Expand Down
5 changes: 3 additions & 2 deletions packages/cli-snapshot/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ OPTIONS
-b, --base-url=base-url the base url pages are hosted at when snapshotting
-c, --config=config configuration file path
-d, --dry-run prints a list of pages to snapshot without snapshotting
-h, --allowed-hostname=allowed-hostname allowed hostnames
-h, --allowed-hostname=allowed-hostname allowed hostnames to capture in asset discovery
-q, --quiet log errors only
-t, --network-idle-timeout=network-idle-timeout asset discovery idle timeout
-t, --network-idle-timeout=network-idle-timeout asset discovery network idle timeout
-v, --verbose log everything
--clean-urls rewrite static index and filepath URLs to be clean
--debug debug asset discovery and do not upload snapshots
--disable-cache disable asset discovery caches
--files=files [default: **/*.{html,htm}] one or more globs matching static file
Expand Down
1 change: 1 addition & 0 deletions packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const percy = await Percy.start(percyOptions)
- `clientInfo` — Client info sent to Percy via a user-agent string
- `environmentInfo` — Environment info also sent with the user-agent string
- `deferUploads` — Defer creating a build and uploading snapshots until later
- `skipUploads` — Skip creating a build and uploading snapshots altogether

The following options can also be defined within a Percy config file

Expand Down
65 changes: 32 additions & 33 deletions packages/core/src/percy.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ export default class Percy {
constructor({
// initial log level
loglevel,
// upload snapshots eagerly by default
deferUploads = false,
// do not eagerly upload snapshots
deferUploads,
// run without uploading anything
skipUploads,
// configuration filepath
config,
// provided to @percy/client
Expand All @@ -52,7 +54,8 @@ export default class Percy {
...options
} = {}) {
if (loglevel) this.loglevel(loglevel);
this.deferUploads = deferUploads;
this.deferUploads = skipUploads || deferUploads;
this.skipUploads = skipUploads;

this.config = PercyConfig.load({
overrides: options,
Expand Down Expand Up @@ -99,7 +102,7 @@ export default class Percy {
// Waits for snapshot idle and flushes the upload queue
async dispatch() {
await this.#snapshots.idle();
await this.#uploads.flush();
if (!this.skipUploads) await this.#uploads.flush();
}

// Immediately stops all queues, preventing any more tasks from running
Expand All @@ -111,40 +114,36 @@ export default 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() {
// throws when the token is missing
this.client.getToken();

// already starting or started
if (this.readyState != null) return;
this.readyState = 0;

try {
// create a percy build as the first immediately queued task
let buildTask = this.#uploads.push('build/create', () => {
// pause other queued tasks until after the build is created
this.#uploads.stop();

return this.client.createBuild()
.then(({ data: { id, attributes } }) => {
this.build = { id };
this.build.number = attributes['build-number'];
this.build.url = attributes['web-url'];
this.#uploads.run();
});
}, 0);

if (!this.deferUploads) {
// when not deferred, wait until the build is created
await buildTask;
} else {
// handle deferred build errors
buildTask.catch(err => {
this.log.error('Failed to create build');
this.log.error(err);
this.close();
// create a percy build as the first immediately queued task
let buildTask = this.#uploads.push('build/create', () => {
// pause other queued tasks until after the build is created
this.#uploads.stop();

return this.client.createBuild()
.then(({ data: { id, attributes } }) => {
this.build = { id };
this.build.number = attributes['build-number'];
this.build.url = attributes['web-url'];
this.#uploads.run();
});
}
}, 0);

// handle deferred build errors
if (this.deferUploads) {
buildTask.catch(err => {
this.log.error('Failed to create build');
this.log.error(err);
this.close();
});
}

try {
// when not deferred, wait until the build is created first
if (!this.deferUploads) await buildTask;
// launch the discovery browser
await this.browser.launch(this.config.discovery.launchOptions);
// if there is a server, start listening
Expand Down Expand Up @@ -194,7 +193,7 @@ export default class Percy {
}

// run, close, and wait for the upload queue to empty
if (this.#uploads.run().close().length) {
if (!this.skipUploads && this.#uploads.run().close().length) {
await this.#uploads.empty(len => {
this.log.progress(`Uploading ${len}` + (
` snapshot${len !== 1 ? 's' : ''}...`), !!len);
Expand Down
20 changes: 20 additions & 0 deletions packages/core/test/percy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,26 @@ describe('Percy', () => {
expect(mockAPI.requests['/builds']).toBeDefined();
});

it('does not create a build when uploads are skipped', async () => {
percy = new Percy({ token: 'PERCY_TOKEN', skipUploads: true });
await expectAsync(percy.start()).toBeResolved();
expect(mockAPI.requests['/builds']).toBeUndefined();

// attempt to dispatch differed uploads
await percy.dispatch();

expect(mockAPI.requests['/builds']).toBeUndefined();

// stopping should also skip uploads
await percy.stop();

expect(mockAPI.requests['/builds']).toBeUndefined();

expect(logger.stderr).toEqual([
'[percy] Build not created'
]);
});

it('stops accepting snapshots when a queued build fails to be created', async () => {
server = await createTestServer({
default: () => [200, 'text/html', '<p>Snapshot</p>']
Expand Down
3 changes: 2 additions & 1 deletion packages/logger/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ Object.assign(logger, {
connect: (...args) => new Logger().connect(...args),
remote: (...args) => new Logger().remote(...args),
loglevel(level, flags = {}) {
if (flags.verbose) level = 'debug';
if (flags.debug) level = 'debug';
else if (flags.verbose) level = 'debug';
Comment on lines +17 to +18
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept these as separate checks in case we ever decide that one of these should produce even more logs

else if (flags.quiet) level = 'warn';
else if (flags.silent) level = 'silent';
return new Logger().loglevel(level);
Expand Down
4 changes: 3 additions & 1 deletion packages/logger/test/logger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ describe('logger', () => {
});

it('can be controlled by a secondary flags argument', () => {
logger.loglevel('info', { debug: true });
expect(logger.loglevel()).toEqual('debug');
logger.loglevel('info', { verbose: true });
expect(logger.loglevel()).toEqual('debug');
logger.loglevel('info', { quiet: true });
Expand Down Expand Up @@ -267,7 +269,7 @@ describe('logger', () => {
expect(helpers.stderr).toEqual([
jasmine.stringMatching('Warn log \\(\\dms\\)'),
jasmine.stringMatching('Error log \\(\\dms\\)'),
jasmine.stringMatching('Debug log \\((9[0-9]|1[01][0-9])ms\\)'),
jasmine.stringMatching('Debug log \\(\\d{2,3}ms\\)'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Flake

jasmine.stringMatching('Final log \\(\\dms\\)')
]);
});
Expand Down