test(shared,clerk-js): Improve coverage around fetcher params #7218
Conversation
🦋 Changeset detectedLatest commit: 4d93083 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdded a placeholder changeset and strengthened unit tests across multiple packages to assert exact pagination and related fetch parameters (e.g., initialPage, pageSize, status, scope) for various organization- and pagination-related hooks and components; no production code changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (9)
Comment |
![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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/many-pants-pump.md(1 hunks)packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx(3 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx(10 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx(11 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(2 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
.changeset/many-pants-pump.md (1)
1-2: Verify PR checklist completion.Per the PR objectives, the following checklist items remain unchecked:
pnpm testruns as expectedpnpm buildruns as expected- JSDoc comments added/updated (if applicable)
- Documentation updated in clerk-docs (if applicable)
Since this PR focuses on test coverage improvements, please confirm that
pnpm testandpnpm buildboth execute successfully before merging.packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx (1)
244-248: LGTM! Good parameter validation.The strict equality checks ensure
getOrganizationMembershipsis called with exact pagination parameters after opening the OrganizationSwitcher. This prevents regression where parameters might be accidentally omitted or modified.Also applies to: 479-483, 510-514
packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx (1)
69-69: LGTM! Comprehensive parameter validation.The strict equality checks ensure the fetcher receives the complete parameter object including both pagination (
initialPage,pageSize) and scope (for) parameters. This guards against parameter omission or incorrect merging.Also applies to: 191-191
packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts (2)
55-55: LGTM! Stricter parameter validation.Upgrading from partial object matching to strict equality ensures the fetcher receives exactly the expected parameters without any extra or missing fields. This improves test precision.
753-755: LGTM! Thorough multi-call parameter validation.Collecting and verifying all fetcher calls ensures parameter consistency across state transitions. The strict equality checks confirm both calls receive complete parameter objects with correct filter values.
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx (2)
149-155: LGTM! Precise pagination parameter validation.The assertions verify exact parameter shapes for both initial and subsequent fetch calls, including undefined values for optional filters. This ensures the hook correctly propagates pagination state without accidentally modifying or omitting parameters.
Also applies to: 208-214
653-661: LGTM! Excellent multi-call validation pattern.Aggregating all fetcher calls and validating each parameter object ensures consistency across the infinite fetch cycle. Checking for specific keys, values, and the presence of
initialPage: 2confirms proper pagination progression.packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx (2)
111-115: LGTM! Comprehensive pagination validation.The assertions verify exact parameter shapes for user membership fetches, ensuring both initial page and subsequent pagination calls pass correct parameters. This prevents regressions in pagination behavior.
Also applies to: 158-162
468-474: LGTM! Robust multi-call parameter verification.The aggregation pattern validates all fetcher calls maintain consistent parameter structure throughout the infinite fetch cycle. Checking sorted keys, fixed values, and
initialPage: 2presence ensures proper pagination state management.Also applies to: 656-662
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
panteliselef
force-pushed
the
elef/improve-fetcher-param-tests
branch
from
1805f2e to
58560c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
253-257: Consider extracting the repeated assertion pattern into a helper function.The same 5-line assertion pattern appears in six different tests, which introduces maintenance overhead. While the assertions are correct and effectively verify fetcher parameters, extracting this into a reusable helper would improve maintainability.
Consider adding a helper function at the top of the test suite:
function expectAllFetcherCallsToMatch(mock: any, expectedParams: object) { expect(mock).toHaveBeenCalled(); const paramsCalls = mock.mock.calls.map(([params]) => params); paramsCalls.forEach(params => { expect(params).toStrictEqual(expectedParams); }); }Then replace each occurrence with:
-expect(fetcherMock).toHaveBeenCalled(); -const paramsCalls = fetcherMock.mock.calls.map(([params]) => params); -paramsCalls.forEach(params => { - expect(params).toStrictEqual({ initialPage: 1, pageSize: 2 }); -}); +expectAllFetcherCallsToMatch(fetcherMock, { initialPage: 1, pageSize: 2 });Also applies to: 303-307, 351-355, 386-390, 438-442, 476-480
packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx (1)
244-248: Consider extracting repeated assertions into a helper function.The same assertion pattern appears in three test cases (lines 244-248, 479-483, 510-514). While the current implementation is clear and readable, you could optionally reduce duplication with a helper function:
const expectOrganizationMembershipsCalledWithDefaults = (fixtures: any) => { expect(fixtures.clerk.user?.getOrganizationMemberships).toHaveBeenCalledTimes(1); expect(fixtures.clerk.user?.getOrganizationMemberships.mock.calls[0][0]).toStrictEqual({ initialPage: 1, pageSize: 10, }); };Then replace each occurrence with:
expectOrganizationMembershipsCalledWithDefaults(fixtures);This would improve maintainability if pagination defaults ever change, but the current approach is also perfectly acceptable for test clarity.
Also applies to: 479-483, 510-514
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.changeset/many-pants-pump.md(1 hunks)packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx(3 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx(10 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.test.tsx(11 hunks)packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx(6 hunks)packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts(2 hunks)packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/many-pants-pump.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/clerk-js/src/ui/hooks/tests/useCoreOrganizationList.test.tsx
- packages/shared/src/react/hooks/tests/usePlans.spec.tsx
- packages/shared/src/react/hooks/tests/usePagesOrInfinite.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (8)
packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganization.test.tsx (5)
149-155: LGTM! Excellent parameter validation for memberships pagination.The assertions correctly verify that
getMembershipsis called with the expected pagination parameters, including proper handling ofinitialPageincrementing onfetchNextand appropriateundefinedvalues for optionalroleandqueryparameters.Also applies to: 208-214
274-279: LGTM! Domains pagination parameters validated correctly.The assertions properly verify pagination behavior for
getDomains, ensuring correct parameter passing across page transitions.Also applies to: 321-326
392-397: LGTM! Membership requests pagination validated correctly.The assertions properly validate
getMembershipRequestsparameters, including the defaultstatus: 'pending'value.Also applies to: 433-438
508-513: LGTM! Invitations pagination parameters validated correctly.The assertions correctly verify
getInvitationsparameters, properly using array type forstatus: ['pending'].Also applies to: 545-550
619-624: Excellent comprehensive validation for infinite fetch!The enhanced assertions provide thorough coverage by:
- Validating exact parameter keys using
Object.keys(params).sort()- Checking each parameter value across all calls
- Confirming pagination occurred by verifying
initialPage === 2existsThis approach catches both missing/extra parameters and incorrect values.
Also applies to: 653-661
packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx (1)
253-257: Excellent test coverage improvements!The added assertions effectively verify that the fetcher is invoked with the exact expected parameter objects across various scenarios (authenticated/unauthenticated, pagination/infinite modes, with/without keepPreviousData). This aligns perfectly with the PR's objective to improve coverage around fetcher parameters.
Also applies to: 303-307, 351-355, 386-390, 438-442, 476-480
packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx (2)
244-248: LGTM! Good test coverage for pagination parameters.The assertions correctly verify that
getOrganizationMembershipsis called with the expected pagination defaults when opening the switcher.
479-483: LGTM! Consistent parameter verification.The assertions correctly verify the pagination parameters when switching organizations.
panteliselef
force-pushed
the
elef/improve-fetcher-param-tests
branch
from
58560c6 to
4d93083
Compare
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit