Repository navigation
test: require every e2e fixture build to be reached by a spec - #1636
Conversation
The fixture-build list is a directory listing: every tests/e2e/fixtures/data-<name>/ directory declares one more full Docusaurus compile in the end-to-end coverage job. Nothing asserted the reverse direction -- that each declared build is actually navigated by a spec -- so an orphaned directory is a compile paid on every pull request that serves a site nobody opens. tests/e2e-data-fixtures.test.mjs guards directory -> build and pins the current names with includes(), both of which keep passing after a build's last spec is deleted. tests/e2e-coverage-builds.test.mjs only checks that the compiler loops over coverageBuildNames(). Playwright reports no failure for an unvisited build, only a slower job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Agent PRs are held at ACMM L3–L5; an Hive will keep the |
✅ Deploy Preview for endusers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Priority: P3. This adds an E2E harness reachability guard; no current user-facing failure is reported. |
mrbobbytables
left a comment
There was a problem hiding this comment.
The fixture-to-spec guard only scans immediate children of tests/e2e via readdirSync(specDir). Playwright collects nested specs recursively, so a valid nested spec could be the only consumer of a fixture build while this test reports it as orphaned. This is the same non-recursive spec-discovery pitfall corrected in #1585/#1586. Please use the repository’s recursive spec discovery pattern and add a nested-spec fixture case before merging.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com>
|
Updated this branch in 332ebbb: spec discovery now uses recursive |
mrbobbytables
left a comment
There was a problem hiding this comment.
The recursive discovery fix and nested-spec regression test address my request. The updated branch passes the focused test, duplication gate, and required CI checks. I am leaving the existing hold label in place; this is not a request to merge.
Test Improvement
Adds
tests/e2e-coverage-build-reach.test.mjs, which requires every e2ecoverage fixture build to be reached by at least one spec.
The fixture-build list is a directory listing, by design: every
tests/e2e/fixtures/data-<name>/directory declares one more build, andnpm run build:e2e:coveragecompiles it intobuild/e2e-coverage-<name>.The module header in
tests/tools/e2e-data-fixtures.cjsstates the cost in thesame breath:
Nothing asserted the other half of that contract.
tests/e2e-data-fixtures.test.mjsguards the directory → builddirection. Its comment names the failure it watches for — "a spec asserting
against a route that 404s" — which is a spec without a build, not a build
without a spec. It then pins the current names with
names.includes('variant')/names.includes('no-revision'), which keeppassing after a build's last spec is deleted.
tests/e2e-coverage-builds.test.mjsonly checks that the compiler loops overcoverageBuildNames()and derives the right out-dir and base URL. It isindifferent to whether the resulting site is ever requested.
only a slower job.
A spec has to show two things to count — the base path
/e2e-coverage-<name>(anchored so
/e2e-coverage-variantis not credited by a hypothetical/e2e-coverage-variant-extra) andE2E_COVERAGE_BUILD: '<name>', the pairingevery existing variant spec already uses. Either alone is weak: a spec that
names the base path but never resolves the build's data is asserting against a
route it cannot describe; one that loads the data but never names the base path
never opens the site.
A second test pins that the ordinary coverage build is addressed by the site
root rather than a build name, so the loop is not later "fixed" by adding it to
the name list.
Both current builds (
variant,no-revision) pass today — this is aregression guard, not a report of a broken build.
Verification
TZ=UTC node --test tests/e2e-coverage-build-reach.test.mjs→ 2 passed.tests/e2e/fixtures/data-orphan/makes it fail withorphan: no spec navigates /e2e-coverage-orphan; the build is a full Docusaurus compile on every run that covers nothing.npm run test:unit:coverage:check→ exit 0; the new file is itself100.00% lines / 100.00% regions, and
src filesstays 100.00/100.00.npx prettier --check→ clean.Coverage evidence
This is a
test-infrastructurefinding, not a coverage gap:src/is at100.00% lines in both lanes.
TZ=UTC node tests/tools/coverage-report.mjsatmain3659352→src files 100.00% lines / 100.00% regions(9900/9900, 2873/2873).e2e-coverage(id11825781781) from CI run38418930699,
head
1e37cdc, 2026-10-13, sealedpassed, 204 captured scripts →src files 100.00% lines, 97.33% regions(656/674). All 18 residualuncovered regions are guard/fallback arms of the documented region-union
class (see the "What the union credits" note in
tests/e2e/data-variants.spec.jsand issue 1256 ([quality] the e2e region union never credits a guard arm taken only in a sibling fixture build #1256)), not unexercised code.The two lanes are reported separately rather than merged: they are different
instruments (Node coverage over build-time modules vs. V8 coverage over browser
bundles) with no compatible raw form. A fresh local e2e run at
3659352wasattempted to replace the 6-day-old artifact, but chromium could not launch in
the analysis environment (
libglib-2.0.so.0missing, no root).Related Issue
None — this PR carries the finding in full. Two attempts to file it as its own
issue were auto-consolidated by the issue relay into issue 1625 (#1625), which is a
different finding (the unexecuted-chunk arm at
tests/tools/e2e-coverage-report.mjsline 835, already claimed andstage/040-covered-by-pr). There is no file or behaviour overlap between thetwo, so this PR deliberately does not mark that issue resolved. If a maintainer
wants a tracking issue for this, it needs to be opened by hand.
Overlap check
Disjoint from every open hold-gated PR. PR 1626 and PR 1627 touch
tests/tools/e2e-coverage-report.mjs, PR 1627 alsoe2e-coverage-run.mjs, PR 1629e2e-coverage-global-setup.mjs; this PR adds one new test file and touches noexisting file. It reads
tests/tools/e2e-data-fixtures.cjsandtests/e2e/*.spec.js, which no open PR modifies.Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88