chore(tests): Use tags instead of projects Playwright #16959
Conversation
WalkthroughThe pull request includes updates to the documentation for the E2E Testing Library, enhancing clarity and modifying usage instructions. Key changes involve a new command structure for running tests using tags instead of project-specific commands. Additionally, the structure of test cases has been simplified, with an emphasis on tagging. The Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
libs/testing/e2e/src/lib/config/playwright-config.ts (3)
Line range hint
3-8: Fix type definition inconsistency forcwdparameterThe interface still marks
cwdas optional with?while the implementation treats it as required. This mismatch could lead to runtime errors when TypeScript validation passes but the function fails.Apply this fix:
interface PlaywrightConfigParams { webServerUrl: string port?: number command: string - cwd?: string + cwd: string timeoutMs?: number }
28-29: Add validation for thecwdparameterConsider adding runtime validation to ensure
cwdis a non-empty string and represents a valid directory path to prevent cryptic errors during test execution.Example validation:
export const createPlaywrightConfig = ({ webServerUrl, port, command, cwd, timeoutMs = 5 * 60 * 1000, }: PlaywrightConfigParams) => { + if (!cwd || cwd.trim() === '') { + throw new Error('cwd must be a non-empty string'); + } return defineConfig({
Line range hint
47-51: Update test organization to use tags instead of projectsAccording to the PR objectives, we're transitioning from projects to tags for test categorization. The current configuration still uses the project-based approach.
Consider replacing the projects configuration with tag-based filtering:
- projects: [ - { name: 'everything', testMatch: 'e2e/**/*.spec.[tj]s' }, - { name: 'smoke', testMatch: 'e2e/smoke/**/*.spec.[tj]s' }, - { name: 'acceptance', testMatch: 'e2e/acceptance/**/*.spec.[tj]s' }, - ], + grep: process.env.TEST_TAG ? new RegExp(`@${process.env.TEST_TAG}`) : /.*/,libs/testing/e2e/README.md (3)
61-64: Enhance the tag-based filtering documentationWhile the command syntax is correct, consider improving this section by:
- Documenting the tagging convention (e.g., always prefix with @)
- Providing a list of commonly used/recommended tags (@fast, @slow, @smoke, etc.)
- Including examples of combining multiple tags
- **Run with a Specific Tag**: Run only the tests tagged with a specific tag: ```bash yarn e2e <app-name> --grep @fast
Test Tags
- Tests can be categorized using tags prefixed with @. Common tags include:
- @fast: Quick tests (< 30s)
- @slow: Long-running tests
- @smoke: Critical path tests
- @flaky: Tests that may need retries
- You can combine multiple tags:
Run tests that are both fast and smoke
- yarn e2e --grep "@fast.*@smoke"
<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. Context: ...e <app-name> --skip-nx-cache ``` - **Run with a Specific Tag**: Run only the tes... (ENGLISH_WORD_REPEAT_BEGINNING_RULE) </details> </details> --- `101-106`: **Clarify the transition from projects to tags** The folder structure is simplified, but it would be helpful to: 1. Show example test files with their tags 2. Provide migration guidance for teams transitioning from the project-based approach ```diff Test cases are written in spec files. Tests are tagged based on their execution time or other criteria. For example, you can use tags like `@fast` for quick tests and `@slow` for longer-running tests. Here is an example of the folder layout for testing the search engine and front-page of the `web` project: ```shell web/ (app name) -├── home-page.spec.ts (feature name, kebab-case) -└── search.spec.ts +├── home-page.spec.ts # Contains @fast tests +└── search.spec.ts # Contains @slow tests
+### Migrating from Projects to Tags
+
+If you're migrating from the project-based approach:
+1. Move tests from project-specific folders to feature-based files
+2. Add appropriate tags to your test descriptions:
- Previous
smokeproject → Add@smoketag
- Previous
acceptanceproject → Add@acceptancetag--- `131-131`: **Enhance the test case example with comprehensive tag usage** The example shows basic tag usage, but it would be more helpful to demonstrate multiple tags and explain tag selection criteria. ```diff - test('should get paid @slow', () => { + // Tags: + // @slow - Takes >30s due to payment processing + // @smoke - Critical business functionality + test('should get paid @slow @smoke', () => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/testing/e2e/README.md(3 hunks)libs/testing/e2e/src/lib/config/playwright-config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/testing/e2e/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/testing/e2e/src/lib/config/playwright-config.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/testing/e2e/README.md
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run with a Specific Tag: Run only the tes...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (1)
libs/testing/e2e/src/lib/config/playwright-config.ts (1)
Line range hint 1-67: Implementation follows library guidelines
The code successfully meets the guidelines for libs/** files:
- Uses TypeScript for type definitions
- Provides a reusable configuration utility
- Follows proper module export patterns for effective tree-shaking
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
libs/testing/e2e/src/lib/config/playwright-config.ts (2)
Line range hint
47-51: Implement tag-based test filteringThe PR title suggests moving from projects to tags, but the configuration still uses project-based filtering. Consider implementing tag-based filtering to align with the PR objectives.
Consider replacing project-based configuration with tag-based filtering:
- projects: [ - { name: 'everything', testMatch: 'e2e/**/*.spec.[tj]s' }, - { name: 'smoke', testMatch: 'e2e/smoke/**/*.spec.[tj]s' }, - { name: 'acceptance', testMatch: 'e2e/acceptance/**/*.spec.[tj]s' }, - ], + grep: process.env.TEST_TAGS ? new RegExp(process.env.TEST_TAGS) : undefined,
Line range hint
11-21: Enhance return type documentationWhile the JSDoc is comprehensive, consider adding more specific details about the return type structure and its configuration options.
Add return type details to the JSDoc:
- * @returns A configuration object for Playwright E2E tests. + * @returns {import('@playwright/test').PlaywrightTestConfig} A configuration object + * containing test directory settings, parallel execution options, retry logic, + * reporter configuration, and web server settings.libs/testing/e2e/README.md (1)
61-64: LGTM! Consider enhancing tag examples.The new tag-based filtering approach is well documented. To make it even more helpful, consider adding more common tag examples like
@smoke,@regression, or@criticalto showcase the tagging convention.🧰 Tools
🪛 LanguageTool
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run with a Specific Tag: Run only the tes...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/testing/e2e/README.md(3 hunks)libs/testing/e2e/src/lib/config/playwright-config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/testing/e2e/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/testing/e2e/src/lib/config/playwright-config.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/testing/e2e/README.md
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run with a Specific Tag: Run only the tes...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (2)
libs/testing/e2e/src/lib/config/playwright-config.ts (2)
Line range hint 3-9: Well-structured TypeScript interface
The TypeScript implementation follows best practices with explicit types and proper exports, enabling good type safety and reusability across NextJS apps.
28-28: Breaking Change: cwd parameter is now required
Removing the default value for cwd is a breaking change that requires all consumers to explicitly specify the working directory.
Let's verify the impact on existing implementations:
Co-authored-by: Kristofer <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/testing/e2e/README.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/testing/e2e/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/testing/e2e/README.md
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run with a Specific Tag: Run only the tes...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/testing/e2e/README.md(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/testing/e2e/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/testing/e2e/README.md
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run Tests with Tags: Use tags to include ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
libs/testing/e2e/README.md (1)
110-115: Add examples of common tag categoriesConsider adding a table of commonly used tags with their descriptions:
| Tag Category | Example | Description | |--------------|---------|-------------| | Speed | @fast | Tests that complete within 30 seconds | | Type | @smoke | Critical path tests | | Feature | @auth | Authentication-related tests | | Status | @flaky | Tests that may need investigation |
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/testing/e2e/README.md(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/testing/e2e/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/testing/e2e/README.md
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e --skip-nx-cache ``` - Run Tests with Tags: Use tags to include ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16959 +/- ##
=======================================
Coverage 36.41% 36.41%
=======================================
Files 6898 6898
Lines 144868 144868
Branches 41403 41403
=======================================
Hits 52751 52751
Misses 92117 92117
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 19 Total Test Services: 0 Failed, 18 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
* chore(tests): Use tags instead of projects Playwright * Tiny fix * Update libs/testing/e2e/README.md Co-authored-by: Kristofer <[email protected]> * Update documentation * Small fix --------- Co-authored-by: Kristofer <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Summary by CodeRabbit
Documentation
test.step.Chores
cwdparameter in the Playwright configuration a required parameter to streamline configuration setup.