From c66c7d1b2825d1d11951b99ab24972516006bf48 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 3 Jul 2019 16:09:37 -0700 Subject: [PATCH] refactor TestSpecPath into TestSpecID --- .vscode/settings.json | 2 ++ src/framework/id.ts | 15 ++++++++++ src/framework/loader.ts | 27 +++++++----------- src/framework/test_group.ts | 8 ++---- src/framework/url_query.ts | 17 ++++++------ src/{tools/run.ts => runtime/cmdline.ts} | 35 ++++++++++++------------ src/runtime/standalone.ts | 26 ++++++++++-------- src/runtime/wpt.ts | 15 +++++----- src/suites/unittests/loading.spec.ts | 14 ++++------ src/suites/unittests/test_group_test.ts | 9 ++---- tools/run | 2 +- 11 files changed, 86 insertions(+), 84 deletions(-) create mode 100644 src/framework/id.ts rename src/{tools/run.ts => runtime/cmdline.ts} (74%) diff --git a/.vscode/settings.json b/.vscode/settings.json index c2fc24bdc64a..0cba38ef31c6 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -15,7 +15,9 @@ ".tscache": true, ".vscode": true, "node_modules": true, + "out": true, "prettier.config.js": true, + "third_party": true, "tsconfig.json": true, "tsconfig.web.json": true, "tslint.json": true, diff --git a/src/framework/id.ts b/src/framework/id.ts new file mode 100644 index 000000000000..f11c422b0827 --- /dev/null +++ b/src/framework/id.ts @@ -0,0 +1,15 @@ +import { ParamsSpec } from './params/index.js'; + +// Identifies a test spec file. +export interface TestSpecID { + // The spec's suite, e.g. 'cts'. + readonly suite: string; + // The spec's path within the suite, e.g. 'command_buffer/compute/basic'. + readonly path: string; +} + +// Identifies a test case (a specific parameterization of a test), within its spec file. +export interface TestCaseID { + readonly name: string; + readonly params: ParamsSpec | null; +} diff --git a/src/framework/loader.ts b/src/framework/loader.ts index e43e7dfac8c4..df5d9a4b45f8 100644 --- a/src/framework/loader.ts +++ b/src/framework/loader.ts @@ -1,8 +1,9 @@ import { GroupRecorder } from './logger.js'; import { ParamsAny, paramsEquals, paramsSupersets } from './params/index.js'; -import { RunCaseIterable, TestCaseID, RunCase } from './test_group.js'; +import { RunCaseIterable, RunCase } from './test_group.js'; import { allowedTestNameCharacters } from './allowed_characters.js'; import { TestSuiteListing } from './listing.js'; +import { TestSpecID, TestCaseID } from './id.js'; // One of the following: // - A shell object describing a directory (from its README.txt). @@ -14,18 +15,13 @@ export interface TestSpecFile { readonly g?: RunCaseIterable; } -// Identifies a test spec (a .spec.ts file). -export interface TestSpecPath { - // The spec's suite, e.g. 'cts'. - readonly suite: string; - // The spec's path within the suite, e.g. 'command_buffer/compute/basic'. - readonly path: string; +// A pending loaded spec (.spec.ts) file, plus identifying information. +export interface TestQueryResult { + readonly id: TestSpecID; + readonly spec: Promise; } type TestQueryResults = IterableIterator; -export interface TestQueryResult extends TestSpecPath { - readonly spec: Promise; -} function* concat(lists: TestQueryResult[][]): TestQueryResults { for (const specs of lists) { @@ -110,8 +106,7 @@ export class TestLoader { const testPrefix = filter.substring(i2 + 1); return [ { - suite, - path, + id: { suite, path }, spec: this.filterByTestMatch(suite, path, testPrefix), }, ]; @@ -132,8 +127,7 @@ export class TestLoader { // - cts:buffers/mapWriteAsync:basic~{filter:"params"} return [ { - suite, - path, + id: { suite, path }, spec: this.filterByParamsMatch(suite, path, test, params), }, ]; @@ -143,8 +137,7 @@ export class TestLoader { // - cts:buffers/mapWriteAsync:basic:{exact:"params"} return [ { - suite, - path, + id: { suite, path }, spec: this.filterByParamsExact(suite, path, test, params), }, ]; @@ -166,7 +159,7 @@ export class TestLoader { const spec: Promise = isReadme ? Promise.resolve({ description }) : this.fileLoader.import(`${suite}/${path}.spec.js`); - entries.push({ suite, path, spec }); + entries.push({ id: { suite, path }, spec }); } } diff --git a/src/framework/test_group.ts b/src/framework/test_group.ts index 12e8fa0465e0..00124543da06 100644 --- a/src/framework/test_group.ts +++ b/src/framework/test_group.ts @@ -1,12 +1,8 @@ import { CaseRecorder, GroupRecorder, TestCaseLiveResult } from './logger.js'; -import { ParamsAny, ParamsSpec, ParamSpecIterable, paramsEquals } from './params/index.js'; +import { ParamsAny, ParamSpecIterable, paramsEquals } from './params/index.js'; import { Fixture } from './fixture.js'; import { allowedTestNameCharacters } from './allowed_characters.js'; - -export interface TestCaseID { - readonly name: string; - readonly params: ParamsSpec | null; -} +import { TestCaseID } from './id.js'; export interface RunCase { readonly id: TestCaseID; diff --git a/src/framework/url_query.ts b/src/framework/url_query.ts index 5457871cb361..27bd3ec40ec4 100644 --- a/src/framework/url_query.ts +++ b/src/framework/url_query.ts @@ -1,5 +1,4 @@ -import { TestSpecPath } from './loader.js'; -import { TestCaseID } from './test_group.js'; +import { TestCaseID, TestSpecID } from './id.js'; export function encodeSelectively(s: string) { let ret = encodeURIComponent(s); @@ -13,12 +12,14 @@ export function encodeSelectively(s: string) { return ret; } -export function makeQueryString(entry: TestSpecPath, testcase: TestCaseID): string { - let s = entry.suite + ':'; - s += entry.path + ':'; - s += testcase.name + ':'; - if (testcase.params) { - s += JSON.stringify(testcase.params); +export function makeQueryString(spec: TestSpecID, testcase?: TestCaseID): string { + let s = spec.suite + ':'; + s += spec.path + ':'; + if (testcase !== undefined) { + s += testcase.name + ':'; + if (testcase.params) { + s += JSON.stringify(testcase.params); + } } return encodeSelectively(s); } diff --git a/src/tools/run.ts b/src/runtime/cmdline.ts similarity index 74% rename from src/tools/run.ts rename to src/runtime/cmdline.ts index a6a3cdfb25e2..835c21df4146 100644 --- a/src/tools/run.ts +++ b/src/runtime/cmdline.ts @@ -5,6 +5,8 @@ import * as process from 'process'; import { TestSpecFile, TestLoader } from '../framework/loader.js'; import { Logger, TestCaseLiveResult } from '../framework/logger.js'; +import { TestSpecID } from '../framework/id.js'; +import { makeQueryString } from '../framework/url_query.js'; function usage(rc: number) { console.log('Usage:'); @@ -17,7 +19,7 @@ if (process.argv.length <= 2) { usage(0); } -if (!fs.existsSync('src/tools/run.ts')) { +if (!fs.existsSync('src/runtime/cmdline.ts')) { console.log('Must be run from repository root'); usage(1); } @@ -42,33 +44,30 @@ for (const a of process.argv.slice(2)) { const listing = await loader.loadTestsFromCmdLine(filterArgs); const log = new Logger(); - const entries = await Promise.all( - Array.from(listing, ({ suite, path, spec }) => - spec.then((s: TestSpecFile) => ({ suite, path, g: s.g })) - ) + const queryResults = await Promise.all( + Array.from(listing, ({ id, spec }) => spec.then((s: TestSpecFile) => ({ id, spec: s }))) ); - const failed: Array<[string, string, TestCaseLiveResult]> = []; - const warned: Array<[string, string, TestCaseLiveResult]> = []; + const failed: Array<[TestSpecID, TestCaseLiveResult]> = []; + const warned: Array<[TestSpecID, TestCaseLiveResult]> = []; // TODO: don't run all tests all at once const running = []; - for (const entry of entries) { - const { suite, path, g } = entry; - if (!g) { + for (const qr of queryResults) { + if (!qr.spec.g) { continue; } - const [rec] = log.record(path); - for (const t of g.iterate(rec)) { + const [rec] = log.record(qr.id.path); + for (const t of qr.spec.g.iterate(rec)) { running.push( (async () => { const res = await t.run(); if (res.status === 'fail') { - failed.push([suite, path, res]); + failed.push([qr.id, res]); } if (res.status === 'warn') { - warned.push([suite, path, res]); + warned.push([qr.id, res]); } })() ); @@ -89,17 +88,17 @@ for (const a of process.argv.slice(2)) { if (warned.length) { console.log(''); console.log('** Warnings **'); - for (const [suite, path, r] of warned) { + for (const [id, r] of warned) { // TODO: actually print query here - console.log(suite + ':' + path, r); + console.log(makeQueryString(id), r); } } if (failed.length) { console.log(''); console.log('** Failures **'); - for (const [suite, path, r] of failed) { + for (const [id, r] of failed) { // TODO: actually print query here - console.log(suite + ':' + path, r); + console.log(makeQueryString(id), r); } } diff --git a/src/runtime/standalone.ts b/src/runtime/standalone.ts index 57f57914ddd5..fe241381c0c4 100644 --- a/src/runtime/standalone.ts +++ b/src/runtime/standalone.ts @@ -4,6 +4,7 @@ import { TestLoader } from '../framework/loader.js'; import { Logger } from '../framework/logger.js'; import { makeQueryString } from '../framework/url_query.js'; import { RunCase } from '../framework/index.js'; +import { TestSpecID } from '../framework/id.js'; const log = new Logger(); @@ -12,7 +13,7 @@ const runButtonCallbacks = new Map(); const resultsJSON = document.getElementById('resultsJSON') as HTMLElement; const resultsVis = document.getElementById('resultsVis') as HTMLElement; -function makeTest(path: string, description: string): HTMLElement { +function makeTest(spec: TestSpecID, description: string): HTMLElement { const test = $('
') .addClass('test') .appendTo(resultsVis); @@ -24,7 +25,7 @@ function makeTest(path: string, description: string): HTMLElement { $('
') .addClass('testname') - .text(path) + .text(makeQueryString(spec)) .appendTo(test); $('
') @@ -102,23 +103,26 @@ function mkCase(testcasesVis: HTMLElement, query: string, t: RunCase) { const loader = new TestLoader(); const listing = await loader.loadTestsFromQuery(window.location.search); - const entries = await Promise.all( - Array.from(listing, ({ suite, path, spec }) => spec.then(s => ({ suite, path, spec: s }))) + + // TODO: everything after this point is very similar across the three runtimes. + + // TODO: start populating page before waiting for everything to load? + const queryResults = await Promise.all( + Array.from(listing, ({ id, spec }) => spec.then(s => ({ id, spec: s }))) ); // TODO: convert listing to tree so it can be displayed as a tree? const runCaseList = []; - for (const entry of entries) { - const { path, spec } = entry; - const testcasesVis = makeTest(path, spec.description); + for (const qr of queryResults) { + const testcasesVis = makeTest(qr.id, qr.spec.description); - if (!spec.g) { + if (!qr.spec.g) { continue; } - const [tRec] = log.record(path); - for (const t of spec.g.iterate(tRec)) { - const query = makeQueryString(entry, t.id); + const [tRec] = log.record(qr.id.path); + for (const t of qr.spec.g.iterate(tRec)) { + const query = makeQueryString(qr.id, t.id); const runCase = mkCase(testcasesVis, query, t); runCaseList.push(runCase); } diff --git a/src/runtime/wpt.ts b/src/runtime/wpt.ts index aca28f2c7d46..6097153dcee2 100644 --- a/src/runtime/wpt.ts +++ b/src/runtime/wpt.ts @@ -16,19 +16,18 @@ declare function async_test(f: (this: WptTestObject) => Promise, name: str const log = new Logger(); const running = []; - const entries = await Promise.all( - Array.from(listing, ({ suite, path, spec }) => spec.then(s => ({ suite, path, spec: s }))) + const queryResults = await Promise.all( + Array.from(listing, ({ id, spec }) => spec.then(s => ({ id, spec: s }))) ); - for (const entry of entries) { - const { path, spec } = entry; - if (!spec.g) { + for (const qr of queryResults) { + if (!qr.spec.g) { continue; } - const [rec] = log.record(path); + const [rec] = log.record(qr.id.path); // TODO: don't run all tests all at once - for (const t of spec.g.iterate(rec)) { + for (const t of qr.spec.g.iterate(rec)) { const run = t.run(); running.push(run); // Note: apparently, async_tests must ALL be added within the same task. @@ -40,7 +39,7 @@ declare function async_test(f: (this: WptTestObject) => Promise, name: str } }); this.done(); - }, makeQueryString(entry, t.id)); + }, makeQueryString(qr.id, t.id)); } } diff --git a/src/suites/unittests/loading.spec.ts b/src/suites/unittests/loading.spec.ts index afad40694da1..31ac5b79c6b1 100644 --- a/src/suites/unittests/loading.spec.ts +++ b/src/suites/unittests/loading.spec.ts @@ -91,9 +91,7 @@ class LoadingTest extends DefaultFixture { async load(filters: string[]) { const listing = await this.loader.loadTests(filters); const entries = Promise.all( - Array.from(listing, ({ suite, path, spec }) => - spec.then((s: TestSpecFile) => ({ suite, path, spec: s })) - ) + Array.from(listing, ({ id, spec }) => spec.then((s: TestSpecFile) => ({ id, spec: s }))) ); return entries; } @@ -139,8 +137,8 @@ g.test('whole group', async t => { { const foo = (await t.load(['suite1:foo:']))[0]; - t.expect(foo.suite === 'suite1'); - t.expect(foo.path === 'foo'); + t.expect(foo.id.suite === 'suite1'); + t.expect(foo.id.path === 'foo'); if (foo.spec.g === undefined) { throw new Error('foo group'); } @@ -181,8 +179,8 @@ g.test('end2end', async t => { throw new Error('listing length'); } - t.expect(l[0].suite === 'suite2'); - t.expect(l[0].path === 'foof'); + t.expect(l[0].id.suite === 'suite2'); + t.expect(l[0].id.path === 'foof'); t.expect(l[0].spec.description === 'desc 2b'); if (l[0].spec.g === undefined) { throw new Error(); @@ -190,7 +188,7 @@ g.test('end2end', async t => { t.expect(l[0].spec.g.iterate instanceof Function); const log = new Logger(); - const [rec, res] = log.record(l[0].path); + const [rec, res] = log.record(l[0].id.path); const rcs = Array.from(l[0].spec.g.iterate(rec)); if (rcs.length !== 2) { throw new Error('iterate length'); diff --git a/src/suites/unittests/test_group_test.ts b/src/suites/unittests/test_group_test.ts index 1c5600fd1480..ace050d7530c 100644 --- a/src/suites/unittests/test_group_test.ts +++ b/src/suites/unittests/test_group_test.ts @@ -1,11 +1,6 @@ -import { - DefaultFixture, - TestGroup, - Fixture, - paramsEquals, - TestCaseID, -} from '../../framework/index.js'; +import { DefaultFixture, TestGroup, Fixture, paramsEquals } from '../../framework/index.js'; import { Logger } from '../../framework/logger.js'; +import { TestCaseID } from '../../framework/id.js'; export class TestGroupTest extends DefaultFixture { async run(g: TestGroup): Promise { diff --git a/tools/run b/tools/run index f72fc4131281..e7aa940d987d 100755 --- a/tools/run +++ b/tools/run @@ -3,4 +3,4 @@ // Run test suites under node. require('../src/tools/setup-ts-in-node.js'); -require('../src/tools/run.ts'); +require('../src/runtime/cmdline.ts');