Repository navigation
test(Timer): complete hardening audit layers 1-2 (#6483) - #6609
ManoharPaturi wants to merge 1 commit into
Conversation
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsTimer (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual Regression25 of 420 shot(s) changed. View the report A change here is a question, not a failure: check whether the after is the
and 5 more. Chat — Full AI Chat — probe light
Chat — Empty State — probe light
Chat — Full AI Chat — probe darkGenerated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for a careful hardening pass — the two newly pinned guards are exactly the kind of invisible coverage this audit exists for, and I verified the Timer suite passes (19 tests) with the timeout-ceiling and invalid-format tests in place. No production code or API changes, which matches the audit result.
One thing does need fixing before this can land: the Default story's new play function is described — in the story comment, this PR description, and the changeset — as failing in a real browser if the display ever freezes at 0s. In this repo, a play assertion only fails CI when its story is listed in TARGETS in .github/scripts/story-play-guard.js, and that list currently contains only ChartTooltip and TabList stories. The guard's own header notes that an unlisted play is observed by nothing that can fail, so a frozen Timer would currently keep every required check green despite those claims. Adding core-timer--default to that TARGETS list (with a short guards description) is the documented one-step promotion and would make the claimed protection real — the guard's timeout comfortably covers the play's wait.
Separately, a small heads-up: the description's Fixes #6483 will auto-close that issue on merge while Layer 3 and the weight observability decision are routed to the owner — worth making sure those have follow-up tracking before merging.
[Automated review]
Fixes #6483
What this does
Completes the contributor-executable layers (1–2) of the Component Hardening Protocol for Timer. (The issue title says XDSTimer; the shipped component is Timer, owner-approved 2026-09-22 — reviewed against
Timer.spec.md.)Audit result
The automated surface is clean:
check:repocontract checks,themingTargets.test.ts(theastryx-timertarget is registered and asserted),check:knowledge(spec in sync), eslint with the repo's own plugin, and the doc props table matched against the implementation line by line. No dead CSS, no dropped props, no stale xds naming, no live-region or anatomy violations — the "noaria-live/roleby default" contract is explicitly tested and rejected additions are honored.The tick pipeline itself held up under review: boundary wakes (second/minute cadence switch at the hour), future-origin clamping, non-finite
startTimefallback, format switching without remount, and StrictMode resource replay are all covered by tests that can fail.What changed
Two untested guards are now pinned (both were implemented but invisible to CI):
startTimecaps its scheduled wait at the 32-bitsetTimeoutceiling (MAX_TIMEOUT_MS). A rawstartTime - nowdelay overflows it and misfires in some browsers; the test proves the cap.formatvalue coerces toelapsed(resolveFormat), and switching back toclockstill works.The Default story gained a play function asserting in a real browser that the zero-duration first paint advances on its own — the core promise of the component, previously only proven in jsdom.
Routed, not built (for the owner / Layer 3)
weightis verified only as render-without-crash acrossText,Timestamp, andTimer— it compiles to a StyleX class with nodata-weightattribute, so jsdom can't observe it. Adata-weightattribute (or a documented exception) is an owner decision.Testing
pnpm vitest run packages/core/src/Timer/Timer.test.tsx: 19 passed (17 pre-existing unchanged).#6306/#6314 was the template for this pass.