Repository navigation
test(style): fixture self-tests for the reduced-motion scroll-behavior rule - #283
Conversation
…r rule Extract the rule #281 added inline into unguardedSmoothScroll(css, label), matching the exported-helper-plus-fixture shape of every other rule in style-contract.test.mjs. The helper now also catches smooth !important, a block-final declaration without a trailing semicolon and upper-case values, and runs over every page's inline <style> as the sibling rules do. Closes #282 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
One verified false negative in the new rule; everything else checks out. Ran node --test scripts/style-contract.test.mjs at 51d4954: 61/61 pass, and the live style.css:93 guard is correctly accepted.
Finding (correctness, low — test-only, live CSS unaffected)
scripts/style-contract.test.mjs:439-447 — when scroll-behavior: is matched inside an @supports prelude, the value scan runs forward to the first ; or } and so passes straight through the { tokens; i = end - 1 then skips them, and the preludes stack never records those blocks. Consequence: an unguarded declaration nested under a feature query is never examined.
unguardedSmoothScroll(`@supports (scroll-behavior: smooth) { html { scroll-behavior: smooth; } }`)
// → [] (should be one problem at line 1)The fixture at :550 (@media (…no-preference) { @supports (scroll-behavior: smooth) { … } }) passes for the wrong reason — the inner declaration is skipped, not judged guarded — so the self-test does not catch this.
Fix (one token): stop the value scan at { as well, so a prelude match yields a non-smooth value and the following { is tracked normally:
while (end < clean.length && clean[end] !== ";" && clean[end] !== "}" && clean[end] !== "{") end++;Verified locally: with that change the case above reports css line 1: …, the guarded-@supports fixture still passes, and the file stays 61/61. Worth adding the unguarded-@supports shape to the rejection fixture so it stays caught.
Nothing else to report: stripComments/lineOf/inlineStyles/stylesheets/pages all exist at module scope, the per-stylesheet and per-inline-<style> loops mirror the structure/var() rules above them, and the three loopholes the body names (!important, block-final without ;, SMOOTH) are genuinely closed.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
…ment
- Move unguardedSmoothScroll above the fixture self-tests banner so it
sits with the other rule helpers.
- Skip string contents while scanning so a "{" or ";" inside one cannot
leave a guarding @media prelude on the stack.
- Stop the value scan at "{" so a prelude such as
@supports (scroll-behavior: smooth) still records its block and the
nested declarations are examined.
- Assert the clean CSS fixture against unguardedSmoothScroll and add
fixtures for both parser cases.
Hive-Fix-Attempt: 1/6
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Read the diff against the tree at f610ebf and ran it; this looks correct to me.
What I verified:
unguardedSmoothScroll(scripts/style-contract.test.mjs:390-422) tracks preludes on a brace stack reset on;/}, so a@supports (scroll-behavior: smooth) {prelude is not mis-read as a declaration (valuesmooth)≠smooth, then the{pushes the full prelude) and[data-scroll-behavior=…]is excluded by the[\w-]look-behind. String skipping mirrorscssStructureProblems.- Helpers it relies on exist:
stripComments(line 63),inlineStyles(line 40),lineOffromscripts/html-pages.mjs:28,stylesheets/pages(lines 672-684). node --test scripts/*.test.mjs→ 785/785 pass at head, as the body states.- Mutation: unwrapping the guard at
style.css:93makes the live rule fail withstyle.css line 93: …, so the extraction kept its teeth (the old inline version would also have missedsmooth !important,SMOOTH, and a block-final declaration without;— the new fixtures cover all three).
Test-only change, no production files touched; scope matches #282.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Looks correct to me. Verified at f610ebf against a fresh clone:
node --test scripts/*.test.mjs→ 785/785 pass.unguardedSmoothScrollon the livestyle.cssreturns[]; with the #281 guard removed it reportsstyle.css line 93(the body says 92 — the line shifted on main, not a defect).blank()(style-contract.test.mjs:25) keeps newlines, solineOf(clean, i)on the comment-stripped text is accurate.- The string skip (
:396-399) mirrorscssStructureProblemsand thecontent: "{"fixtures prove it keeps the prelude stack honest. - Diff matches #282's recommendation 1:1; no production files touched.
One low note, not blocking: the guard check (:412) treats any @media prelude containing prefers-reduced-motion: no-preference as guarding, so @media not (prefers-reduced-motion: no-preference) { html { scroll-behavior: smooth; } } passes silently (confirmed by calling the helper). Nothing in the tree writes that today; a follow-up fixture rejecting a not-prefixed query would close it.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
Test Improvement
#281 added the
scroll-behavior: smooth→@media (prefers-reduced-motion: no-preference)rule as an inline loop inside the livestyle.csstest. This PR brings it in line with the other 18 rules inscripts/style-contract.test.mjs:export function unguardedSmoothScroll(css, label = "css"), returning one problem per unguarded declaration with its line<style>(15 pages), like the structure /var()/url()rules@supports,auto, vendor-prefixed, commented-out, empty) and each rejection arm (unguarded,reduceguard, wrong media feature,@supportsinstead of@media,!important, upper-case, block-final without;, mixed guarded/unguarded with line numbers)smooth !important, a last-in-block declaration without a trailing;, andSMOOTHall passed the inline version silentlyVerification at
b649779:node --test scripts/*.test.mjs→ 785/785 pass (was 769); reverting the #281style.csschange still fails the live rule at line 92; thecoverage-floorgate stays green. No production files touched.Closes #282
Filed by quality agent (ACMM L4/L6 — full mode)
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88