From f7f73d108554929b24b392b1be3136633147a723 Mon Sep 17 00:00:00 2001 From: Wil Wilsman Date: Mon, 21 Jun 2021 16:11:55 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Allow=20snapshotting=20a=20list=20o?= =?UTF-8?q?f=20pages=20to=20utilize=20the=20base-url=20flag=20(#382)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ Allow base command initial overrides argument with percyrc method * ✨ Allow base-url to work with a list of page pathnames Also allow a list of page URLs rather than requiring objects containing URLs * ✨ Allow custom schema validation error messages * ✨ Add pattern validation to static.base-url config option * 🐛 Fix snapshot name default uri path * 🚨 Fix lint issues --- packages/cli-command/src/command.js | 4 +- .../cli-snapshot/src/commands/snapshot.js | 140 +++++++++++------- packages/cli-snapshot/src/config.js | 6 +- packages/cli-snapshot/test/snapshot.test.js | 113 ++++++++++---- packages/config/src/validate.js | 8 +- packages/config/test/index.test.js | 29 +++- packages/core/src/config.js | 4 +- 7 files changed, 209 insertions(+), 95 deletions(-) diff --git a/packages/cli-command/src/command.js b/packages/cli-command/src/command.js index 8c7f9cb4d..644e2eb8f 100644 --- a/packages/cli-command/src/command.js +++ b/packages/cli-command/src/command.js @@ -48,7 +48,7 @@ export default class PercyCommand extends Command { // respective `percyrc` parameter. The flag input is then merged with options // loaded from a config file and default config options. The PERCY_TOKEN // environment variable is also included as a convenience. - percyrc() { + percyrc(initialOverrides = {}) { let flags = Object.entries(this.constructor.flags); let overrides = flags.reduce((conf, [name, flag]) => ( flag.percyrc?.split('.').reduce((target, key, i, paths) => { @@ -56,7 +56,7 @@ export default class PercyCommand extends Command { target[key] = last ? this.flags[name] : (target[key] ?? {}); return last ? conf : target[key]; }, conf) ?? conf - ), {}); + ), initialOverrides); // will also validate config and log warnings let config = PercyConfig.load({ diff --git a/packages/cli-snapshot/src/commands/snapshot.js b/packages/cli-snapshot/src/commands/snapshot.js index 127198498..1a7999554 100644 --- a/packages/cli-snapshot/src/commands/snapshot.js +++ b/packages/cli-snapshot/src/commands/snapshot.js @@ -8,6 +8,13 @@ import YAML from 'yaml'; import { schema } from '../config'; import pkg from '../../package.json'; +// Throw a better error message for invalid urls +function validURL(input, base) { + try { return new URL(input, base || undefined); } catch (error) { + throw new Error(`Invalid URL: ${error.input}`); + } +} + export class Snapshot extends Command { static description = 'Snapshot a list of pages from a file or directory'; @@ -23,27 +30,26 @@ export class Snapshot extends Command { ...flags.config, 'base-url': flags.string({ - char: 'b', - description: 'the url path to serve the static directory from', - default: schema.static.properties.baseUrl.default, - percyrc: 'static.baseUrl' + description: 'the base url pages are hosted at when snapshotting', + char: 'b' }), + 'dry-run': flags.boolean({ + description: 'prints a list of pages to snapshot without snapshotting', + char: 'd' + }), + + // static only flags files: flags.string({ - char: 'f', - multiple: true, description: 'one or more globs matching static file paths to snapshot', default: schema.static.properties.files.default, - percyrc: 'static.files' + percyrc: 'static.files', + multiple: true }), ignore: flags.string({ - char: 'i', - multiple: true, description: 'one or more globs matching static file paths to ignore', - percyrc: 'static.ignore' - }), - 'dry-run': flags.boolean({ - char: 'd', - description: 'prints a list of pages to snapshot without snapshotting' + default: schema.static.properties.ignore.default, + percyrc: 'static.ignore', + multiple: true }) }; @@ -56,52 +62,72 @@ export class Snapshot extends Command { async run() { if (!this.isPercyEnabled()) { - this.log.info('Percy is disabled. Skipping snapshots'); - return; + return this.log.info('Percy is disabled. Skipping snapshots'); } - let config = this.percyrc(); let { pathname } = this.args; if (!fs.existsSync(pathname)) { - return this.error(`Not found: ${pathname}`); - } else if (config.static.baseUrl[0] !== '/') { - return this.error('The base-url flag must begin with a forward slash (/)'); + this.error(`Not found: ${pathname}`); + } + + let { 'base-url': baseUrl, 'dry-run': dry } = this.flags; + let isStatic = fs.lstatSync(pathname).isDirectory(); + + if (baseUrl) { + if (isStatic && !baseUrl.startsWith('/')) { + this.error('The base-url must begin with a forward slash (/) ' + ( + 'when snapshotting static directories')); + } else if (!isStatic && !baseUrl.startsWith('http')) { + this.error('The base-url must include a protocol and hostname ' + ( + 'when snapshotting a list of pages')); + } } - let pages = fs.lstatSync(pathname).isDirectory() - ? await this.loadStaticPages(pathname, config.static) + this.percy = new Percy({ + ...this.percyrc({ static: isStatic ? { baseUrl } : null }), + clientInfo: `${pkg.name}/${pkg.version}`, + server: false + }); + + let pages = isStatic + ? await this.loadStaticPages(pathname) : await this.loadPagesFile(pathname); - if (!pages.length) { - return this.error('No snapshots found'); - } + pages = pages.map(page => { + // allow a list of urls + if (typeof page === 'string') page = { url: page }; - if (this.flags['dry-run']) { - let list = pages.reduce((acc, { name, additionalSnapshots = [] }) => { - return acc.concat(name, additionalSnapshots.map(add => { - let { prefix = '', suffix = '' } = add; - return add.name || `${prefix}${name}${suffix}`; - })); - }, []); - - return this.log.info(`Found ${list.length} ` + ( - `snapshot${list.length === 1 ? '' : 's'}:\n` + - list.join('\n') - )); - } + // validate and prepend the baseUrl + let uri = validURL(page.url, !isStatic && baseUrl); + page.url = uri.href; - this.percy = await Percy.start({ - clientInfo: `${pkg.name}/${pkg.version}`, - server: false, - ...config + // default page name to url /pathname?search#hash + page.name ||= `${uri.pathname}${uri.search}${uri.hash}`; + + return page; }); + let l = pages.length; + if (!l) this.error('No snapshots found'); + + if (!dry) await this.percy.start(); + else this.log.info(`Found ${l} snapshot${l === 1 ? '' : 's'}`); + for (let page of pages) { - this.percy.snapshot(page); + if (dry) { + this.log.info(`Snapshot found: ${page.name}`); + this.log.debug(`-> url: ${page.url}`); + + for (let s of (page.additionalSnapshots || [])) { + let name = s.name || `${s.prefix || ''}${page.name}${s.suffix || ''}`; + this.log.info(`Snapshot found: ${name}`); + this.log.debug(`-> url: ${page.url}`); + } + } else { + this.percy.snapshot(page); + } } - - await this.percy.idle(); } // Called on error, interupt, or after running @@ -131,19 +157,21 @@ export class Snapshot extends Command { } // Starts a static server and returns a list of pages to snapshot. - async loadStaticPages(pathname, { baseUrl, files, ignore }) { - ignore = [].concat(ignore).filter(Boolean); - let paths = await globby(files, { cwd: pathname, ignore }); - let addr = ''; + async loadStaticPages(pathname) { + let { baseUrl, files, ignore } = this.percy.config.static; - if (!this.flags['dry-run']) { - addr = await this.serve(pathname, baseUrl); - } + let paths = await globby(files, { + ignore: [].concat(ignore).filter(Boolean), + cwd: pathname + }); + + let addr = !this.flags['dry-run'] + ? await this.serve(pathname, baseUrl) + : 'http://localhost'; - return paths.sort().map(path => ({ - url: `${addr}${baseUrl}${path}`, - name: `${baseUrl}${path}` - })); + return paths.sort().map(path => ( + `${addr}${baseUrl}${path}` + )); } // Loads pages to snapshot from a js, json, or yaml file. diff --git a/packages/cli-snapshot/src/config.js b/packages/cli-snapshot/src/config.js index 9e16dc0a9..f84c2d1c7 100644 --- a/packages/cli-snapshot/src/config.js +++ b/packages/cli-snapshot/src/config.js @@ -5,7 +5,11 @@ export const schema = { properties: { baseUrl: { type: 'string', - default: '/' + pattern: '^/', + default: '/', + errors: { + pattern: 'must start with a forward slash (/)' + } }, files: { anyOf: [ diff --git a/packages/cli-snapshot/test/snapshot.test.js b/packages/cli-snapshot/test/snapshot.test.js index 6cf787907..75cd9aa59 100644 --- a/packages/cli-snapshot/test/snapshot.test.js +++ b/packages/cli-snapshot/test/snapshot.test.js @@ -41,6 +41,9 @@ describe('percy snapshot', () => { url: 'http://localhost:8000' }], { depth: null }) + ')'); fs.writeFileSync('nope', 'not here'); + fs.writeFileSync('urls.yml', [ + '- /', '- /one', '- /two' + ].join('\n')); }); afterAll(() => { @@ -49,6 +52,7 @@ describe('percy snapshot', () => { fs.unlinkSync('pages-fn.js'); fs.unlinkSync('pages.json'); fs.unlinkSync('pages.yml'); + fs.unlinkSync('urls.yml'); fs.unlinkSync(path.join('public', 'test-1.html')); fs.unlinkSync(path.join('public', 'test-2.html')); fs.unlinkSync(path.join('public', 'test-3.htm')); @@ -82,7 +86,8 @@ describe('percy snapshot', () => { }); it('errors when the provided path doesn\'t exist', async () => { - await expectAsync(Snapshot.run(['./404'])).toBeRejectedWithError('EEXIT: 1'); + await expectAsync(Snapshot.run(['./404'])) + .toBeRejectedWithError('EEXIT: 1'); expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ @@ -90,17 +95,9 @@ describe('percy snapshot', () => { ]); }); - it('errors when the base-url is invalid', async () => { - await expectAsync(Snapshot.run(['./public', '--base-url=wrong'])).toBeRejectedWithError('EEXIT: 1'); - - expect(logger.stdout).toEqual([]); - expect(logger.stderr).toEqual([ - '[percy] Error: The base-url flag must begin with a forward slash (/)' - ]); - }); - it('errors when there are no snapshots to take', async () => { - await expectAsync(Snapshot.run(['./public', '--files=no-match'])).toBeRejectedWithError('EEXIT: 1'); + await expectAsync(Snapshot.run(['./public', '--files=no-match'])) + .toBeRejectedWithError('EEXIT: 1'); expect(logger.stdout).toEqual([]); expect(logger.stderr).toEqual([ @@ -109,15 +106,27 @@ describe('percy snapshot', () => { }); describe('snapshotting static directories', () => { + it('errors when the base-url is invalid', async () => { + await expectAsync(Snapshot.run(['./public', '--base-url=wrong'])) + .toBeRejectedWithError('EEXIT: 1'); + + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy] Error: The base-url must begin with a forward slash (/) ' + + 'when snapshotting static directories' + ]); + }); + it('starts a static server and snapshots matching files', async () => { await Snapshot.run(['./public']); expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', - '[percy] Snapshot taken: /test-3.htm', - '[percy] Snapshot taken: /test-2.html', + '[percy] Processing 3 snapshots...', '[percy] Snapshot taken: /test-1.html', + '[percy] Snapshot taken: /test-2.html', + '[percy] Snapshot taken: /test-3.htm', '[percy] Finalized build #1: https://percy.io/test/test/123' ])); }); @@ -128,21 +137,23 @@ describe('percy snapshot', () => { expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', - '[percy] Snapshot taken: /base/test-3.htm', - '[percy] Snapshot taken: /base/test-2.html', + '[percy] Processing 3 snapshots...', '[percy] Snapshot taken: /base/test-1.html', + '[percy] Snapshot taken: /base/test-2.html', + '[percy] Snapshot taken: /base/test-3.htm', '[percy] Finalized build #1: https://percy.io/test/test/123' ])); }); it('does not take snapshots and prints a list with --dry-run', async () => { await Snapshot.run(['./public', '--dry-run']); + expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ - '[percy] Found 3 snapshots:\n' + - '/test-1.html\n' + - '/test-2.html\n' + - '/test-3.htm' + '[percy] Found 3 snapshots', + '[percy] Snapshot found: /test-1.html', + '[percy] Snapshot found: /test-2.html', + '[percy] Snapshot found: /test-3.htm' ]); }); }); @@ -160,12 +171,44 @@ describe('percy snapshot', () => { await server.close(); }); + it('errors when the base-url is invalid', async () => { + await expectAsync(Snapshot.run(['./pages.yml', '--base-url=/wrong'])) + .toBeRejectedWithError('EEXIT: 1'); + + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy] Error: The base-url must include a protocol and hostname ' + + 'when snapshotting a list of pages' + ]); + }); + + it('errors with unknown file extensions', async () => { + await expectAsync(Snapshot.run(['./nope'])) + .toBeRejectedWithError('EEXIT: 1'); + + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy] Error: Unsupported filetype: ./nope' + ]); + }); + + it('errors when a page url is invalid', async () => { + await expectAsync(Snapshot.run(['./urls.yml'])) + .toBeRejectedWithError('EEXIT: 1'); + + expect(logger.stdout).toEqual([]); + expect(logger.stderr).toEqual([ + '[percy] Error: Invalid URL: /' + ]); + }); + it('snapshots pages from .yaml files', async () => { await Snapshot.run(['./pages.yml']); expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ '[percy] Percy has started!', + '[percy] Processing 1 snapshot...', '[percy] Snapshot taken: YAML Snapshot', '[percy] Finalized build #1: https://percy.io/test/test/123' ]); @@ -177,6 +220,7 @@ describe('percy snapshot', () => { expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ '[percy] Percy has started!', + '[percy] Processing 1 snapshot...', '[percy] Snapshot taken: JSON Snapshot', '[percy] Finalized build #1: https://percy.io/test/test/123' ]); @@ -188,6 +232,7 @@ describe('percy snapshot', () => { expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ '[percy] Percy has started!', + '[percy] Processing 1 snapshot...', '[percy] Snapshot taken: JS Snapshot', '[percy] Snapshot taken: JS Snapshot 2', '[percy] Snapshot taken: Other JS Snapshot', @@ -201,26 +246,32 @@ describe('percy snapshot', () => { expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ '[percy] Percy has started!', + '[percy] Processing 1 snapshot...', '[percy] Snapshot taken: JS Function Snapshot', '[percy] Finalized build #1: https://percy.io/test/test/123' ]); }); - it('errors with unknown file extensions', async () => { - await expectAsync(Snapshot.run(['./nope'])).toBeRejectedWithError('EEXIT: 1'); + it('snapshots pages from a list of URLs', async () => { + await Snapshot.run(['./urls.yml', '--base-url=http://localhost:8000']); - expect(logger.stdout).toEqual([]); - expect(logger.stderr).toEqual([ - '[percy] Error: Unsupported filetype: ./nope' - ]); + expect(logger.stderr).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + '[percy] Percy has started!', + '[percy] Processing 3 snapshots...', + '[percy] Snapshot taken: /', + '[percy] Snapshot taken: /one', + '[percy] Snapshot taken: /two', + '[percy] Finalized build #1: https://percy.io/test/test/123' + ])); }); it('does not take snapshots and prints a list with --dry-run', async () => { await Snapshot.run(['./pages.yml', '--dry-run']); expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ - '[percy] Found 1 snapshot:\n' + - 'YAML Snapshot' + '[percy] Found 1 snapshot', + '[percy] Snapshot found: YAML Snapshot' ]); logger.reset(); @@ -228,10 +279,10 @@ describe('percy snapshot', () => { await Snapshot.run(['./pages.js', '--dry-run']); expect(logger.stderr).toEqual([]); expect(logger.stdout).toEqual([ - '[percy] Found 3 snapshots:\n' + - 'JS Snapshot\n' + - 'JS Snapshot 2\n' + - 'Other JS Snapshot' + '[percy] Found 1 snapshot', + '[percy] Snapshot found: JS Snapshot', + '[percy] Snapshot found: JS Snapshot 2', + '[percy] Snapshot found: Other JS Snapshot' ]); }); }); diff --git a/packages/config/src/validate.js b/packages/config/src/validate.js index 5ec375eab..037ca31bc 100644 --- a/packages/config/src/validate.js +++ b/packages/config/src/validate.js @@ -5,6 +5,7 @@ const { assign, entries } = Object; const ajv = new Ajv({ verbose: true, allErrors: true, + strict: false, schemas: { config: getDefaultSchema() } @@ -57,10 +58,13 @@ export default function validate(config) { if (!result) { for (let error of ajv.errors) { - let { instancePath, keyword, params, message, data } = error; + let { instancePath, keyword, params, message, parentSchema, data } = error; let path = instancePath ? instancePath.substr(1).split('/') : []; - if (keyword === 'required') { + if (parentSchema.errors?.[keyword]) { + let custom = parentSchema.errors[keyword]; + message = typeof custom === 'function' ? custom(error) : custom; + } else if (keyword === 'required') { message = 'missing required property'; path.push(params.missingProperty); } else if (keyword === 'additionalProperties') { diff --git a/packages/config/test/index.test.js b/packages/config/test/index.test.js index 53a7badb0..632266898 100644 --- a/packages/config/test/index.test.js +++ b/packages/config/test/index.test.js @@ -123,8 +123,29 @@ describe('PercyConfig', () => { }); it('returns a failing result with errors', () => { + PercyConfig.addSchema({ + foo: { + type: 'string', + pattern: '^[a-zA-z]*$', + errors: { + pattern: 'must not contain numbers' + } + }, + bar: { + type: 'string', + const: 'baz', + errors: { + const({ data }) { + return `must not be ${data}`; + } + } + } + }); + expect(PercyConfig.validate({ - test: { value: 1, foo: 'bar' } + test: { value: 1, foo: 'bar' }, + foo: 'He11o', + bar: 'qux' })).toEqual({ result: false, errors: [{ @@ -133,6 +154,12 @@ describe('PercyConfig', () => { }, { path: ['test', 'value'], message: 'must be a string, received a number' + }, { + path: ['foo'], + message: 'must not contain numbers' + }, { + path: ['bar'], + message: 'must not be qux' }] }); }); diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 043aaf2f5..4c7f8aba7 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -153,10 +153,10 @@ export function getSnapshotConfig({ additionalSnapshots ??= deprecated.snapshots; } - // default name to the URL /path?search#hash + // default name to the URL /pathname?search#hash if (!name) { let uri = new URL(url); - name = `${uri.path}${uri.search}${uri.hash}`; + name = `${uri.pathname}${uri.search}${uri.hash}`; } // additional snapshots must be named but allow inheritance with a prefix/suffix