Add comprehensive unit and integration test coverage - #568
Merged
Merged
Conversation
…n CSS Close the highest-value coverage gaps found in the repo-wide test audit. New unit tests (configurator/vitest): - colorUtils: OKLCH→sRGB, luminance, contrast ratio/rating, oklch parse/round-trip - variableScales: shape + exact var(--sf-*) wire format of every sibling scale - powerKnobs: range invariants (min≤default≤max, on-grid) + encode/decode round-trip - theme.svelte: persistence, toggle, forceTheme (no-persist), dark-class binding - domains: domainOf routing + misc fallback (locks the check:curation contract) New negative/writer tests (node --test) — prove the CI gates actually bite: - version-sync (writer): propagates version to every artifact; checker agrees; idempotent - check-llm-guide / check-macro-catalog / check-token-registry: each fails on its own drift class instead of silently going green New behavioural specs (Playwright): - forms.spec: input/textarea/select/:disabled + --sf-field-border-color indirection - motion.spec: prefers-reduced-motion gating + --sf-motion-scale duration scaling Enabling fix: version-sync.js and the three check-*.js gates now honour SLASHED_ROOT (mirroring check-version-sync.js) so they can run against throwaway fixture trees. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SETyJ7R12Bxeuux7yF6y7F
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds new Vitest unit tests for configurator library modules (colorUtils, domains, powerKnobs, theme, variableScales), introduces a ChangesConfigurator library unit tests
SLASHED_ROOT override and gate/writer test coverage
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoAdd unit/integration tests for configurator libs, CI gates, and CSS bundles
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
12 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Address Qodo review on #568: `process.env.SLASHED_ROOT ?? …` accepted an empty string or a relative path, which path.join would resolve against the process CWD instead of an absolute root (parse.js requires an absolute root). Treat empty/whitespace as unset and resolve any override to absolute in version-sync.js and the check-{llm-guide,macro-catalog,token-registry,version-sync}.js gates. Add a guard test asserting an empty SLASHED_ROOT falls back to the repo root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SETyJ7R12Bxeuux7yF6y7F
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/version-sync.js (1)
22-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSame trim/resolve mismatch — apply the fix here too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/version-sync.js` around lines 22 - 23, The ROOT initialization in version-sync.js has the same trim/resolve mismatch, so make the resolved path use the trimmed environment value consistently. Update the ROOT assignment near the top of the script to pass the trimmed SLASHED_ROOT value into path.resolve, matching the intended behavior used elsewhere and keeping the handling of ROOT consistent.scripts/check-version-sync.js (1)
22-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSame trim/resolve mismatch as
check-macro-catalog.js.The trimmed value is used for the truthiness check but the untrimmed value is passed to
path.resolve. Apply the same fix here and in the other scripts that share this pattern (check-llm-guide.js,check-token-registry.js,version-sync.js).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-version-sync.js` around lines 22 - 23, The ROOT environment parsing in check-version-sync.js has the same trim/resolve mismatch as the other validation scripts, where the trimmed value is only used for the truthiness check but the original untrimmed value is still passed into path.resolve. Update the ROOT assignment to resolve the already trimmed value, and apply the same pattern consistently in check-macro-catalog.js, check-llm-guide.js, check-token-registry.js, and version-sync.js so all scripts use the same sanitized SLASHED_ROOT handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-macro-catalog.js`:
- Around line 22-23: The ROOT initialization in check-macro-catalog.js is using
a trimmed truthiness check but still passes the untrimmed SLASHED_ROOT into
path.resolve, so make the value used by path.resolve be the trimmed env var
instead. Update the ROOT assignment so it consistently derives from the same
trimmed string, keeping the check and resolved path aligned and avoiding
whitespace-sensitive directory targeting.
---
Duplicate comments:
In `@scripts/check-version-sync.js`:
- Around line 22-23: The ROOT environment parsing in check-version-sync.js has
the same trim/resolve mismatch as the other validation scripts, where the
trimmed value is only used for the truthiness check but the original untrimmed
value is still passed into path.resolve. Update the ROOT assignment to resolve
the already trimmed value, and apply the same pattern consistently in
check-macro-catalog.js, check-llm-guide.js, check-token-registry.js, and
version-sync.js so all scripts use the same sanitized SLASHED_ROOT handling.
In `@scripts/version-sync.js`:
- Around line 22-23: The ROOT initialization in version-sync.js has the same
trim/resolve mismatch, so make the resolved path use the trimmed environment
value consistently. Update the ROOT assignment near the top of the script to
pass the trimmed SLASHED_ROOT value into path.resolve, matching the intended
behavior used elsewhere and keeping the handling of ROOT consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ba5a8d9-910b-4084-99c8-8b2f284edc8b
📒 Files selected for processing (16)
configurator/tests/colorUtils.test.tsconfigurator/tests/domains.test.tsconfigurator/tests/powerKnobs.test.tsconfigurator/tests/theme.test.tsconfigurator/tests/variableScales.test.tsscripts/check-llm-guide.jsscripts/check-macro-catalog.jsscripts/check-token-registry.jsscripts/check-version-sync.jsscripts/version-sync.jstests/check-llm-guide.test.jstests/check-macro-catalog.test.jstests/check-token-registry.test.jstests/forms.spec.jstests/motion.spec.jstests/version-sync.test.js
Address CodeRabbit review on #568: the hardening checked process.env.SLASHED_ROOT?.trim() for truthiness but passed the *untrimmed* value to path.resolve, so a whitespace-padded override (e.g. " /tmp/x ") would resolve to a padded, wrong path. Capture the trimmed value once and resolve that, across all five root-resolving scripts. Add a test asserting a whitespace-padded SLASHED_ROOT resolves to the intended fixture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SETyJ7R12Bxeuux7yF6y7F
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds extensive test coverage across the codebase, closing critical gaps in unit testing for pure functions, build scripts, and UI components. The changes introduce 11 new test files covering color math, version synchronization, token registry validation, form styling, motion behavior, power knobs, theme persistence, domain routing, and variable scales.
Key Changes
New Test Files
Configurator unit tests:
configurator/tests/colorUtils.test.ts— 7 pure color conversion functions (oklchToRgb, rgbToHex, luminance, contrast ratio, WCAG rating, OKLch parsing/stringification) with zero prior coverage. These functions power every color picker, WCAG panel, and exported palette; a silent regression would corrupt all resolved colors.configurator/tests/powerKnobs.test.ts— Validates knob structure (min/default/max ranges, step size, encode/decode round-trips). Ensures sliders are resettable and emit valid CSS.configurator/tests/theme.test.ts— Tests the studio light/dark theme store (persistence, toggle, root class binding). Covers 8 components' shared theme state.configurator/tests/domains.test.ts— Validates token-to-panel routing (domainOf). Ensures knobs land in the correct UI panel and that the runtime classifier agrees with CI's check-curation.configurator/tests/variableScales.test.ts— Tests sibling-scale option lists for variable pickers (SPACE_SCALE, RADIUS_SCALE, etc.). Verifies exactvar(--sf-…)wire format and step completeness.Build script tests:
tests/version-sync.test.js— Positive tests for the version propagator script. Confirms the writer actually syncs all downstream artifacts (roadmap.md, configurator/package.json, package-lock.json) from the source of truth. Validates idempotency and pre-release suffix preservation.tests/check-token-registry.test.js— Negative tests for token registry validation. Asserts the gate catches duplicate ids, nextId violations, uint16 ceiling breaches, and missing catalogue registrations.tests/check-llm-guide.test.js— Negative tests for LLM guide sync. Confirms the gate detects stale token references and missing guide files.tests/check-macro-catalog.test.js— Negative tests for macro class documentation. Validates detection of undocumented CSS classes and phantom documented classes.Behavioral/integration tests:
tests/forms.spec.js— Playwright tests for optional/forms.css. Covers text input, textarea, select styling; disabled state; --sf-field-border-color indirection; placeholder color; and width stretching in both light and dark themes.tests/motion.spec.js— Playwright tests for core/motion.css. Validates prefers-reduced-motion gating, --sf-motion-scale linear scaling, entrance animations, and keyframe binding.Script Modifications
scripts/check-token-registry.js,scripts/version-sync.js,scripts/check-llm-guide.js,scripts/check-macro-catalog.js— AddedSLASHED_ROOTenvironment variable support to allow tests to run these scripts against fixture directories instead of the real repo root.Notable Implementation Details
prefers-reduced-motionto validate both the media query gating and the --sf-motion-scale scaling formula.https://claude.ai/code/session_01SETyJ7R12Bxeuux7yF6y7F
Summary by CodeRabbit
Tests
Chores