feat(testing): support per-instance testing tokens in setupClerkTestingToken#8833
feat(testing): support per-instance testing tokens in setupClerkTestingToken#8833jacekradko wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR extends Clerk's testing helpers to support instance-specific bot-protection bypass tokens for Playwright and integration tests. The core enhancement allows ChangesPer-instance testing token support
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant setupClerkTestingToken
participant BrowserContext
participant TestingTokenProvider
participant ClerkFrontendAPI
TestRunner->>setupClerkTestingToken: setup({ page, options })
setupClerkTestingToken->>BrowserContext: resolve frontendApiUrl and check (context, frontendApiUrl)
alt first registration for frontendApiUrl
setupClerkTestingToken->>TestingTokenProvider: resolve testingToken once
TestingTokenProvider-->>setupClerkTestingToken: token or undefined
setupClerkTestingToken->>BrowserContext: context.route(frontendApiUrl pattern)
BrowserContext->>ClerkFrontendAPI: forward request with _clerk_testing_token
else already registered for frontendApiUrl
setupClerkTestingToken-->>TestRunner: no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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
🧹 Nitpick comments (3)
packages/testing/src/cypress/setupClerkTestingToken.ts (2)
33-33: 💤 Low valueConsider using nullish coalescing for stricter precedence semantics.
Using
||will fall through to the environment variable iftestingTokenis any falsy value (including empty string). For stricter "explicit parameter takes precedence" semantics,??(nullish coalescing) would only fall through fornullorundefined.♻️ Proposed refinement
- const testingToken = params?.options?.testingToken || Cypress.env('CLERK_TESTING_TOKEN'); + const testingToken = params?.options?.testingToken ?? Cypress.env('CLERK_TESTING_TOKEN');🤖 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 `@packages/testing/src/cypress/setupClerkTestingToken.ts` at line 33, Replace the loose OR fallback with nullish coalescing so an explicit falsy testingToken parameter (e.g., empty string) is honored: change the assignment to use testingToken = params?.options?.testingToken ?? Cypress.env('CLERK_TESTING_TOKEN') in the setupClerkTestingToken logic so only null or undefined trigger reading the CLERK_TESTING_TOKEN env var.
25-25: ⚡ Quick winAdd explicit return type annotation.
This exported public API function lacks an explicit return type. As per coding guidelines, public APIs should always define explicit return types for clarity and type safety.
♻️ Proposed fix
-export const setupClerkTestingToken = (params?: SetupClerkTestingTokenParams) => { +export const setupClerkTestingToken = (params?: SetupClerkTestingTokenParams): void => {🤖 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 `@packages/testing/src/cypress/setupClerkTestingToken.ts` at line 25, The exported public API function setupClerkTestingToken lacks an explicit return type; update its signature to include a concrete return type that matches the implementation (for example : void or : Promise<void> if async) by changing "export const setupClerkTestingToken = (params?: SetupClerkTestingTokenParams)" to include the appropriate return annotation (e.g., "export const setupClerkTestingToken = (params?: SetupClerkTestingTokenParams): void" or ": Promise<void>") so the public API has an explicit, type-safe return.Source: Coding guidelines
packages/testing/src/playwright/unstable/page-objects/index.ts (1)
26-36: ⚡ Quick winDocument the new
testingTokenOptionsparameter on this public API.
createPageObjectsnow exposes new behavior viatestingTokenOptions, but the public API contract in this file is still undocumented. Please add/update JSDoc for the new option and expected precedence behavior to avoid reference-doc drift.As per coding guidelines, public APIs under
packages/**/src/**/*.{ts,tsx}should maintain comprehensive JSDoc, and public/reference-facing API changes should keep generated docs aligned (with Docs-team awareness when needed).🤖 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 `@packages/testing/src/playwright/unstable/page-objects/index.ts` around lines 26 - 36, Add/update JSDoc on the exported createPageObjects function to document the new testingTokenOptions parameter and its behavior: describe the type (PlaywrightSetupClerkTestingTokenOptions), defaulting behavior when testingTokenOptions is omitted, and the precedence rules vs useTestingToken and baseURL (e.g., when testingTokenOptions is provided it takes precedence for token setup, otherwise useTestingToken controls token injection). Include a short summary of the function purpose, annotate each parameter (page, useTestingToken, baseURL, testingTokenOptions) and the return value, and ensure the docblock follows the project's public API JSDoc conventions used elsewhere in packages/*/src so generated docs remain accurate.Source: Coding guidelines
🤖 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 `@integration/testUtils/index.ts`:
- Around line 39-53: The current logic caches a failed minting result
(undefined) causing permanent fallback; change it so failures are not
permanently cached by either (a) only inserting into testingTokenCache after a
successful createTestingToken() resolves (reference testingTokenCache,
publishableKey, clerkClient.testingTokens.createTestingToken), or (b) if you
must cache the promise, attach a .catch handler that removes
testingTokenCache.delete(publishableKey) before rethrowing/returning undefined
so transient errors don’t remain cached; ensure the returned promise behavior is
preserved and logs continue to run on errors.
---
Nitpick comments:
In `@packages/testing/src/cypress/setupClerkTestingToken.ts`:
- Line 33: Replace the loose OR fallback with nullish coalescing so an explicit
falsy testingToken parameter (e.g., empty string) is honored: change the
assignment to use testingToken = params?.options?.testingToken ??
Cypress.env('CLERK_TESTING_TOKEN') in the setupClerkTestingToken logic so only
null or undefined trigger reading the CLERK_TESTING_TOKEN env var.
- Line 25: The exported public API function setupClerkTestingToken lacks an
explicit return type; update its signature to include a concrete return type
that matches the implementation (for example : void or : Promise<void> if async)
by changing "export const setupClerkTestingToken = (params?:
SetupClerkTestingTokenParams)" to include the appropriate return annotation
(e.g., "export const setupClerkTestingToken = (params?:
SetupClerkTestingTokenParams): void" or ": Promise<void>") so the public API has
an explicit, type-safe return.
In `@packages/testing/src/playwright/unstable/page-objects/index.ts`:
- Around line 26-36: Add/update JSDoc on the exported createPageObjects function
to document the new testingTokenOptions parameter and its behavior: describe the
type (PlaywrightSetupClerkTestingTokenOptions), defaulting behavior when
testingTokenOptions is omitted, and the precedence rules vs useTestingToken and
baseURL (e.g., when testingTokenOptions is provided it takes precedence for
token setup, otherwise useTestingToken controls token injection). Include a
short summary of the function purpose, annotate each parameter (page,
useTestingToken, baseURL, testingTokenOptions) and the return value, and ensure
the docblock follows the project's public API JSDoc conventions used elsewhere
in packages/*/src so generated docs remain accurate.
🪄 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), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: a6997c84-afdd-438a-bc9a-dffde0a53f76
📒 Files selected for processing (11)
.changeset/eighty-husky-fly.mdintegration/testUtils/index.tspackages/testing/src/common/types.tspackages/testing/src/cypress/setupClerkTestingToken.tspackages/testing/src/playwright/__tests__/setupClerkTestingToken.test.tspackages/testing/src/playwright/index.tspackages/testing/src/playwright/setupClerkTestingToken.tspackages/testing/src/playwright/unstable/index.tspackages/testing/src/playwright/unstable/page-objects/app.tspackages/testing/src/playwright/unstable/page-objects/index.tspackages/testing/src/playwright/unstable/page-objects/testingToken.ts
API Changes Report
Summary
@clerk/testingCurrent version: 2.1.0 Subpath
|
🦋 Changeset detectedLatest commit: 874068f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@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/vue
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/testing/src/playwright/helpers.ts (1)
26-64: ⚡ Quick winConsider documenting the
setupClerkTestingTokenOptionsparameter.The JSDoc for
clerk.signIndoes not document thesetupClerkTestingTokenOptionsparameter. Since this PR introduces lazy testing token providers (a significant new capability for multi-instance scenarios), consider adding a@paramentry explaining when and how to use this option, especially the new lazy provider support.Example:
* `@param` opts.setupClerkTestingTokenOptions - Optional token setup configuration. * Pass `{ testingToken: string | () => Promise<string> }` to override the default * CLERK_TESTING_TOKEN for per-instance test scenarios.As per coding guidelines, public API JSDoc is used to generate customer-facing reference documentation, so documenting this parameter will help users understand the new multi-instance testing capabilities.
🤖 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 `@packages/testing/src/playwright/helpers.ts` around lines 26 - 64, Add a `@param` entry to the JSDoc for clerk.signIn documenting the setupClerkTestingTokenOptions parameter: describe that opts.setupClerkTestingTokenOptions is optional, accepts { testingToken: string | () => Promise<string> } (or similar) to override the default CLERK_TESTING_TOKEN, and explain when to use it (per-instance test scenarios / lazy token providers) and that it works with the helper’s internal setupClerkTestingToken call to enable multi-instance testing; reference the symbol name setupClerkTestingTokenOptions so readers can find usage in the signIn helper.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/testing/src/playwright/helpers.ts`:
- Around line 26-64: Add a `@param` entry to the JSDoc for clerk.signIn
documenting the setupClerkTestingTokenOptions parameter: describe that
opts.setupClerkTestingTokenOptions is optional, accepts { testingToken: string |
() => Promise<string> } (or similar) to override the default
CLERK_TESTING_TOKEN, and explain when to use it (per-instance test scenarios /
lazy token providers) and that it works with the helper’s internal
setupClerkTestingToken call to enable multi-instance testing; reference the
symbol name setupClerkTestingTokenOptions so readers can find usage in the
signIn helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 86bc96d8-be47-4477-be60-3d11b40d9dd7
📒 Files selected for processing (2)
.changeset/eighty-husky-fly.mdpackages/testing/src/playwright/helpers.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/eighty-husky-fly.md
1 participant
The testing-token captcha bypass has been single-instance:
clerkSetuplast-writer-wins onCLERK_FAPI/CLERK_TESTING_TOKEN, the route guard was a per-contextWeakSetso a second registration for a different host silently no-oped, and the page objects always registered against the global host. Any test running against an app other than the last-initialized one got no bypass at all, which is how legal-consent spent a week red against the captcha-enabled staging instance (run 27344100979).setupClerkTestingTokennow acceptsoptions.testingToken(string or lazy provider) and registers one route per (context, frontendApiUrl) pair, and the integration harness passes per-app options derived from each app's own publishable key, minting one token per instance through the existing retry-wrapped clerkClient. Tests need no changes; the auto-setup on navigation picks it up.Verified locally with the two-app scenario that reproduces the bug (
E2E_APP_ID=react.vite.withLegalConsent,react.vite.withEmailCodes, global env pointing at the second app): on main the legal-consent traffic shows 0 token-carrying requests on its own FAPI host; on this branch every FAPI request is intercepted and reissued with the instance's token.Summary by CodeRabbit
New Features
Behavior Changes
Cypress