Merge branch 'master' into glsl-dependent

src/common/framework/params_utils.ts

@@ -1,5 +1,5 @@
 import { comparePublicParamsPaths, Ordering } from './query/compare.js';
-import { kWildcard, kParamSeparator } from './query/separators.js';
+import { kWildcard, kParamSeparator, kParamKVSeparator } from './query/separators.js';
 
 // Consider adding more types here if needed
 export type ParamArgument = void | undefined | number | string | boolean | number[];
@@ -25,7 +25,9 @@ export function extractPublicParams(params: CaseParams): CaseParams {
   return publicParams;
 }
 
-export const badParamValueChars = new RegExp('[=' + kParamSeparator + kWildcard + ']');
+export const badParamValueChars = new RegExp(
+  '[' + kParamKVSeparator + kParamSeparator + kWildcard + ']'
+);
 
 export function publicParamsEquals(x: CaseParams, y: CaseParams): boolean {
   return comparePublicParamsPaths(x, y) === Ordering.Equal;

src/common/framework/query/compare.ts

@@ -1,4 +1,4 @@
-import { CaseParams, extractPublicParams } from '../params_utils.js';
+import { CaseParams, paramKeyIsPublic } from '../params_utils.js';
 import { assert, objectEquals } from '../util/util.js';
 
 import { TestQuery } from './query.js';
@@ -68,18 +68,16 @@ function comparePaths(a: readonly string[], b: readonly string[]): Ordering {
   }
 }
 
