Skip to content

test(e2e): credit coverage for runs that span a source boundary - #1240

Merged
mrbobbytables merged 1 commit into
mainfrom
quality/e2e-source-boundary-attribution
Oct 9, 2026
Merged

mrbobbytables merged 1 commit into
mainfrom
quality/e2e-source-boundary-attribution

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Test Improvement

tests/tools/e2e-coverage-report.mjs — convertScript / the new
sourceBoundaryOffsets + splitRangeAtBoundaries helpers — plus a new
tests/e2e-coverage-source-boundary.test.mjs. No other file is touched.

The defect

normalizeRanges rewrites a V8 record into the coarsest partition it can: one
range per run of equal counts, however far that run reaches. In a bundle that
carries several src/** modules such a run routinely starts in one module and
ends in another.

v8-to-istanbul cannot place that range. offsetToOriginalRelative
(v8-to-istanbul lib/source.js) maps the start and the end back through the
source map and gives up when they disagree:

if (start.source !== end.source) {
  return {}
}

applyCoverage then returns before recording a branch, so neither module is
credited. convertScript builds attributedPaths from exactly the branches and
functions that were recorded, so the multi-source guard in collectE2ECoverage

if (converted.sourceFiles.length > 1 && !converted.attributedPaths.has(path)) continue;

goes on to skip every source of that script. Coverage a browser genuinely
executed never reaches the region tally — which is the shape #1236 describes.

The fix

Split each normalized range at the generated offsets where the source map
changes source, so every sub-range lies inside one original file. Each
non-final sub-range ends one offset short of the cut, because the first offset
at a boundary already belongs to the next source and would re-trigger the
start.source !== end.source bail; that offset is not lost, it opens the
following sub-range. A 1:1 bundle has no boundaries, so no range is split and
single-source scripts are byte-identical to before.

Evidence

Replayed deterministically against the e2e-coverage artifact 11609771627
(run 37914482229-1, head b00ff4e) named in #1236 — the artifact carries its
own scripts/ copies, so no browser is needed:

regions src region % uncovered regions
b00ff4e as-is 447/466 95.92 19
with this diff 649/665 97.59 16

Line coverage is unchanged at 100.00% (2099/2099). 199 regions that the report
could not see before are now measured. Concretely,
src/components/MemberDirectory/DirectoryFreshness.js drops lines 30, 44 and 49
from uncoveredRegions (19 30 44 49 → 19), and
src/components/MemberDirectory/utils.js becomes measurable for the first time.

npm run test:unit: 2089/2089 pass. The three new tests are red without the
tests/tools/ change (2 fail) and green with it; the single-source test passes
both ways, which is the point of including it.

What this does not close

  • src/components/CommunityPeople/index.js line 134 — the specific instrumented
    case [quality] coverage executed on e2e fixture pages is not credited: 19 src regions stay at zero across four runs #1236 opens with — is still reported uncovered after this change. The
    boundary defect is real and is the bulk of the loss, but it is not the whole
    of that one symptom.
  • Step 4 of the issue (ratcheting --check-source-regions past 95 in
    .github/workflows/ci.yml) is not here. A contributor-tier token cannot push
    a diff touching .github/workflows/**; GitHub rejects it server-side. With
    this change the measured number is 97.59, so the threshold has room to move.

Related Issue

Refs #1236 (needs-human: src/components/CommunityPeople/index.js:134 is still uncovered after this fix, and ratcheting --check-source-regions in .github/workflows/ci.yml needs workflow-write permission this agent does not have)

Overlap with open held PRs

tests/tools/e2e-coverage-report.mjs is also touched by #1233 (parseArgs,
main) and #1211 (the constants block near the top). This diff is confined to
convertScript and two new helpers directly above it, so it shares no hunk with
either.


Filed by quality agent (hold-gated mode). Human review required.

— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88

normalizeRanges rewrites a V8 record into the coarsest partition it can:
one range per run of equal counts, however far that run reaches. In a
bundle carrying several src/** modules such a run routinely starts in one
module and ends in another, and v8-to-istanbul abandons it outright --
offsetToOriginalRelative returns {} when a range's start and end map to
different originals, so applyCoverage records no branch for either module.
convertScript builds attributedPaths from exactly those branches, so the
multi-source guard in collectE2ECoverage then skips every source of the
script and coverage a browser genuinely executed is dropped.

Split each normalized range at the generated offsets where the source map
changes source, ending each sub-range one offset short of the cut so its
end position stays on the source side of the boundary. A 1:1 bundle has no
boundaries and is left byte-identical.

Replayed against the e2e-coverage artifact of run 37914482229 at b00ff4e:
src regions move from 447/466 (95.92%) to 649/665 (97.59%), uncovered
regions fall from 19 to 16, src/components/MemberDirectory/DirectoryFreshness.js
drops lines 30, 44 and 49, and src/components/MemberDirectory/utils.js
becomes measurable for the first time. Line coverage is unchanged at 100%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: quality <quality@hive.kubestellar.io>
@hivecommons-hive hivecommons-hive Bot added the hold label Oct 9, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor Author

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. Non-outreach agents are held at ACMM L3–L5; the outreach agent is always held because it publishes project-facing communication.

Hive will keep the hold label until a human removes it. Operators can make a deliberate one-off release during an ACMM level change with release_level_holds=true, but level changes never release this hold automatically.

@mrbobbytables

Copy link
Copy Markdown
Member

Heads-up: PRs1211 1233 also edit tests/tools/e2e-coverage-report.mjs. Expect merge conflicts as each lands; please merge in sequence (rebase after each) or consolidate.

@mrbobbytables
mrbobbytables added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit b469a1c Oct 9, 2026
7 checks passed
mrbobbytables added a commit to mrbobbytables/endusers that referenced this pull request Oct 11, 2026
…ide the harness floor (cncf#1287)

Resolves cncf#1253 by the same route cncf#1211 took for cncf#1210: document the
unreachable fallbacks rather than delete them or add an ignore marker.

tests/tools/e2e-coverage-report.mjs sits at 97.98% regions and every one
of its 10 uncovered regions is a defensive fallback no test can reach.
The in-file note already listed eight of them; this adds the two it
predates (the second fallback on the resolvedSources chain, and the
parts.length === 0 arm in splitRangeAtBoundaries from cncf#1240), states the
resulting ceiling, and says how the list and the floor move together.

tests/coverage-gate-thresholds.test.mjs records that ceiling next to
--check-harness-file-regions so the floor cncf#1250 raised to 97 is never
set above what the file can measure, and refreshes the measured numbers
to the suite this commit ships.

Signed-off-by: Bob Killen <bkillen@linuxfoundation.org>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant