Repository navigation
docs(mosaic): favor integration tests and stop asserting CSS in jsdom - #9883
alexcarpenter wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0897b9d The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (9)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughThe PR replaces isolated Mosaic model, controller, and view testing guidance with integration-first guidance. It emphasizes real-layer composition, mocked Clerk package boundaries, DOM-driven interactions, and user-observable assertions. Unit tests remain for pure helpers and complex machines. Related architecture, migration, skill, and layer references now describe this approach. Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The updated testing guidance is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
🤖 Two small additions that fit the guide's approach: 1. Say outright that pass-through props aren't tested. The styled-component section says to test "defaults and prop wiring a developer would rely on", which is loose enough that agents keep writing "prop X reached child Y" tests. Suggested wording:
2. Extend "test a behavior once" to blocks, not just primitives. Rule 1 names styled components vs primitives. Features duplicate blocks the same way: every user-profile action test was re-asserting
|
Description
Rewrites the Mosaic testing guidance (
.claude/skills/mosaic/references/testing.md) so agents stop producing large numbers of low-value unit tests.The previous docs prescribed five test files per flow (model, controller, view, wrapper-with-all-layers-mocked, integration). That produced heavy duplication (one rule asserted in four files), tests coupled to internals (machine state names, busy keys, harness
<output>elements,vi.mockof internal modules), and StyleX "probe" tests that assert compiled CSS atoms in jsdom, which never loads the stylesheet.The new guide follows the testing trophy (Write tests. Not too many. Mostly integration.):
@clerk/shared/react, driven withuserEventand role queries.vi.mocka module insidepackages/mosaic..cl-<slot>/data-<axis>contract is pinned once centrally and once per part; visual checks belong in a real browser.references/mosaic-architecture.mdand the other Mosaic skill references (SKILL.md,migration.md,models.md,controllers.md,views.md,headless.md) drop the per-layer test mandate and point to the new guide. Existing tests are unchanged.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change