Repository navigation
test: extend branch coverage floor to inline page scripts - #277
kubestellar-prow[bot] merged 1 commit into
Conversation
|
@thatguywob this PR is back in your court ( Review asks:
Reply here or push a new head to this branch (same sign-off); CI and the review swarm re-run automatically and the PR returns to the normal path. Your relay session received this as a |
Attribute harness execution to stable inline URLs and report original page lines. Assert all 21 previously uncovered ranges and verify mapping with real child coverage. Hive-Run: hivecommons#275 Hive-Plan: issue-275-inline-coverage-floor Hive-Spec: issue-275#recommendation Signed-off-by: thatguywob <thatguywob@users.noreply.github.com>
There was a problem hiding this comment.
Decision needed: @clubanderson should decide whether #277 can be closed now that #276 (merged, closes #275) already carries the same change.
Conclusion: this duplicates #276, which merged to main and closed #275. Merging #277 is no longer possible as-is (mergeable: CONFLICTING), and a rebase would reproduce work that already landed. Flagging for a maintainer: if you agree, this one could be closed in favour of #276. I have not closed anything.
What I verified (both diffs read in full):
- Both create
scripts/inline-scripts.mjswith a byte-identicalHARNESSED_SCRIPTSregistry (hero / ACMM / projects inindex.html, story dialog instories/index.html) lifted out ofscripts/page-scripts.test.mjs. - Both tag each extracted block with
//# sourceURL=inline:<page>#<hook>so V8 attributes thenew Functionbody to a stable URL (#277scripts/inline-scripts.mjs:23; #276scripts/inline-scripts.mjs:34). - Both extend
scripts/coverage-floor.test.mjsto readinline:scripts, shift V8 offsets past the(function anonymous(…) {wrapper, and report the page's own line number (#277coverage-floor.test.mjs:130-146and thelinesmap inproblemsOf; #276inlineRunsFromReports/shiftRangesand{ src, line }entries inproblemsOf). - Both point the three harnesses (
carousels,acmm-levels,story-dialog) at the shared extractor, add apreparehook to eachboot*(), and assert the same 21 dead ranges the extended floor finds (hero 13, projects 3, ACMM 3, dialog 2). - Both update the same
README.md## Checksentry forcoverage-floor.
Where they differ (implementation detail, not intent): #277's inlineScript() returns a string and exposes inlineBlock() for the line; #276's returns { src, line }. #277 matches <script\b[^>]*> (any attributes); #276 matches bare <script> only. #277 adds scripts/inline-scripts.test.mjs; #276 asserts the throw arm inside page-scripts.test.mjs and adds a compiled-length sanity check. None of these are defects in #277 — they are just a second answer to the same question, and the first one is already on main.
Scope note: the PR body says "Fixes #275"; that issue is now closed via #276, so there is no remaining issue for this PR to resolve.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
572babc to
de585d6
Compare
|
Supersession sweep found that this PR's claimed issue is already closed by another merged PR.
Decision needed (@hivecommons): #276 already merged and closed the claimed issue, but this PR touches files the merged PR did not. Recommended: keep open until the extra changes are reviewed, or close as superseded if they are not wanted. Another option: merge anyway. Files the merged PR did not touch: This comment is edited in place by the supersession sweep; it is not duplicated. |
There was a problem hiding this comment.
Decision needed: @clubanderson should decide whether the test-only residual of #277 (three extra assertions over branches the floor already enforces, plus a README bullet) is worth landing now that #276 closed #275.
Conclusion: the code is correct and the suite is green, but after #276 merged this PR no longer does what its body says; what remains is additive test coverage over branches that are already executed on main. A maintainer needs to decide whether that residual is worth a merge.
What I verified at de585d6 (applied onto current main, node --test scripts/*.test.mjs → 768 pass, 0 fail):
- The headline claim — "share their harness registry and extraction helper, give each Function body a stable
inline:URL, include those ranges in the floor" — is already onmainfrom #276 (f408d4a). This diff touches no.mjshelper and no.html; it is 74 added lines acrossscripts/inline-scripts.test.mjs(new),scripts/coverage-floor.test.mjs:343-382,scripts/carousels.test.mjs:1002-1019and one README bullet (README.md:207-208). The body and "Fixes #275" describe #276's work, not this residual. scripts/coverage-floor.test.mjs:326("every branch of every scripts/*.mjs helper and every inline page script is executed") passes onmainwithout this PR, so thetry { target.focus({ preventScroll: true }) } catch { target.focus() }fallback atindex.html:811-812that the new carousel test exercises is already covered; the new test pins the retry order ([{preventScroll:true}, undefined]) andtabIndex === -1, which the floor alone does not assert. That is the only behavioural assertion here thatmainlacks.- The new fixture at
coverage-floor.test.mjs:343overlaps the existing one at:293(both spawn a real child run against a tmp page and assert the dead-branch line); the additions are the differing-parameter-list wrapper and theinline:unknown/node:fs/ unknown-file URL filtering at:377.scripts/inline-scripts.test.mjsdirectly unit-testsinlineScript/inlineUrl, whichmainonly covers indirectly through the floor.
Nothing here is wrong; nothing here is required. If it lands, the body should be rewritten to describe the residual and Fixes #275 dropped (the issue is already closed). I have not changed any labels or closed anything.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Needs a decision, not a merge:
Head: verdict requires_human @de585d6, 0/0 checks green, DCO yes, soak 0s of 30m0s. How to release it:
Clear the cause and the hive re-evaluates on its next pass; the stage moves to |
clubanderson
left a comment
There was a problem hiding this comment.
Land it: inline page scripts under the coverage floor is a real gain even though #276 closed #275; extra assertions are cheap. Thanks @thatguywob.
|
/hive decision land as-is; change body to |
|
LGTM label has been added. DetailsGit tree hash: 46a1cc131ce111464c5db124b7285c8da746dffd |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Needs a decision, not a merge:
Head: verdict requires_human @de585d6, 0/0 checks green, DCO yes, soak 0s of 30m0s. How to release it:
Clear the cause and the hive re-evaluates on its next pass; the stage moves to |
Summary
The coverage floor previously ignored the four inline scripts shipped to browsers. Share their harness registry and extraction helper, give each Function body a stable
inline:URL, and include those ranges in the existing floor with original page line numbers.Add behavioral assertions for all 21 reported gaps, including hero keyboard focus without a link and engines rejecting focus options. Real child-process fixtures verify dead-branch detection, differing Function parameter lists, page-line mapping, and coverage combined across processes. Update README checks; page scripts and workflow behavior are unchanged.
Fixes #275
Checks run locally
node --test scripts/*.test.mjs— 765 passedscripts/check-links.test.sh— 114 passedscripts/check-redirects.test.sh— 32 passedscripts/check-redirects.sh— 13 entries, zero errorsscripts/check-links.sh --external-warn— stopped the slow external-network probe before completiongit diff --checkCommits carry DCO sign-off and Hive provenance trailers. This implementation was generated with an AI coding agent.
— hive: backend=codex model=gpt-6.1-sol
🐝 Hive Agent:
contributor| SHA:572babc