chore(repo,testing): Add API keys component e2e tests #6192
Conversation
🦋 Changeset detectedLatest commit: 45f8084 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 ↗︎
|
📝 Walkthrough""" WalkthroughThis change introduces integration tests for the API keys component, specifically targeting its creation, revocation, and copying functionalities. A new Playwright page object is added to facilitate UI interactions with the API keys component, and this page object is registered in the central page objects factory. A new React page is created to render the APIKeys component within a Next.js app template. Additionally, a changeset documents the introduction of API keys testing helpers in the Assessment against linked issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)`**/*.{js,ts,tsx,jsx}`: All code must pass ESLint checks with the project's configuration. Use Prettier for consistent code formatting.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`**/*.{ts,tsx}`: Maintain comprehensive JSDoc comments for public APIs.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`**/*.{test,spec}.{js,ts,tsx,jsx}`: Unit tests are required for all new functionality.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`integration/**`: Framework integration templates and E2E tests must be placed in the integration/ directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/global.mdc) List of files the instruction was applied to:
`**/*.ts`: Always define explicit return types for functions, especially public ...
📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc) List of files the instruction was applied to:
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
integration/templates/next-app-router/src/app/api-keys/page.tsx (1)
3-5: Consider adding explicit return type for consistency.The component implementation is clean and follows React best practices. Consider adding an explicit return type for better TypeScript consistency.
-export default function Page() { +export default function Page(): JSX.Element { return <APIKeys />; }packages/testing/src/playwright/unstable/page-objects/apiKeys.ts (2)
4-4: Add explicit return type for the factory function.Consider adding an explicit return type for better TypeScript documentation and consistency with coding guidelines.
-export const createAPIKeysComponentPageObject = (testArgs: { page: EnhancedPage }) => { +export const createAPIKeysComponentPageObject = (testArgs: { page: EnhancedPage }): ReturnType<typeof createAPIKeysComponentPageObject> => {Alternatively, define an interface for the returned object type for better maintainability.
20-22: Consider using more semantic selectors for better test reliability.CSS class selectors like
.cl-apiKeys-rootcan be brittle if styling changes. Consider usingdata-testidattributes or role-based selectors where possible for more stable tests.waitForMounted: () => { - return page.waitForSelector('.cl-apiKeys-root', { state: 'attached' }); + return page.waitForSelector('[data-testid="api-keys-root"]', { state: 'attached' }); },This applies to other CSS class selectors throughout the file as well.
integration/tests/machine-auth/component.test.ts (1)
125-127: Consider adding error handling for clipboard operations.Clipboard operations can sometimes fail due to browser security policies or timing issues. Consider wrapping the clipboard read in a try-catch or adding retry logic.
-const clipboardText = await page.evaluate('navigator.clipboard.readText()'); -expect(clipboardText).toBe(data.secret); +try { + const clipboardText = await page.evaluate('navigator.clipboard.readText()'); + expect(clipboardText).toBe(data.secret); +} catch (error) { + throw new Error(`Failed to read clipboard: ${error.message}`); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
integration/playwright.config.ts(1 hunks)integration/templates/next-app-router/src/app/api-keys/page.tsx(1 hunks)integration/tests/machine-auth/component.test.ts(1 hunks)packages/testing/src/playwright/unstable/page-objects/apiKeys.ts(1 hunks)packages/testing/src/playwright/unstable/page-objects/index.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
`**/*.{js,ts,tsx,jsx}`: All code must pass ESLint checks with the project's configuration. Use Prettier for consistent code formatting.
**/*.{js,ts,tsx,jsx}: All code must pass ESLint checks with the project's configuration.
Use Prettier for consistent code formatting.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)
List of files the instruction was applied to:
packages/testing/src/playwright/unstable/page-objects/index.tsintegration/templates/next-app-router/src/app/api-keys/page.tsxpackages/testing/src/playwright/unstable/page-objects/apiKeys.tsintegration/playwright.config.tsintegration/tests/machine-auth/component.test.ts
`**/*.{ts,tsx}`: Maintain comprehensive JSDoc comments for public APIs.
**/*.{ts,tsx}: Maintain comprehensive JSDoc comments for public APIs.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)
List of files the instruction was applied to:
packages/testing/src/playwright/unstable/page-objects/index.tsintegration/templates/next-app-router/src/app/api-keys/page.tsxpackages/testing/src/playwright/unstable/page-objects/apiKeys.tsintegration/playwright.config.tsintegration/tests/machine-auth/component.test.ts
`packages/**`: All publishable packages under the @clerk namespace must be located in the packages/ directory.
packages/**: All publishable packages under the @clerk namespace must be located in the packages/ directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/global.mdc)
List of files the instruction was applied to:
packages/testing/src/playwright/unstable/page-objects/index.tspackages/testing/src/playwright/unstable/page-objects/apiKeys.ts
`**/index.ts`: Use index.ts files for clean imports but avoid deep barrel exports.
**/index.ts: Use index.ts files for clean imports but avoid deep barrel exports.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/react.mdc)
List of files the instruction was applied to:
packages/testing/src/playwright/unstable/page-objects/index.ts
`**/*.ts`: Always define explicit return types for functions, especially public ...
**/*.ts: Always define explicit return types for functions, especially public APIs.
Use proper type annotations for variables and parameters where inference isn't clear.
Avoidanytype; preferunknownwhen type is uncertain, then narrow with type guards.
Useinterfacefor object shapes that might be extended; usetypefor unions, primitives, and computed types.
Preferreadonlyproperties for immutable data structures.
Useprivatefor internal implementation details,protectedfor inheritance, andpublicexplicitly for clarity in public APIs.
Prefer composition and interfaces over deep inheritance chains; use mixins for shared behavior.
Use ES6 imports/exports consistently; avoid barrel files (index.ts re-exports) to prevent circular dependencies.
Use type-only imports (import type { ... }) where possible.
Useas constfor literal types and thesatisfiesoperator for type checking without widening.
Enable--incrementaland--tsBuildInfoFilefor faster builds.
Use ESLint with @typescript-eslint/recommended rules and Prettier for formatting.
Use lint-staged and Husky for pre-commit checks.
Use type-coverage to measure type safety.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
List of files the instruction was applied to:
packages/testing/src/playwright/unstable/page-objects/index.tspackages/testing/src/playwright/unstable/page-objects/apiKeys.tsintegration/playwright.config.tsintegration/tests/machine-auth/component.test.ts
`integration/**`: Framework integration templates and E2E tests must be placed in the integration/ directory.
integration/**: Framework integration templates and E2E tests must be placed in the integration/ directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/global.mdc)
List of files the instruction was applied to:
integration/templates/next-app-router/src/app/api-keys/page.tsxintegration/playwright.config.tsintegration/tests/machine-auth/component.test.ts
`**/*.{jsx,tsx}`: Always use functional components with hooks instead of class c...
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components.
Follow PascalCase naming for components; the filename and component name should match.
Limit component size to 150-200 lines; extract logic into custom hooks.
Export components as named exports for better tree-shaking.
One component per file with matching filename and component name.
Co-locate related files (component, test, stories) in the same directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/react.mdc)
List of files the instruction was applied to:
integration/templates/next-app-router/src/app/api-keys/page.tsx
`**/*.{test,spec}.{js,ts,tsx,jsx}`: Unit tests are required for all new functionality.
**/*.{test,spec}.{js,ts,tsx,jsx}: Unit tests are required for all new functionality.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc)
List of files the instruction was applied to:
integration/tests/machine-auth/component.test.ts
🧬 Code Graph Analysis (1)
packages/testing/src/playwright/unstable/page-objects/index.ts (1)
packages/testing/src/playwright/unstable/page-objects/apiKeys.ts (1)
createAPIKeysComponentPageObject(4-59)
⏰ Context from checks skipped due to timeout of 90000ms (24)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 13)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
integration/playwright.config.ts (1)
23-23: LGTM: Necessary permission for clipboard testing.The addition of
clipboard-readpermission properly enables the API key copying tests to verify clipboard functionality.packages/testing/src/playwright/unstable/page-objects/index.ts (2)
3-3: LGTM: Proper integration following established patterns.The import follows the existing alphabetical ordering and naming conventions.
50-50: LGTM: Consistent page object registration.The apiKeys page object is properly integrated using the same pattern as other page objects.
packages/testing/src/playwright/unstable/page-objects/apiKeys.ts (2)
44-47: Verify async/await pattern in selectExpiration method.The method properly handles the asynchronous dropdown interaction pattern. The implementation correctly waits for the dropdown to open before selecting an option.
7-16: Well-structured expiration options mapping.The
expirationOptionsconstant withas constassertion provides good type safety and maintains the mapping between internal values and UI display text.integration/tests/machine-auth/component.test.ts (3)
7-8: LGTM: Proper test configuration for stateful API key tests.Using serial mode is appropriate here since the tests likely depend on the state of created API keys. The environment configuration targeting API keys is also correct.
12-21: LGTM: Proper test lifecycle management.The setup and teardown properly handle fake user creation and cleanup, ensuring test isolation at the suite level.
114-127: LGTM: Well-implemented clipboard verification test.The test properly:
- Intercepts the API response to get the expected secret
- Triggers the copy action
- Reads from clipboard using the configured permission
- Verifies the clipboard content matches the API response
This is a comprehensive integration test that validates the entire copy flow.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
wobsoriano
changed the title
| await expect(table.locator('.cl-tableRow', { hasText: apiKeyName })).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('can copy api keys', async ({ page, context }) => { |
There was a problem hiding this comment.
should we also test toggling visiblity?
Description
This PR adds integration tests to the
<APIKeys />componentResolves USER-2238
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit