Uh oh!
There was an error while loading. Please reload this page.
chore(repo): Clean up machine integration tests - #8214
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@clerk/agent-toolkit@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/dev-cli@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
📝 WalkthroughWalkthroughThis PR consolidates machine authentication test infrastructure by refactoring machine auth utilities and test registration. The changes migrate test implementations from 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/testUtils/createTestUtils.ts`:
- Around line 23-32: The exported signature for createTestUtils is unsound:
change it to explicit overloads (one overload for Params that includes
page/useTestingToken returning FullReturn and another for Params without page
returning OnlyAppReturn) instead of the current conditional return type; remove
generic defaults that reference body locals (drop/replace Services = typeof
services, PO = typeof pageObjects, BH = typeof browserHelpers) and either accept
plain generics without those defaults or infer types from exported module-level
types, and eliminate the runtime "as any" casts by narrowing the return with a
runtime check on params.page so the implementation returns the correct typed
shape (use the overloads to guide callers and keep internal variables services,
pageObjects, browserHelpers as implementation details).
In `@integration/testUtils/machineAuthHelpers.ts`:
- Around line 184-198: The createMachineClient and createOAuthClient functions
currently return raw Clerk clients and must be wrapped with the retry proxy from
integration/testUtils/retryableClerkClient.ts; update both functions to import
and use the retry wrapper (e.g., withRetry or the exported retryable client
factory) so they return the retry-wrapped client instance instead of the direct
createClerkClient(...) result, ensuring provisioning/cleanup in
beforeAll/afterAll get 429/502/503/504 backoff behavior.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: e5f82168-7e00-4ff5-bb84-7b76d3ef86eb
📒 Files selected for processing (8)
integration/testUtils/createTestUtils.tsintegration/testUtils/index.tsintegration/testUtils/machineAuthHelpers.tsintegration/testUtils/machineAuthService.tsintegration/tests/astro/machine.test.tsintegration/tests/next-machine.test.tsintegration/tests/react-router/machine.test.tsintegration/tests/tanstack-start/machine.test.ts
💤 Files with no reviewable changes (1)
- integration/testUtils/machineAuthService.ts
Uh oh!
There was an error while loading. Please reload this page.
| const createMachineClient = () => | ||
| createClerkClient({ | ||
| secretKey: instanceKeys.get('with-api-keys').sk, | ||
| }); | ||
| const buildApp = async (adapter: MachineAuthTestAdapter, addRoutes: RouteBuilder): Promise<Application> => { | ||
| const config = addRoutes(adapter.baseConfig.clone()); | ||
| return config.commit(); | ||
| }; | ||
| const createOAuthClient = (app: Application) => | ||
| createClerkClient({ | ||
| secretKey: app.env.privateVariables.get('CLERK_SECRET_KEY'), | ||
| publishableKey: app.env.publicVariables.get('CLERK_PUBLISHABLE_KEY'), | ||
| }); |
There was a problem hiding this comment.
Wrap these direct Clerk clients with the retry proxy.
createMachineClient() and createOAuthClient() bypass withRetry, so the machine/OAuth provisioning and cleanup in this file loses the 429/502/503/504 backoff that integration/testUtils/retryableClerkClient.ts:1-92 provides. These calls run in beforeAll/afterAll, so one transient Clerk API failure will fail the whole suite.
🐛 Proposed fix
+import { withRetry } from './retryableClerkClient';+
const createMachineClient = () =>
- createClerkClient({- secretKey: instanceKeys.get('with-api-keys').sk,- });+ withRetry(+ createClerkClient({+ secretKey: instanceKeys.get('with-api-keys').sk,+ }),+ );
@@
const createOAuthClient = (app: Application) =>
- createClerkClient({- secretKey: app.env.privateVariables.get('CLERK_SECRET_KEY'),- publishableKey: app.env.publicVariables.get('CLERK_PUBLISHABLE_KEY'),- });+ withRetry(+ createClerkClient({+ secretKey: app.env.privateVariables.get('CLERK_SECRET_KEY'),+ publishableKey: app.env.publicVariables.get('CLERK_PUBLISHABLE_KEY'),+ }),+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/testUtils/machineAuthHelpers.ts` around lines 184 - 198, The
createMachineClient and createOAuthClient functions currently return raw Clerk
clients and must be wrapped with the retry proxy from
integration/testUtils/retryableClerkClient.ts; update both functions to import
and use the retry wrapper (e.g., withRetry or the exported retryable client
factory) so they return the retry-wrapped client instance instead of the direct
createClerkClient(...) result, ensuring provisioning/cleanup in
beforeAll/afterAll get 429/502/503/504 backoff behavior.
Uh oh!
There was an error while loading. Please reload this page.
Description
Refactors the machine integration tests to use shared Playwright helpers instead of duplicating the same auth contract across framework-specific files.
This moves the repeated API key, M2M, and OAuth machine test scenarios into
integration/testUtils/machineAuthHelpers.tsand reduces the framework test files to thin adapters that only define route wiring and endpoint paths.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit