fix(#934): make the required E2E aggregate consult its own shards

.github/workflows/e2e-local.yml

@@ -763,7 +763,9 @@ jobs:
           on_baseline = os.environ.get("TESTED_SHA", "") == BASE_SHA
           if not on_baseline:
               print("")
-              print("Gate passed: 24/24 shards, 0 failures, above the floor.")
+              print("Artifact totals OK: 24/24 shards, 0 failures, above the floor.")
+              print("The shard matrix's OWN verdict is a separate step after this one")
+              print("— these totals cannot see a shard that failed after reporting (#934).")
               print("COUNT PARITY not evaluated — the cloud numbers describe")
               print(f"{BASE_SHA[:8]}; on any other commit the test set has moved and a")
               print("difference is uninterpretable. Re-run on that SHA to decide parity.")
@@ -775,3 +777,51 @@ jobs:
               sys.exit(1)
           print("PARITY: local matches the cloud baseline exactly.")
           EOF
+
+      # ── THE VERDICT THE SHARDS ALREADY REACHED (#934) ────────────────────
+      #
+      # Everything above reconstructs a verdict from uploaded artifacts. This asks the
+      # shards. They are different questions, and until now only the reconstructed one
+      # was asked — so this REQUIRED context concluded `success` on a run whose own
+      # matrix concluded `failure`, and PR #928 merged with three shards red.
+      #
+      # HOW THAT PASSED, from the aggregate's own log in run 32560171118:
+      #
+      #     shards reporting: 24/24
+      #       passed   2035   baseline 1807
+      #       failed      0
+      #
+      # `chromium/firefox/webkit gen 2/6` had each executed ZERO tests and failed on the
+      # zero-test detector, which was doing its job. Every condition above still passed:
+      # `Upload results` is `if: always()`, so a failing shard still uploads a valid
+      # results.json describing nothing; a shard with no tests has no FAILING tests; and
+      # Playwright redistributed the work, so the total went UP, clearing the floor.
+      #
+      # The blast radius is wider than that one detector. EVERY anti-vacuity guard in this
+      # lane runs `if: always()` after the tests, so none of their verdicts reach the
+      # artifacts — including check-zero-assertions.mjs (#861), whose whole subject is tests
+      # that pass while measuring nothing. Those are `expected` in results.json, so the
+      # totals count them as PASSED. The guards built to stop vacuous green were invisible
+      # to the check that decides whether green means anything.
+      #
+      # A STEP, NOT A JOB-LEVEL `if:`, for the reason every other condition here is a step:
+      # branch protection is never satisfied by a `skipped` job, so this must run and pass
+      # on a docs-only PR. Gated on `changes` so it does — when the matrix is skipped its
+      # result is `skipped`, which is correct there and must not fail.
+      #
+      # Deliberately LAST: the totals print first, so a future reader sees the numbers that
+      # looked fine and then the verdict that did not.
+      - name: The shards themselves must have passed
+        if: needs.changes.outputs.run == 'true'
+        run: |
+          result='${{ needs.e2e-local.result }}'
+          echo "shard matrix concluded: $result"
+          if [ "$result" != "success" ]; then
+            echo "::error::the E2E shard matrix concluded '$result', so this check fails."
+            echo "::error::The totals above may look clean — they are reconstructed from"
+            echo "::error::uploaded artifacts and cannot see a shard that failed AFTER"
+            echo "::error::writing a valid report (a zero-test shard, a zero-assertion"
+            echo "::error::verdict, any post-upload step). Open the red shard job; its own"
+            echo "::error::log names the cause. Do not relax this to make a run green."
+            exit 1
+          fi

scripts/__tests__/e2e-local-aggregate-consults-shards.test.js

@@ -0,0 +1,180 @@
+/**
+ * The required aggregate must consult the shard matrix's own verdict (#934).
+ *
+ * WHAT HAPPENED. `E2E (local) result` concluded `success` on run 32560171118, whose shard
+ * matrix concluded `failure` — `chromium/firefox/webkit gen 2/6` had each executed zero
+ * tests. Branch protection reads the named context, not the run, so PR #928 merged with
+ * three shards red.
+ *
+ * The aggregate reconstructed a verdict from uploaded artifacts and never asked the shards.
+ * Every condition it did check passed: `Upload results` is `if: always()`, so a failing
+ * shard still uploads a valid results.json describing nothing; a shard with no tests has no
+ * failing tests; and Playwright redistributed the work, so the total went UP and cleared
+ * the floor.
+ *
+ * WHY A TEST AND NOT JUST THE FIX. The fix is one `if:` and one `exit 1`. Deleting it turns
+ * the required check back into one that cannot fail, and nothing goes red to say so — the
+ * defect's entire signature is that everything looks green. That is #396, and it is exactly
+ * the class this repo keeps re-deriving.
+ *
+ * COMMENTS ARE STRIPPED BEFORE MATCHING. The workflow now carries a long comment block
+ * explaining this defect, and that prose names the very symbols asserted below. A guard that
+ * matched raw text would pass with the guarded code deleted — which has happened here four
+ * times, twice to guards written to catch this class.
+ */
+
+'use strict';
+
+const { describe, it } = require('node:test');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const WF = path.resolve(
+  __dirname,
+  '..',
+  '..',
+  '.github',
+  'workflows',
+  'e2e-local.yml'
+);
+const RAW = fs.readFileSync(WF, 'utf8');
+
+/** Whole-line YAML comments removed. Trailing `#` inside a value is not one. */
+function stripComments(text) {
+  return text
+    .split('\n')
+    .filter((l) => !/^\s*#/.test(l))
+    .join('\n');
+}
+
+/** The `parity:` job block, comments removed. */
+function parityBlock(text = RAW) {
+  const start = text.indexOf('\n  parity:');
+  if (start === -1) return '';
+  return stripComments(text.slice(start));
+}
+
+/** Does this text consult the shard matrix's conclusion? Both dot and bracket forms. */
+function consultsShards(text) {
+  return (
+    /needs\.e2e-local\.result/.test(text) ||
+    /needs\[\s*['"]e2e-local['"]\s*\]\.result/.test(text)
+  );
+}
+
+describe('the required aggregate consults its shards (#934)', () => {
+  it('the parser found the job it is asserting about', () => {
+    // Anti-vacuity, first. A renamed job or a moved file makes every assertion below
+    // pass by inspecting an empty string — the same shape as the defect itself.
+    assert.ok(RAW.length > 2000, `${WF} is suspiciously small — stale path?`);
+    const block = parityBlock();
+    assert.ok(block.length > 500, 'parity: job not found, or it is empty');
+    assert.match(
+      block,
+      /name: E2E \(local\) result/,
+      'the required context name is gone from the parity job — if it was renamed, branch ' +
+        'protection now waits on a context nobody reports'
+    );
+  });
+
+  it('comment stripping actually removes comments', () => {
+    // The control for the control. If stripComments() silently stopped working, the
+    // assertion below would go back to matching the prose that explains the fix.
+    const stripped = stripComments(RAW);
+    assert.ok(
+      stripped.length < RAW.length - 2000,
+      'stripComments removed almost nothing — this file is heavily commented, so the ' +
+        'stripper is broken and the assertions below may be matching prose'
+    );
+    assert.doesNotMatch(
+      stripped,
+      /WHY THIS EXISTS/,
+      'a known comment survived stripping'
+    );
+  });
+
+  it('the aggregate reads the shard matrix result, in CODE not prose', () => {
+    assert.ok(
+      consultsShards(parityBlock()),
+      'the parity job no longer reads `needs.e2e-local.result`. It is then judging the ' +
+        'run purely by uploaded artifacts, which cannot see a shard that failed AFTER ' +
+        'writing a valid report — a zero-test shard, a zero-assertion verdict (#861), or ' +
+        'any post-upload step. That is how #928 merged with three shards red.'
+    );
+  });
+
+  it('and can actually fail on it', () => {
+    // Reading the value and printing it would satisfy the assertion above while gating
+    // nothing. The job must exit non-zero.
+    const block = parityBlock();
+    const start = block.search(/needs\.e2e-local\.result|needs\[/);
+    const region = block.slice(start, start + 1200);
+    assert.match(
+      region,
+      /exit 1/,
+      'the shard result is read but nothing exits non-zero on it — the check reports the ' +
+        'failure and passes anyway'
+    );
+    assert.match(
+      region,
+      /!=\s*["']?success/,
+      'the comparison is not against `success`. Any other conclusion — failure, ' +
+        'cancelled, timed_out — must fail this check.'
+    );
+  });
+
+  it('the step is gated on `changes`, so a docs-only PR still passes', () => {
+    // The counterweight. When the matrix is skipped its result is `skipped`, which is
+    // correct there. An ungated assertion would block every docs-only PR — and a required
+    // check that fails on documentation is how this lane loses its unfiltered trigger.
+    const block = parityBlock();
+    const stepStart = block.indexOf(
+      '- name: The shards themselves must have passed'
+    );
+    assert.notStrictEqual(
+      stepStart,
+      -1,
+      'the shard-verdict step was renamed; update this test deliberately, not reflexively'
+    );
+    const step = block.slice(stepStart, stepStart + 400);
+    assert.match(
+      step,
+      /if:\s*needs\.changes\.outputs\.run == 'true'/,
+      'the shard-verdict step is not gated on the changes job'
+    );
+  });
+
+  it('the job itself is still ungated and still `if: always()`', () => {
+    // Branch protection is never satisfied by a `skipped` job. Moving the gate from the
+    // steps up to the job would make the required check go PENDING FOREVER on docs-only
+    // PRs — passing locally, blocking merges, with nothing red to explain it.
+    const block = parityBlock();
+    const head = block.slice(0, block.indexOf('steps:'));
+    assert.match(
+      head,
+      /if:\s*always\(\)/,
+      'parity is no longer `if: always()`'
+    );
+    assert.match(
+      head,
+      /needs:\s*\[changes,\s*e2e-local\]/,
+      'parity no longer depends on both `changes` and `e2e-local` — it cannot read a ' +
+        'result it does not depend on'
+    );
+  });
+
+  it('CONTROL: the matcher reports absence when the call is removed', () => {
+    // Without this, an always-true matcher would satisfy every assertion above. This is
+    // the mutation the reviewer cannot perform by reading.
+    const mutated = parityBlock().replace(
+      /\$\{\{\s*needs\.e2e-local\.result\s*\}\}/g,
+      "'success'"
+    );
+    assert.ok(
+      !consultsShards(mutated),
+      'the matcher still reports the call present after it was removed — it is matching ' +
+        'something other than the code it claims to check'
+    );
+  });
+});

tests/e2e/color-contrast.spec.ts

@@ -233,6 +233,20 @@ console.log(
       .join('\n')
 );
 
+// SHARDABLE, NOT ATOMIC (#915).
+//
+// CI sets `fullyParallel: false` (playwright.config.ts:78) so that specs sharing the
+// PRIMARY/TERTIARY test users run serially. Under that setting Playwright shards by FILE:
+// a file is an indivisible unit, and this one is 142 of chromium-gen's 668 tests. Adding a
+// third house theme pushed it over a balancing threshold and left `gen 2/6` with ZERO tests
+// — measured: shard 2 went 39 -> 0, and the zero-assertion gate (#861) correctly refused to
+// call that green.
+//
+// This sweep authenticates nothing and shares no fixture user, so it is safe to schedule per
+// test. `mode: 'parallel'` makes its cases individually distributable across shards without
+// touching `workers`, which stays 1 in CI for the specs that genuinely need it.
+test.describe.configure({ mode: 'parallel' });
+
 test.describe('WCAG AAA color-contrast-enhanced (violations only)', () => {
   // Match pa11yci.json viewport.
   test.use({ viewport: { width: 1280, height: 1024 } });