[pull] main from gpuweb:main

.github/pull_request_template.md

@@ -14,9 +14,9 @@ Issue: #<!-- Fill in the issue number here. See docs/intro/life_of.md -->
 
 **Requirements for [reviewer sign-off](https://github.com/gpuweb/cts/blob/main/docs/reviews.md):**
 
-- [ ] Tests are properly located in the test tree.
-- [ ] [Test descriptions](https://github.com/gpuweb/cts/blob/main/docs/intro/plans.md) allow a reader to "read only the test plans and evaluate coverage completeness", and accurately reflect the test code.
+- [ ] Tests are properly located.
+- [ ] [Test descriptions](https://github.com/gpuweb/cts/blob/main/docs/intro/plans.md) are accurate and complete.
 - [ ] Tests provide complete coverage (including validation control cases). **Missing coverage MUST be covered by TODOs.**
-- [ ] Helpers and types promote readability and maintainability.
+- [ ] Tests avoid [over-parameterization](https://github.com/gpuweb/cts/blob/main/docs/organization.md#parameterization) (see case count report).
 
 When landing this PR, be sure to make any necessary issue status updates.

.github/workflows/pr-comment.yml

@@ -0,0 +1,94 @@
+name: pr-comment
+
+on:
+  # Runs once when the 'pr' workflow is requested, and again when it's completed
+  workflow_run:
+    workflows:
+      - pr
+    types:
+      - requested
+      - completed
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    permissions:
+      pull-requests: write
+    steps:
+      - # https://github.com/orgs/community/discussions/25220#discussioncomment-11316244
+        name: 'Get PR context'
+        id: pr-context
+        env:
+          GH_TOKEN: ${{ github.token }}
+          PR_TARGET_REPO: ${{ github.repository }}
+          PR_BRANCH: |-
+            ${{
+              (github.event.workflow_run.head_repository.owner.login != github.event.workflow_run.repository.owner.login)
+                && format('{0}:{1}', github.event.workflow_run.head_repository.owner.login, github.event.workflow_run.head_branch)
+                || github.event.workflow_run.head_branch
+            }}
+        run: |
+          gh pr view --repo "${PR_TARGET_REPO}" "${PR_BRANCH}" \
+             --json 'number' --jq '"number=\(.number)"' \
+             >> "${GITHUB_OUTPUT}"
+      - name: Find Comment
+        uses: peter-evans/find-comment@v4
+        id: find-comment
+        with:
+          issue-number: ${{ steps.pr-context.outputs.number }}
+          comment-author: 'github-actions[bot]'
+          body-includes: "<!-- pr-comment -->"
+
+      # If 'pr' workflow run was 'requested'...
+      - if: github.event.action == 'requested'
+        name: PR workflow started - Update Comment
+        uses: peter-evans/create-or-update-comment@v5
+        with:
+          issue-number: ${{ steps.pr-context.outputs.number }}
+          comment-id: ${{ steps.find-comment.outputs.comment-id }}
+          edit-mode: replace
+          body: |
+            <!-- pr-comment -->
+            Waiting for [build job](https://github.com/${{github.repository}}/actions/runs/${{github.event.workflow_run.id}}) (at ${{ github.event.workflow_run.head_sha }})...
+
+      # If 'pr' workflow run was 'completed'...
+      - if: github.event.action == 'completed'
+        name: PR workflow completed - Download pr-comment-body.txt
+        uses: actions/download-artifact@v4
+        continue-on-error: true
+        with:
+          github-token: ${{ github.token }}
+          repository: ${{ github.event.workflow_run.repository.full_name }}
+          run-id: ${{ github.event.workflow_run.id }}
+          name: pr-comment-body.txt
+      - if: github.event.action == 'completed'
+        name: PR workflow completed - Create pr-comment.txt
+        env:
+          REPO: ${{github.repository}}
+          RUN_ID: ${{github.event.workflow_run.id}}
+          RUN_HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
+        run: |
+          set -eu
+
+          (
+            cat << EOF
+          <!-- pr-comment -->
+          Results for [build job](https://github.com/${REPO}/actions/runs/${RUN_ID}) (at ${RUN_HEAD_SHA}):
+
+          EOF
+
+            if [ -f pr-comment-body.txt ]; then
+              cat pr-comment-body.txt
+            else
+              echo '(No pr-comment-body.txt artifact found from this job.)'
+            fi
+          ) > pr-comment.txt
+      - if: github.event.action == 'completed'
+        name: PR workflow completed - Update Comment
+        uses: peter-evans/create-or-update-comment@v5
+        with:
+          issue-number: ${{ steps.pr-context.outputs.number }}
+          comment-id: ${{ steps.find-comment.outputs.comment-id }}
+          edit-mode: replace
+          body-path: pr-comment.txt

.github/workflows/pr.yml

@@ -1,4 +1,4 @@
-name: Pull Request CI
+name: pr
 
 on:
   pull_request:
@@ -12,8 +12,10 @@ jobs:
   build:
     runs-on: ubuntu-latest
     timeout-minutes: 15
+    permissions:
+      pull-requests: write
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v5
         with:
           persist-credentials: false
       - uses: actions/setup-node@v3
@@ -32,3 +34,36 @@ jobs:
       - name: test wpt lint
         run: ./wpt lint
         working-directory: ./wpt
+      - name: compute case count after PR
+        run: |
+          tools/validate --print-case-count-report src/webgpu > case-count-report-after.txt
+      - name: checkout before PR
+        env:
+          PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
+        run: |
+          git fetch origin "${PR_BASE_REF}"
+          git checkout "${PR_BASE_REF}"
+      - name: compute case count before PR and diff
+        id: case_count_diff
+        run: |
+          set -eu
+
+          tools/validate --print-case-count-report src/webgpu > case-count-report-before.txt
+          # Diff only showing the changed lines and nothing else
+          diff --unified=0 case-count-report-{before,after}.txt | grep '^[+-][^+-]' | tee case-count-report-diff.txt
+          (
+            echo '```diff'
+            if [[ "$(wc -l < case-count-report-diff.txt)" -le 20 ]] ; then
+              cat case-count-report-diff.txt
+            else
+              head -n18 case-count-report-diff.txt
+              echo '[snip - full report in action logs]'
+              tail -n2 case-count-report-diff.txt
+            fi
+            echo '```'
+          ) > pr-comment-body.txt
+      - name: Upload pr-comment-body.txt
+        uses: actions/upload-artifact@v4
+        with:
+          name: pr-comment-body.txt
+          path: pr-comment-body.txt

.github/workflows/push.yml

@@ -1,4 +1,4 @@
-name: Push CI
+name: push
 
 on:
   workflow_dispatch:

docs/organization.md

@@ -69,8 +69,45 @@ an aspect of the API is tested fully. Examples:
 - [`webgpu:api,validation,render_pass,resolve:resolve_attachment:*`](https://github.com/gpuweb/cts/blob/ded3b7c8a4680a1a01621a8ac859facefadf32d0/src/webgpu/api/validation/render_pass/resolve.spec.ts#L35)
 - [`webgpu:api,validation,createBindGroupLayout:bindingTypeSpecific_optional_members:*`](https://github.com/gpuweb/cts/blob/ded3b7c8a4680a1a01621a8ac859facefadf32d0/src/webgpu/api/validation/createBindGroupLayout.spec.ts#L68)
 
-Use your own discretion when deciding the balance between heavily parameterizing
-a test and writing multiple separate tests.
+Combinatorial explosions make it easy to over-parameterize tests. Be intentional about the
+combinations of parameters being tested. For example, for validation tests, avoid (sub)cases
+that trigger multiple validation errors at once. In general, avoid combining over parameters
+that shouldn't have any significant interaction in an implementation.
+
+- Instead of:
+    ```ts
+    u
+      // valid and invalid values for x/y
+      .combine('x', [0, 1, 4, 5, 100])
+      .combine('y', [0, 1, 4, 5, 100])
+    ```
+- Try:
+    ```ts
+    u.combineWithParams([
+      // control case
+      { x: 4, y: 4 },
+      // invalid values of x
+      { x: 0, y: 4 },
+      { x: 5, y: 4 },
+      // invalid values of y
+      { x: 4, y: 0 },
+      { x: 4, y: 5 },
+    ])
+    ```
+- Or even:
+    ```ts
+    u.combineWithParams([
+      // control case
+      { x: 4, y: 4 },
+      // invalid values of x
+      ...u.combine('y', [4]).combine('x', [0, 5])
+      // invalid values of y
+      ...u.combine('x', [4]).combine('y', [0, 5])
+    ])
+    ```
+
+Use your own discretion when deciding to heavily parameterize a test, use a
+more targeted parameterization, or write multiple separate tests.
 
 #### Guidelines
 

src/common/internal/test_group.ts

@@ -70,8 +70,6 @@ export function makeTestGroup<F extends Fixture>(fixture: FixtureClass<F>): Test
 export interface IterableTestGroup {
   iterate(): Iterable<IterableTest>;
   validate(fileQuery: TestQueryMultiFile): void;
-  /** Returns the file-relative test paths of tests which have >0 cases. */
-  collectNonEmptyTests(): { testPath: string[] }[];
 }
 export interface IterableTest {
   testPath: string[];

src/common/tools/crawl.ts

@@ -47,12 +47,19 @@ async function crawlFilesRecursively(dir: string): Promise<string[]> {
 
 export async function crawl(
   suiteDir: string,
-  opts: { validate: boolean; printMetadataWarnings: boolean } | null = null
+  opts: {
+    validate: boolean;
+    printMetadataWarnings: boolean;
+    printCaseCountReport: boolean;
+  } | null = null
 ): Promise<TestSuiteListingEntry[]> {
   if (!fs.existsSync(suiteDir)) {
     throw new Error(`Could not find suite: ${suiteDir}`);
   }
 
+  let totalCases = 0;
+  let totalSubcases = 0;
+
   let validateTimingsEntries;
   if (opts?.validate) {
     const metadata = loadMetadataForSuite(suiteDir);
@@ -88,10 +95,34 @@ export async function crawl(
 
         mod.g.validate(new TestQueryMultiFile(suite, pathSegments));
 
-        for (const { testPath } of mod.g.collectNonEmptyTests()) {
-          const testQuery = new TestQueryMultiCase(suite, pathSegments, testPath, {}).toString();
-          if (validateTimingsEntries) {
-            validateTimingsEntries.testsFoundInFiles.add(testQuery);
+        if (opts?.printCaseCountReport || validateTimingsEntries) {
+          for (const t of mod.g.iterate()) {
+            const testQuery = new TestQueryMultiCase(
+              suite,
+              pathSegments,
+              t.testPath,
+              {}
+            ).toString();
+
+            let cases = 0;
+            let subcases = 0;
+            for (const c of t.iterate(null)) {
+              cases++;
+              if (opts?.printCaseCountReport) {
+                subcases += c.computeSubcaseCount();
+              }
+            }
+
+            if (opts?.printCaseCountReport) {
+              const perCase = (subcases / cases).toFixed(0);
+              console.log(`${testQuery} - ${cases} cases, ${subcases} subcases (~${perCase}/case)`);
+              totalCases += cases;
+              totalSubcases += subcases;
+            }
+
+            if (validateTimingsEntries && cases > 0) {
+              validateTimingsEntries.testsFoundInFiles.add(testQuery);
+            }
           }
         }
       }
@@ -161,6 +192,9 @@ export async function crawl(
     }
   }
 
+  if (opts?.printCaseCountReport) {
+    console.log(`-----\nTOTAL: ${totalCases} cases, ${totalSubcases} subcases`);
+  }
   return entries;
 }
 

src/common/tools/validate.ts

@@ -18,7 +18,8 @@ Example:
 
 Options:
   --help                     Print this message and exit.
-  --print-metadata-warnings  Print non-fatal warnings about listing_meta.json files.
+  --print-metadata-warnings  Print non-fatal warnings about listing_meta.json files to stderr.
+  --print-case-count-report  Print the case/subcase counts of every test to stdout.
 `);
   process.exit(rc);
 }
@@ -32,12 +33,19 @@ if (args.indexOf('--help') !== -1) {
 }
 
 let printMetadataWarnings = false;
+let printCaseCountReport = false;
 const suiteDirs = [];
 for (const arg of args) {
-  if (arg === '--print-metadata-warnings') {
-    printMetadataWarnings = true;
-  } else {
-    suiteDirs.push(arg);
+  switch (arg) {
+    case '--print-metadata-warnings':
+      printMetadataWarnings = true;
+      break;
+    case '--print-case-count-report':
+      printCaseCountReport = true;
+      break;
+    default:
+      suiteDirs.push(arg);
+      break;
   }
 }
 
@@ -49,5 +57,6 @@ for (const suiteDir of suiteDirs) {
   void crawl(suiteDir, {
     validate: true,
     printMetadataWarnings,
+    printCaseCountReport,
   });
 }