From fef5ce9a8c6df045d78091a91a70ca3533812027 Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Wed, 2 Aug 2023 14:28:10 -0400 Subject: [PATCH 1/6] Add vdiff subcommands --- bin/d2l-test-runner.js | 32 ++++++++++++-- bin/migrate-goldens.js | 40 ------------------ src/server/cli/test-runner.js | 30 +++++++++---- src/server/cli/vdiff/migrate.js | 42 +++++++++++++++++++ .../server/cli/vdiff/report.js | 2 +- 5 files changed, 93 insertions(+), 53 deletions(-) delete mode 100755 bin/migrate-goldens.js create mode 100755 src/server/cli/vdiff/migrate.js rename bin/d2l-test-vdiff-report.js => src/server/cli/vdiff/report.js (82%) mode change 100755 => 100644 diff --git a/bin/d2l-test-runner.js b/bin/d2l-test-runner.js index c4d07e25..99b63865 100755 --- a/bin/d2l-test-runner.js +++ b/bin/d2l-test-runner.js @@ -1,7 +1,33 @@ #!/usr/bin/env node -import { argv } from 'node:process'; +import { argv, stdout } from 'node:process'; +import commandLineArgs from 'command-line-args'; import { runner } from '../src/server/cli/test-runner.js'; -const options = await runner.getOptions(argv); +const cli = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true }); -await runner.start(options); +if (cli.subcommand === 'vdiff') { + const vdiff = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true, argv: cli._unknown || [] }); + + if (!vdiff.subcommand) { + runTests(); + } else if (vdiff.subcommand === 'golden') { + argv.splice(argv.findIndex(a => a === 'golden'), 1, '--golden'); + stdout.write('\nGenerating vdiff goldens...\n'); + runTests(); + } else if (vdiff.subcommand === 'report') { + import('../src/server/cli/vdiff/report.js'); + } else if (vdiff.subcommand === 'migrate') { + const { migrate } = await import('../src/server/cli/vdiff/migrate.js'); + await migrate(vdiff._unknown); + } else { + stdout.write(`\nfatal: unknown subcomamnd: ${vdiff.subcommand}\n`); + } +} +else { + runTests(); +} + +async function runTests() { + const options = await runner.getOptions(argv); + await runner.start(options); +} diff --git a/bin/migrate-goldens.js b/bin/migrate-goldens.js deleted file mode 100755 index 2977d990..00000000 --- a/bin/migrate-goldens.js +++ /dev/null @@ -1,40 +0,0 @@ -#!/usr/bin/env node -import { appendFile, mkdir, readFile, rename, rm } from 'node:fs/promises'; -import { join, normalize, parse } from 'node:path'; -import commandLineArgs from 'command-line-args'; -import { glob } from 'glob'; -import { PATHS } from '../src/server/visual-diff-plugin.js'; -import { stdout } from 'node:process'; - -const { pattern = './test/**' } = commandLineArgs({ name: 'pattern', type: String, defaultOption: true }, { partial: true }); -const oldSuffix = 'screenshots/ci/golden'; -const dirs = await glob(`${pattern}/${oldSuffix}`, { ignore: 'node_modules/**', posix: true }); -let fileCount = 0; - -const gitignore = await readFile('.gitignore', { encoding: 'UTF8' }).catch(() => ''); -if (!new RegExp(`${PATHS.VDIFF_ROOT}/(\n|$)`).test(gitignore)) { - const newline = gitignore.endsWith('\n') ? '' : '\n'; - await appendFile('.gitignore', `${newline}${PATHS.VDIFF_ROOT}/\n`); -} - -await Promise.all(dirs.map(async dir => { - const files = await glob(`${dir}/*/*.png`, { posix: true }); - - await Promise.all(files.map(async file => { - fileCount += 1; - const { base: name, dir } = parse(file); - const dirName = parse(dir).name; - - const newName = name - .replace(/^d2l-/, '') - .replace(new RegExp(`^${dirName}-`), ''); - - const newDir = dir.replace(`${oldSuffix}/${dirName}`, `${PATHS.GOLDEN}/${dirName}/chromium`); - - await mkdir(newDir, { recursive: true }); - return rename(file, join(newDir, newName)); - })); - return rm(normalize(join(dir, '..', '..')), { recursive: true }); -})); - -stdout.write(`\nMigrated ${fileCount} ${fileCount === 1 ? 'golden' : 'goldens'} found in ${dirs.length} test ${dirs.length === 1 ? 'directory' : 'directories'}\n`); diff --git a/src/server/cli/test-runner.js b/src/server/cli/test-runner.js index 0bc64c59..6fd3d594 100755 --- a/src/server/cli/test-runner.js +++ b/src/server/cli/test-runner.js @@ -66,12 +66,6 @@ async function getTestRunnerOptions(argv = []) { description: 'Run tests in Firefox', order: 3 }, - { - name: 'golden', - type: Boolean, - description: 'Generate new golden screenshots. Ignored unless group is "vdiff".', - order: 14 - }, { name: 'grep', alias: 'g', @@ -83,7 +77,7 @@ async function getTestRunnerOptions(argv = []) { name: 'help', type: Boolean, description: 'Print usage information and exit', - order: 15 + order: 14 }, { name: 'open', @@ -134,19 +128,37 @@ async function getTestRunnerOptions(argv = []) { }, { header: 'Usage', - content: 'd2l-test-runner [options]', + content: 'd2l-test-runner [options]\nd2l-test-runner [options]\n', }, { header: 'Options', optionList: optionDefinitions .map(o => { - o.description += '\n'; const longAlias = optionDefinitions.find(clone => clone !== o && clone.longAlias === o.name)?.name; if (longAlias) o.name += `, --${longAlias}`; return o; }) .filter(o => 'order' in o) .sort((a, b) => (a.order > b.order ? 1 : -1)) + }, + { + header: 'Commands', + content: [{ + example: 'vdiff', + desc: 'Run tests for the vdiff group' + }, + { + example: 'vdiff report', + desc: 'Open the latest vdiff report' + }, + { + example: 'vdiff golden', + desc: 'Generate new golden screenshots' + }, + { + example: 'vdiff migrate [directory]', + desc: 'Migrate from @brightspace-ui/visual-diff. Restrict which goldens are migrated with a directory glob.' + }] } ]); process.stdout.write(help); diff --git a/src/server/cli/vdiff/migrate.js b/src/server/cli/vdiff/migrate.js new file mode 100755 index 00000000..5036a7cc --- /dev/null +++ b/src/server/cli/vdiff/migrate.js @@ -0,0 +1,42 @@ +#!/usr/bin/env node +import { appendFile, mkdir, readFile, rename, rm } from 'node:fs/promises'; +import { join, normalize, parse } from 'node:path'; +import commandLineArgs from 'command-line-args'; +import { glob } from 'glob'; +import { PATHS } from '../../visual-diff-plugin.js'; +import { stdout } from 'node:process'; + +export async function migrate(argv) { + const { pattern = './test/**' } = commandLineArgs({ name: 'pattern', type: String, defaultOption: true }, { partial: true, argv }); + const oldSuffix = 'screenshots/ci/golden'; + const dirs = await glob(`${pattern}/${oldSuffix}`, { ignore: 'node_modules/**', posix: true }); + let fileCount = 0; + + const gitignore = await readFile('.gitignore', { encoding: 'UTF8' }).catch(() => ''); + if (!new RegExp(`${PATHS.VDIFF_ROOT}/(\n|$)`).test(gitignore)) { + const newline = gitignore.endsWith('\n') ? '' : '\n'; + await appendFile('.gitignore', `${newline}${PATHS.VDIFF_ROOT}/\n`); + } + + await Promise.all(dirs.map(async dir => { + const files = await glob(`${dir}/*/*.png`, { posix: true }); + + await Promise.all(files.map(async file => { + fileCount += 1; + const { base: name, dir } = parse(file); + const dirName = parse(dir).name; + + const newName = name + .replace(/^d2l-/, '') + .replace(new RegExp(`^${dirName}-`), ''); + + const newDir = dir.replace(`${oldSuffix}/${dirName}`, `${PATHS.GOLDEN}/${dirName}/chromium`); + + await mkdir(newDir, { recursive: true }); + return rename(file, join(newDir, newName)); + })); + return rm(normalize(join(dir, '..', '..')), { recursive: true }); + })); + + stdout.write(`\nMigrated ${fileCount} ${fileCount === 1 ? 'golden' : 'goldens'} found in ${dirs.length} test ${dirs.length === 1 ? 'directory' : 'directories'}\n`); +} diff --git a/bin/d2l-test-vdiff-report.js b/src/server/cli/vdiff/report.js old mode 100755 new mode 100644 similarity index 82% rename from bin/d2l-test-vdiff-report.js rename to src/server/cli/vdiff/report.js index 3ee7b9a9..3406e854 --- a/bin/d2l-test-vdiff-report.js +++ b/src/server/cli/vdiff/report.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -import { PATHS } from '../src/server/visual-diff-plugin.js'; +import { PATHS } from '../../visual-diff-plugin.js'; import { startDevServer } from '@web/dev-server'; await startDevServer({ From eaa6c863b55ddad0760eaf4741061e30c24cf6b8 Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Wed, 2 Aug 2023 16:03:51 -0400 Subject: [PATCH 2/6] Default to all goldens everywhere --- src/server/cli/vdiff/migrate.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/cli/vdiff/migrate.js b/src/server/cli/vdiff/migrate.js index 5036a7cc..cddf3bc9 100755 --- a/src/server/cli/vdiff/migrate.js +++ b/src/server/cli/vdiff/migrate.js @@ -6,8 +6,8 @@ import { glob } from 'glob'; import { PATHS } from '../../visual-diff-plugin.js'; import { stdout } from 'node:process'; -export async function migrate(argv) { - const { pattern = './test/**' } = commandLineArgs({ name: 'pattern', type: String, defaultOption: true }, { partial: true, argv }); +export async function migrate(argv = []) { + const { pattern = './**' } = commandLineArgs({ name: 'pattern', type: String, defaultOption: true }, { partial: true, argv }); const oldSuffix = 'screenshots/ci/golden'; const dirs = await glob(`${pattern}/${oldSuffix}`, { ignore: 'node_modules/**', posix: true }); let fileCount = 0; From 2efbb4db47b447a5dce4ace15963365ac9ac68c2 Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Wed, 2 Aug 2023 16:23:14 -0400 Subject: [PATCH 3/6] Clean up binaries --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 5f6b48dc..e06b5678 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,7 @@ }, "bin": { "d2l-test-runner": "./bin/d2l-test-runner.js", - "d2l-test-vdiff-report": "./bin/d2l-test-vdiff-report.js", - "migrate-goldens": "./bin/migrate-goldens.js" + "dtr": "./bin/d2l-test-runner.js" }, "author": "D2L Corporation", "license": "Apache-2.0", From 436634ca33f36c69e78ed47fb502668606c25f55 Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Wed, 2 Aug 2023 16:43:31 -0400 Subject: [PATCH 4/6] Re-add --golden, but as a hidden option --- src/server/cli/test-runner.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/server/cli/test-runner.js b/src/server/cli/test-runner.js index 6fd3d594..5f9f8126 100755 --- a/src/server/cli/test-runner.js +++ b/src/server/cli/test-runner.js @@ -66,6 +66,10 @@ async function getTestRunnerOptions(argv = []) { description: 'Run tests in Firefox', order: 3 }, + { + name: 'golden', + type: Boolean + }, { name: 'grep', alias: 'g', From 716cf93c8f3711560b78eb67b0bbe37e8651ff0e Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Thu, 3 Aug 2023 19:25:28 -0400 Subject: [PATCH 5/6] Add binary tests --- bin/d2l-test-runner.js | 10 +++-- src/server/bail.js | 7 ++++ src/server/cli/vdiff/migrate.js | 6 ++- src/server/cli/vdiff/report.js | 3 ++ test/bin/d2l-test-runner.test.js | 65 +++++++++++++++++++++++++++++++- 5 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 src/server/bail.js diff --git a/bin/d2l-test-runner.js b/bin/d2l-test-runner.js index 99b63865..ddd0e0f7 100755 --- a/bin/d2l-test-runner.js +++ b/bin/d2l-test-runner.js @@ -1,9 +1,11 @@ #!/usr/bin/env node -import { argv, stdout } from 'node:process'; import commandLineArgs from 'command-line-args'; +import process from 'node:process'; import { runner } from '../src/server/cli/test-runner.js'; -const cli = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true }); +const { argv, stdout } = process; + +const cli = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true, argv }); if (cli.subcommand === 'vdiff') { const vdiff = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true, argv: cli._unknown || [] }); @@ -15,10 +17,10 @@ if (cli.subcommand === 'vdiff') { stdout.write('\nGenerating vdiff goldens...\n'); runTests(); } else if (vdiff.subcommand === 'report') { - import('../src/server/cli/vdiff/report.js'); + await import('../src/server/cli/vdiff/report.js'); } else if (vdiff.subcommand === 'migrate') { const { migrate } = await import('../src/server/cli/vdiff/migrate.js'); - await migrate(vdiff._unknown); + await migrate.start(vdiff._unknown); } else { stdout.write(`\nfatal: unknown subcomamnd: ${vdiff.subcommand}\n`); } diff --git a/src/server/bail.js b/src/server/bail.js new file mode 100644 index 00000000..32d6f0c5 --- /dev/null +++ b/src/server/bail.js @@ -0,0 +1,7 @@ +const bail = new Set(); +const bailOn = key => bail.delete(key); + +export { + bail, + bailOn +}; diff --git a/src/server/cli/vdiff/migrate.js b/src/server/cli/vdiff/migrate.js index cddf3bc9..430af78c 100755 --- a/src/server/cli/vdiff/migrate.js +++ b/src/server/cli/vdiff/migrate.js @@ -6,7 +6,7 @@ import { glob } from 'glob'; import { PATHS } from '../../visual-diff-plugin.js'; import { stdout } from 'node:process'; -export async function migrate(argv = []) { +async function start(argv = []) { const { pattern = './**' } = commandLineArgs({ name: 'pattern', type: String, defaultOption: true }, { partial: true, argv }); const oldSuffix = 'screenshots/ci/golden'; const dirs = await glob(`${pattern}/${oldSuffix}`, { ignore: 'node_modules/**', posix: true }); @@ -40,3 +40,7 @@ export async function migrate(argv = []) { stdout.write(`\nMigrated ${fileCount} ${fileCount === 1 ? 'golden' : 'goldens'} found in ${dirs.length} test ${dirs.length === 1 ? 'directory' : 'directories'}\n`); } + +export const migrate = { + start +}; diff --git a/src/server/cli/vdiff/report.js b/src/server/cli/vdiff/report.js index 3406e854..798fdcb9 100644 --- a/src/server/cli/vdiff/report.js +++ b/src/server/cli/vdiff/report.js @@ -1,7 +1,10 @@ #!/usr/bin/env node +import { bailOn } from '../../bail.js'; import { PATHS } from '../../visual-diff-plugin.js'; import { startDevServer } from '@web/dev-server'; +bailOn('report') || + await startDevServer({ config: { nodeResolve: false, diff --git a/test/bin/d2l-test-runner.test.js b/test/bin/d2l-test-runner.test.js index 425751ec..75403516 100644 --- a/test/bin/d2l-test-runner.test.js +++ b/test/bin/d2l-test-runner.test.js @@ -1,18 +1,79 @@ import { assert, restore, stub } from 'sinon'; -import { argv } from 'node:process'; +import { bail } from '../../src/server/bail.js'; +import { expect } from 'chai'; +import { migrate } from '../../src/server/cli/vdiff/migrate.js'; +import process from 'node:process'; import { runner } from '../../src/server/cli/test-runner.js'; +const { argv, stdout } = process; + +const run = async() => { + await import(`../../bin/d2l-test-runner.js?${Math.random()}`); +}; + describe('d2l-test-runner', () => { + beforeEach(() => { + bail.clear(); + }); + it('starts test runner with options', async() => { const opts = { my: 'options' }; const optionsStub = stub(runner, 'getOptions').returns(opts); const startStub = stub(runner, 'start'); - await import('../../bin/d2l-test-runner.js'); + await run(); assert.calledOnceWithExactly(optionsStub, argv); assert.calledOnceWithExactly(startStub, opts); restore(); }); + + it('runs report.js', async() => { + const optionsStub = stub(runner, 'getOptions'); + const startStub = stub(runner, 'start'); + + bail.add('report'); + argv.splice(0, argv.length, 'fake-node', 'fake-test-runner', 'vdiff', 'report'); + await run(); + + expect(bail).to.not.include('report'); + + assert.notCalled(optionsStub); + assert.notCalled(startStub); + + restore(); + }); + + it('generates goldens', async() => { + const optionsStub = stub(runner, 'getOptions'); + const startStub = stub(runner, 'start'); + const stdoutStub = stub(stdout, 'write'); + + argv.splice(0, argv.length, 'fake-node', 'fake-test-runner', 'vdiff', 'golden'); + await run(); + + expect(argv).to.deep.equal(['fake-node', 'fake-test-runner', 'vdiff', '--golden']); + assert.calledOnceWithExactly(optionsStub, argv); + assert.calledOnce(startStub); + assert.calledOnceWithExactly(stdoutStub, '\nGenerating vdiff goldens...\n'); + + restore(); + }); + + it('calls migrate()', async() => { + const migrateStub = stub(migrate, 'start'); + const optionsStub = stub(runner, 'getOptions'); + const startStub = stub(runner, 'start'); + + argv.splice(0, argv.length, 'fake-node', 'fake-test-runner', 'vdiff', 'migrate', './test/**/dir'); + await run(); + + assert.calledOnceWithExactly(migrateStub, ['./test/**/dir']); + assert.notCalled(optionsStub); + assert.notCalled(startStub); + + restore(); + }); + }); From 126be01fb6105945768738d3a421ad0b2f845efb Mon Sep 17 00:00:00 2001 From: Danny Gleckler Date: Fri, 4 Aug 2023 09:25:59 -0400 Subject: [PATCH 6/6] Cleanup --- bin/d2l-test-runner.js | 9 ++++----- src/server/bail.js | 7 ------- src/server/cli/vdiff/report.js | 29 +++++++++++++++-------------- test/bin/d2l-test-runner.test.js | 21 +++++++-------------- 4 files changed, 26 insertions(+), 40 deletions(-) delete mode 100644 src/server/bail.js diff --git a/bin/d2l-test-runner.js b/bin/d2l-test-runner.js index ddd0e0f7..6ac43891 100755 --- a/bin/d2l-test-runner.js +++ b/bin/d2l-test-runner.js @@ -4,7 +4,6 @@ import process from 'node:process'; import { runner } from '../src/server/cli/test-runner.js'; const { argv, stdout } = process; - const cli = commandLineArgs({ name: 'subcommand', defaultOption: true }, { stopAtFirstUnknown: true, argv }); if (cli.subcommand === 'vdiff') { @@ -17,15 +16,15 @@ if (cli.subcommand === 'vdiff') { stdout.write('\nGenerating vdiff goldens...\n'); runTests(); } else if (vdiff.subcommand === 'report') { - await import('../src/server/cli/vdiff/report.js'); - } else if (vdiff.subcommand === 'migrate') { + const { report } = await import('../src/server/cli/vdiff/report.js'); + await report.start(); + } else if (vdiff.subcommand === 'migrate') { const { migrate } = await import('../src/server/cli/vdiff/migrate.js'); await migrate.start(vdiff._unknown); } else { stdout.write(`\nfatal: unknown subcomamnd: ${vdiff.subcommand}\n`); } -} -else { +} else { runTests(); } diff --git a/src/server/bail.js b/src/server/bail.js deleted file mode 100644 index 32d6f0c5..00000000 --- a/src/server/bail.js +++ /dev/null @@ -1,7 +0,0 @@ -const bail = new Set(); -const bailOn = key => bail.delete(key); - -export { - bail, - bailOn -}; diff --git a/src/server/cli/vdiff/report.js b/src/server/cli/vdiff/report.js index 798fdcb9..79d468b7 100644 --- a/src/server/cli/vdiff/report.js +++ b/src/server/cli/vdiff/report.js @@ -1,18 +1,19 @@ #!/usr/bin/env node -import { bailOn } from '../../bail.js'; import { PATHS } from '../../visual-diff-plugin.js'; import { startDevServer } from '@web/dev-server'; -bailOn('report') || - -await startDevServer({ - config: { - nodeResolve: false, - open: `./${PATHS.REPORT_ROOT}/`, - rootDir: `${PATHS.VDIFF_ROOT}`, - preserveSymlinks: false, - watch: true - }, - readCliArgs: false, - readFileConfig: false -}); +export const report = { + start() { + return startDevServer({ + config: { + nodeResolve: false, + open: `./${PATHS.REPORT_ROOT}/`, + rootDir: `${PATHS.VDIFF_ROOT}`, + preserveSymlinks: false, + watch: true + }, + readCliArgs: false, + readFileConfig: false + }); + } +}; diff --git a/test/bin/d2l-test-runner.test.js b/test/bin/d2l-test-runner.test.js index 75403516..ce7d3e42 100644 --- a/test/bin/d2l-test-runner.test.js +++ b/test/bin/d2l-test-runner.test.js @@ -1,8 +1,8 @@ import { assert, restore, stub } from 'sinon'; -import { bail } from '../../src/server/bail.js'; import { expect } from 'chai'; import { migrate } from '../../src/server/cli/vdiff/migrate.js'; import process from 'node:process'; +import { report } from '../../src/server/cli/vdiff/report.js'; import { runner } from '../../src/server/cli/test-runner.js'; const { argv, stdout } = process; @@ -13,8 +13,8 @@ const run = async() => { describe('d2l-test-runner', () => { - beforeEach(() => { - bail.clear(); + afterEach(() => { + restore(); }); it('starts test runner with options', async() => { @@ -29,20 +29,17 @@ describe('d2l-test-runner', () => { restore(); }); - it('runs report.js', async() => { + it('starts report server', async() => { + const reportStub = stub(report, 'start'); const optionsStub = stub(runner, 'getOptions'); const startStub = stub(runner, 'start'); - bail.add('report'); argv.splice(0, argv.length, 'fake-node', 'fake-test-runner', 'vdiff', 'report'); await run(); - expect(bail).to.not.include('report'); - + assert.calledOnce(reportStub); assert.notCalled(optionsStub); assert.notCalled(startStub); - - restore(); }); it('generates goldens', async() => { @@ -57,11 +54,9 @@ describe('d2l-test-runner', () => { assert.calledOnceWithExactly(optionsStub, argv); assert.calledOnce(startStub); assert.calledOnceWithExactly(stdoutStub, '\nGenerating vdiff goldens...\n'); - - restore(); }); - it('calls migrate()', async() => { + it('starts migration', async() => { const migrateStub = stub(migrate, 'start'); const optionsStub = stub(runner, 'getOptions'); const startStub = stub(runner, 'start'); @@ -72,8 +67,6 @@ describe('d2l-test-runner', () => { assert.calledOnceWithExactly(migrateStub, ['./test/**/dir']); assert.notCalled(optionsStub); assert.notCalled(startStub); - - restore(); }); });