fix(testing): Better route interception hierarchy #5673
Conversation
🦋 Changeset detectedLatest commit: 587c438 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
jacekradko
force-pushed
the
fix/better-route-interception-hierarchy
branch
from
3246848 to
6705a94
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR improves the interception hierarchy for FAPI calls by switching from page.route to context.route, allowing consumers to override FAPI calls consistently. Key changes include:
- Updating setupClerkTestingToken to accept a BrowserContext (with fallback to page.context()) and route requests via the context.
- Modifying helper and test files to extract and pass the context where needed.
- Re-enabling previously skipped tests in resiliency tests and adding error handling in test cleanup.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/testing/src/playwright/setupClerkTestingToken.ts | Updated to prefer context for routing and modified API error handling. |
| packages/testing/src/playwright/helpers.ts | Now extracts the browser context from page before calling setup token. |
| integration/tests/reverification.test.ts | Added try-catch around cleanup operations in afterAll. |
| integration/tests/resiliency.test.ts | Unskipped tests previously marked as TODO for hotloaded client/environment. |
| integration/tests/non-secure-context.test.ts | Updated test signature to receive and pass context. |
| integration/testUtils/index.ts | Updated testing token utilities to use context. |
| .changeset/fruity-areas-train.md | Changeset documentation updated to reflect routing changes. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
integration/tests/resiliency.test.ts:229
- Unskipping the test 'clerk-js environment fails and status degraded' without resolving the associated flakiness risks intermittent failures. It may be beneficial to temporarily re-skip or add robustness measures until the issue is resolved.
test('clerk-js environment fails and status degraded', async ({ page, context }) => {
| @@ -197,8 +197,7 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('resilienc | |||
| await expect(page.getByText('Clerk is loading', { exact: true })).toBeHidden(); | |||
| }); | |||
|
|
|||
| // TODO: Fix detection of hotloaded clerk-js failing | |||
| test.skip('clerk-js client fails and status degraded', async ({ page, context }) => { | |||
| test('clerk-js client fails and status degraded', async ({ page, context }) => { | |||
There was a problem hiding this comment.
Re-enabling the test 'clerk-js client fails and status degraded' without addressing its known hotloading issues may lead to unstable behavior. Consider revisiting the underlying flakiness or adding proper conditions to mitigate intermittent failures.
Copilot uses AI. Check for mistakes.
| await fakeViewer.deleteIfExists(); | ||
| await fakeAdmin.deleteIfExists(); | ||
| await app.teardown(); | ||
| try { |
There was a problem hiding this comment.
[nitpick] While wrapping cleanup functions in a try-catch block prevents test failures during teardown, silently logging errors could mask underlying issues. Consider enhancing error reporting or re-throwing critical failures after logging.
Copilot uses AI. Check for mistakes.
Description
Switching over our interception of FAPI calls from
page.routetocontext.routeas routes set up with page.route() take precedence over browser context routes when request matches both handlers.This allows for consumers to use
page.routeto override calls to FAPI consistently.Related: #5672
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change