-export function comparePublicParamsPaths(a0: CaseParams, b0: CaseParams): Ordering {
-  const a = extractPublicParams(a0);
-  const b = extractPublicParams(b0);
-  const aKeys = Object.keys(a);
+export function comparePublicParamsPaths(a: CaseParams, b: CaseParams): Ordering {
+  const aKeys = Object.keys(a).filter(k => paramKeyIsPublic(k));
   const commonKeys = new Set(aKeys.filter(k => k in b));
 
   for (const k of commonKeys) {
     if (!objectEquals(a[k], b[k])) {
       return Ordering.Unordered;
     }
   }
-  const bKeys = Object.keys(b);
+  const bKeys = Object.keys(b).filter(k => paramKeyIsPublic(k));
   const aRemainingKeys = aKeys.length - commonKeys.size;
   const bRemainingKeys = bKeys.length - commonKeys.size;
   if (aRemainingKeys === 0 && bRemainingKeys === 0) return Ordering.Equal;

src/common/framework/query/stringify_params.ts

@@ -1,16 +1,17 @@
 import {
   CaseParams,
-  extractPublicParams,
   ParamArgument,
   badParamValueChars,
+  paramKeyIsPublic,
 } from '../params_utils.js';
 import { assert } from '../util/util.js';
 
 import { kParamKVSeparator } from './separators.js';
 
 export function stringifyPublicParams(p: CaseParams): string[] {
-  const pub = extractPublicParams(p);
-  return Object.entries(pub).map(([k, v]) => stringifySingleParam(k, v));
+  return Object.keys(p)
+    .filter(k => paramKeyIsPublic(k))
+    .map(k => stringifySingleParam(k, p[k]));
 }
 
 export function stringifySingleParam(k: string, v: ParamArgument) {

src/common/framework/test_group.ts

@@ -7,7 +7,7 @@ import {
   publicParamsEquals,
 } from './params_utils.js';
 import { kPathSeparator } from './query/separators.js';
-import { stringifySingleParam } from './query/stringify_params.js';
+import { stringifyPublicParams } from './query/stringify_params.js';
 import { validQueryPart } from './query/validQueryPart.js';
 import { assert } from './util/util.js';
 
@@ -34,6 +34,7 @@ export function makeTestGroup<F extends Fixture>(fixture: FixtureClass<F>): Test
 // Interface for running tests
 export interface RunCaseIterable {
   iterate(): Iterable<RunCase>;
+  checkCaseNamesAndDuplicates(): void;
 }
 export function makeTestGroupForUnitTesting<F extends Fixture>(
   fixture: FixtureClass<F>
@@ -84,6 +85,12 @@ class TestGroup<F extends Fixture> implements RunCaseIterable, TestGroupBuilder<
     this.tests.push(test);
     return test;
   }
+
+  checkCaseNamesAndDuplicates(): void {
+    for (const test of this.tests) {
+      test.checkCaseNamesAndDuplicates();
+    }
+  }
 }
 
 interface TestBuilderWithName<F extends Fixture, P extends {}> extends TestBuilderWithParams<F, P> {
@@ -109,27 +116,27 @@ class TestBuilder<F extends Fixture, P extends {}> {
     this.testFn = fn;
   }
 
-  params<NewP extends {}>(casesIterable: Iterable<NewP>): TestBuilderWithParams<F, NewP> {
-    assert(this.cases === undefined, 'test case is already parameterized');
-    const cases = Array.from(casesIterable);
-    const seen: CaseParams[] = [];
-    // This is n^2.
-    for (const spec of cases) {
-      const publicParams = extractPublicParams(spec);
-
-      // Check type of public params: can only be (currently):
-      // number, string, boolean, undefined, number[]
-      for (const [k, v] of Object.entries(publicParams)) {
-        stringifySingleParam(k, v); // To check for invalid params values
-      }
+  checkCaseNamesAndDuplicates(): void {
+    if (this.cases === undefined) {
+      return;
+    }
 
+    // This is n^2.
+    const seen: CaseParams[] = [];
+    for (const testcase of this.cases) {
+      // stringifyPublicParams also checks for invalid params values
+      const testcaseString = stringifyPublicParams(testcase);
       assert(
-        !seen.some(x => publicParamsEquals(x, publicParams)),
-        'Duplicate test case params: ' + JSON.stringify(publicParams)
+        !seen.some(x => publicParamsEquals(x, testcase)),
+        `Duplicate public test case params: ${testcaseString}`
       );
-      seen.push(publicParams);
+      seen.push(testcase);
     }
-    this.cases = cases;
+  }
+
+  params<NewP extends {}>(casesIterable: Iterable<NewP>): TestBuilderWithParams<F, NewP> {
+    assert(this.cases === undefined, 'test case is already parameterized');
+    this.cases = Array.from(casesIterable);
 
     return (this as unknown) as TestBuilderWithParams<F, NewP>;
   }

src/common/framework/util/util.ts

@@ -1,8 +1,8 @@
 import { timeout } from './timeout.js';
 
-export function assert(condition: boolean, msg?: string): asserts condition {
+export function assert(condition: boolean, msg?: string | (() => string)): asserts condition {
   if (!condition) {
-    throw new Error(msg);
+    throw new Error(msg && (typeof msg === 'string' ? msg : msg()));
   }
 }
 

src/common/runtime/wpt.ts

@@ -21,10 +21,7 @@ declare function async_test(f: (this: WptTestObject) => Promise<void>, name: str
   assert(qs.length === 1, 'currently, there must be exactly one ?q=');
   const testcases = await loader.loadTests(qs[0]);
 
-  const log = await addWPTTests(testcases);
-
-  const resultsElem = document.getElementById('results') as HTMLElement;
-  resultsElem.textContent = log.asJSON(2);
+  await addWPTTests(testcases);
 })();
 
 // Note: async_tests must ALL be added within the same task. This function *must not* be async.

src/common/templates/cts.html

@@ -25,24 +25,4 @@
 
 <script src=/resources/testharness.js></script>
 <script src=/resources/testharnessreport.js></script>
-<style>
-#results {
-    font-family: monospace;
-    width: 100%;
-    height: 15em;
-}
-
-/* Test Name column */
-#results > tbody > tr > td:nth-child(2) {
-    word-break: break-word;
-}
-
-/* Message column */
-#results > tbody > tr > td:nth-child(3) {
-    white-space: pre-wrap;
-    word-break: break-word;
-}
-</style>
-
-<textarea id=results></textarea>
 <script type=module src=/webgpu/common/runtime/wpt.js></script>

src/common/tools/crawl.ts

@@ -35,6 +35,8 @@ export async function crawl(suite: string): Promise<TestSuiteListingEntry[]> {
       assert(mod.description !== undefined, 'Test spec file missing description: ' + filename);
       assert(mod.g !== undefined, 'Test spec file missing TestGroup definition: ' + filename);
 
+      mod.g.checkCaseNamesAndDuplicates();
+
       const path = filepathWithoutExtension.split('/');
       entries.push({ file: path, description: mod.description.trim() });
     } else if (path.basename(file) === 'README.txt') {

src/common/tools/gen_wpt_cts_html.ts

@@ -5,6 +5,7 @@ import { DefaultTestFileLoader } from '../framework/file_loader.js';
 import { TestQueryMultiTest } from '../framework/query/query.js';
 import { kBigSeparator, kWildcard } from '../framework/query/separators.js';
 import { TestSuiteListingEntry } from '../framework/test_suite_listing.js';
+import { assert } from '../framework/util/util.js';
 
 function printUsageAndExit(rc: number): void {
   console.error(`\
@@ -59,9 +60,9 @@ const [
       .split('\n')
       .filter(a => a.length)
       .sort((a, b) => b.length - a.length);
-    const expectationLines = (await fs.readFile(expectationsFile, 'utf8'))
-      .split('\n')
-      .filter(l => l.length);
+    const expectationLines = new Set(
+      (await fs.readFile(expectationsFile, 'utf8')).split('\n').filter(l => l.length)
+    );
 
     const expectations: Map<string, string[]> = new Map();
     for (const prefix of argsPrefixes) {
@@ -77,7 +78,7 @@ const [
           continue expLoop;
         }
       }
-      throw new Error('All input lines must start with one of the prefixes. ' + exp);
+      console.log('note: ignored expectation: ' + exp);
     }
 
     const loader = new DefaultTestFileLoader();
@@ -90,7 +91,17 @@ const [
 
       lines.push(undefined); // output blank line between prefixes
       for (const q of tree.iterateCollapsedQueries()) {
-        lines.push(prefix + q.toString());
+        const urlQueryString = prefix + q.toString(); // "?worker=0&q=..."
+        // Check for a safe-ish path length limit. Filename must be <= 255, and on Windows the whole
+        // path must be <= 259. Leave room for e.g.:
+        // 'c:\b\s\w\xxxxxxxx\layout-test-results\external\wpt\webgpu\cts_worker=0_q=...-actual.txt'
+        assert(
+          urlQueryString.length < 185,
+          'Generated test variant would produce too-long -actual.txt filename. \
+Try broadening suppressions to avoid long test variant names. ' +
+            urlQueryString
+        );
+        lines.push(urlQueryString);
       }
     }
     await generateFile(lines);

src/unittests/test_group.spec.ts

@@ -93,33 +93,69 @@ g.test('duplicate_test_name').fn(t => {
   });
 });
 
-g.test('duplicate_test_params').fn(t => {
-  const g = makeTestGroupForUnitTesting(UnitTest);
+g.test('duplicate_test_params,none').fn(t => {
+  {
+    const g = makeTestGroupForUnitTesting(UnitTest);
+    g.test('abc')
+      .params([])
+      .fn(() => {});
+    g.checkCaseNamesAndDuplicates();
+  }
 
-  t.shouldThrow('Error', () => {
+  {
+    const g = makeTestGroupForUnitTesting(UnitTest);
+    g.test('abc').fn(() => {});
+    g.checkCaseNamesAndDuplicates();
+  }
+
+  {
+    const g = makeTestGroupForUnitTesting(UnitTest);
+    g.test('abc')
+      .params([
+        { a: 1 }, //
+      ])
+      .fn(() => {});
+    g.checkCaseNamesAndDuplicates();
+  }
+});
+
+g.test('duplicate_test_params,basic').fn(t => {
+  {
+    const g = makeTestGroupForUnitTesting(UnitTest);
     g.test('abc')
       .params([
         { a: 1 }, //
         { a: 1 },
       ])
-      .fn(() => {
-        //
-      });
-  });
+      .fn(() => {});
+    t.shouldThrow('Error', () => {
+      g.checkCaseNamesAndDuplicates();
+    });
+  }
+  {
+    const g = makeTestGroupForUnitTesting(UnitTest);
+    g.test('abc')
+      .params([
+        { a: 1, b: 3 }, //
+        { b: 3, a: 1 },
+      ])
+      .fn(() => {});
+    t.shouldThrow('Error', () => {
+      g.checkCaseNamesAndDuplicates();
+    });
+  }
 });
 
 g.test('duplicate_test_params,with_different_private_params').fn(t => {
   const g = makeTestGroupForUnitTesting(UnitTest);
-
+  g.test('abc')
+    .params([
+      { a: 1, _b: 1 }, //
+      { a: 1, _b: 2 },
+    ])
+    .fn(() => {});
   t.shouldThrow('Error', () => {
-    g.test('abc')
-      .params([
-        { a: 1, _b: 1 }, //
-        { a: 1, _b: 2 },
-      ])
-      .fn(() => {
-        //
-      });
+    g.checkCaseNamesAndDuplicates();
   });
 });
 
@@ -139,11 +175,21 @@ g.test('invalid_test_name').fn(t => {
   }
 });
 
-g.test('valid_param_value').fn(() => {
+g.test('param_value,valid').fn(() => {
   const g = makeTestGroup(UnitTest);
   g.test('a').params([{ x: JSON.stringify({ a: 1, b: 2 }) }]);
 });
 
+g.test('param_value,invalid').fn(t => {
+  for (const badChar of ';=*') {
+    const g = makeTestGroupForUnitTesting(UnitTest);
+    g.test('a').params([{ badChar }]);
+    t.shouldThrow('Error', () => {
+      g.checkCaseNamesAndDuplicates();
+    });
+  }
+});
+
 g.test('throws').fn(async t0 => {
   const g = makeTestGroupForUnitTesting(UnitTest);