feat: add playwright e2e testing framework #4468
Conversation
|
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on enhancing the development environment and improving user verification features. Key modifications include updating commands in the Changes
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (4)
turbo.json (1)
36-36: Consider moving thetest:e2eentry to a more appropriate section.The introduction of the
test:e2eentry is a good step towards configuring end-to-end testing. However, placing it under thestubsection might not be the most suitable location.Consider the following suggestions:
Move the
test:e2eentry to a more relevant section in the configuration file, such as creating a newtestingsection or placing it alongside other testing-related configurations.Provide a brief comment explaining the purpose and potential usage of the
test:e2eentry. This will help other developers understand its significance and how it relates to end-to-end testing.If there are any specific configuration options that need to be set for end-to-end testing, consider adding them within the
test:e2eobject. This will make the configuration more comprehensive and easier to understand.By addressing these points, the configuration file will have a clearer structure and provide better guidance for configuring end-to-end testing.
README.zh-CN.md (1)
135-137: Approve the addition of the "Star History" section.The "Star History" section is a valuable addition to the README file, providing users and contributors with a visual representation of the repository's popularity over time. The placement of the section after the "CHANGELOG" and before the "Contributor" section is appropriate.
To improve accessibility, consider adding alt text to the image:
-[](https://star-history.com/#vbenjs/vue-vben-admin&Date) +[](https://star-history.com/#vbenjs/vue-vben-admin&Date)README.md (1)
135-138: Useful addition of the star history chart. Consider the external API dependency and use a relative repository path.The star history chart is a valuable addition to the README, providing insights into the repository's popularity and growth over time.
However, please note the following:
- The chart relies on an external API (star-history.com), which introduces a dependency. Consider monitoring the availability and reliability of this service to ensure the chart remains accessible.
- The image URL currently uses an absolute repository path (
vbenjs/vue-vben-admin). To handle potential repository name changes, consider using a relative path instead.To use a relative repository path, update the image URL as follows:
-[](https://star-history.com/#vbenjs/vue-vben-admin&Date) +[](https://star-history.com/#vbenjs/vue-vben-admin&Date)This way, the chart will still work correctly if the repository name changes in the future.
playground/__tests__/e2e/common/auth.ts (1)
6-6: Translate code comments to English for consistencyThe comments are currently written in Chinese. For better collaboration among all team members, especially those who may not read Chinese, please consider translating the comments to English.
Also applies to: 18-18, 19-19, 26-26, 33-33, 39-39, 43-43
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (21)
- .gitpod.yml (1 hunks)
- README.ja-JP.md (1 hunks)
- README.md (1 hunks)
- README.zh-CN.md (1 hunks)
- docs/src/en/guide/essentials/development.md (1 hunks)
- docs/src/guide/essentials/development.md (1 hunks)
- internal/lint-configs/eslint-config/src/configs/node.ts (1 hunks)
- internal/lint-configs/eslint-config/src/custom-config.ts (1 hunks)
- package.json (4 hunks)
- packages/@core/ui-kit/shadcn-ui/package.json (2 hunks)
- packages/effects/common-ui/src/components/captcha/slider-captcha/slider-captcha-action.vue (1 hunks)
- packages/effects/common-ui/src/ui/authentication/forget-password.vue (1 hunks)
- packages/effects/common-ui/src/ui/authentication/login.vue (1 hunks)
- packages/effects/common-ui/src/ui/authentication/register.vue (1 hunks)
- playground/tests/e2e/auth-login.spec.ts (1 hunks)
- playground/tests/e2e/common/auth.ts (1 hunks)
- playground/package.json (1 hunks)
- playground/playwright.config.ts (1 hunks)
- pnpm-workspace.yaml (3 hunks)
- turbo.json (1 hunks)
- vitest.config.ts (1 hunks)
Additional context used
GitHub Check: Test (ubuntu-latest)
playground/__tests__/e2e/auth-login.spec.ts
[failure] 5-5: playground/tests/e2e/auth-login.spec.ts
Error: Playwright Test did not expect test.beforeEach() to be called here.
Most common reasons include:
- You are calling test.beforeEach() in a configuration file.
- You are calling test.beforeEach() in a file that is imported by the configuration file.
- You have two different versions of @playwright/test. This usually happens
when one of the dependencies in your package.json depends on @playwright/test.
❯ TestTypeImpl._currentSuite node_modules/.pnpm/[email protected]/node_modules/playwright/lib/common/testType.js:71:13
❯ TestTypeImpl._hook node_modules/.pnpm/[email protected]/node_modules/playwright/lib/common/testType.js:144:24
❯ Function.beforeEach node_modules/.pnpm/[email protected]/node_modules/playwright/lib/transform/transform.js:262:12
❯ playground/tests/e2e/auth-login.spec.ts:5:6
Additional comments not posted (41)
.gitpod.yml (1)
6-6: Clarify the purpose and impact of thedev:playscript.I noticed the command has been updated from
pnpm run devtopnpm run dev:play. Could you please provide more context about what thedev:playscript does and how it differs from thedevscript?Understanding the purpose and impact of this change will help in assessing whether it aligns with the PR objectives and if it has any potential side effects on the development workflow.
vitest.config.ts (1)
8-8: Verify existing tests with the new Happy DOM environment.The change from JSDOM to Happy DOM as the test environment is a good move towards better performance and compatibility. However, it's crucial to ensure that existing tests are not adversely affected by this change.
Please take the following actions:
- Review existing tests, especially those that rely on DOM manipulation or rendering, and make necessary adjustments to ensure compatibility with Happy DOM.
- Run the entire test suite with the new environment to verify that all tests pass without any unexpected behavior.
- If any tests fail or exhibit unusual behavior, investigate and address those issues before merging this change.
playground/__tests__/e2e/auth-login.spec.ts (3)
1-3: LGTM!The imports are correctly specified and follow the proper syntax. The custom
authLoginfunction is imported from the appropriate module.
9-14: LGTM!The test case is well-structured and follows the Playwright testing conventions. It correctly retrieves the page title and asserts that it contains the expected string.
16-19: Verify the implementation of theauthLoginfunction.The test case correctly calls the
authLoginfunction to verify the login functionality. However, the effectiveness of this test case relies on the correctness and reliability of theauthLoginfunction.Please ensure that the
authLoginfunction is implemented correctly and handles various scenarios, such as:
- Login with valid credentials
- Login with invalid credentials
- Error handling and appropriate error messages
- Edge cases and boundary conditions
You can use the following script to navigate to the
authLoginfunction and review its implementation:Verification successful
Retrying the search for the
authLoginfunction implementation.The previous script failed to recognize the
typescriptfile type. Please run the following script to search for theauthLoginfunction definition using the correct file type identifier.
Verify the source of the
authLoginfunction used in the test.To ensure that the
authLoginfunction in your testauth-login.spec.tsis correctly implemented and sourced, please run the following script to extract its import statement:
Review the implementation of the
authLoginfunction in the test utilities.To ensure that the
authLoginfunction used in your test is implemented correctly and handles all necessary scenarios, please run the following script to display its implementation:
Verified the implementation of the
authLoginfunction.The
authLoginfunction inplayground/__tests__/e2e/common/auth.tsis correctly implemented and handles the login process comprehensively, including captcha interactions and necessary assertions. The test case inauth-login.spec.tsappropriately utilizes this function to verify the login functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Navigate to the `authLogin` function and review its implementation. # Test: Search for the function definition. Expect: The function is defined in the `./common/auth` module. rg --type typescript -A 5 $'authLogin\('Length of output: 76
Script:
#!/bin/bash # Description: Retry searching for the `authLogin` function implementation with the correct file type. # Test: Search for the function definition using the correct TypeScript file type. Expect: The function is defined in the `./common/auth` module. rg --type ts -A 5 'authLogin\('Length of output: 2209
Script:
#!/bin/bash # Description: Extract the import statement of `authLogin` in the test file. rg 'import .*authLogin' playground/__tests__/e2e/auth-login.spec.tsLength of output: 112
Script:
#!/bin/bash # Description: Display the implementation of `authLogin` in the test utilities. cat playground/__tests__/e2e/common/auth.tsLength of output: 1760
packages/effects/common-ui/src/components/captcha/slider-captcha/slider-captcha-action.vue (1)
54-54: LGTM!The addition of the
nameattribute with the valuecaptcha-actionis a valid and descriptive choice. It can be used as a reference to the element for scripting or styling purposes, improving code readability and maintainability.internal/lint-configs/eslint-config/src/configs/node.ts (1)
27-27: LGTM!The addition of
'@playwright/test'to theallowModulesarray in then/no-extraneous-importrule configuration is consistent with the PR objective of introducing the Playwright testing framework. This change allows developers to import and use the@playwright/testmodule without triggering an ESLint error.playground/package.json (1)
23-26: LGTM! The addition of Playwright scripts enhances the project's testing capabilities.The introduction of the Playwright end-to-end testing framework and the associated scripts (
test:e2e,test:e2e-ui,test:e2e-codegen) aligns with the PR objective and brings several benefits to the project:
test:e2eenables running end-to-end tests using Playwright, ensuring the application functions as expected from a user's perspective.test:e2e-uiprovides an option to run the tests with a user interface, which can be helpful for debugging and visualizing the test execution.test:e2e-codegenallows generating code for Playwright tests, streamlining the test creation process and reducing the effort required to set up new tests.Overall, these additions enhance the project's testing capabilities and contribute to maintaining a high-quality codebase.
packages/effects/common-ui/src/ui/authentication/forget-password.vue (1)
98-98: Accessibility improvement!Adding the
aria-labelattribute with the value "submit" enhances the accessibility of the button for users relying on assistive technologies. It accurately conveys the purpose of the button, making the form more inclusive. Great work on improving the user experience!packages/effects/common-ui/src/ui/authentication/register.vue (1)
100-100: Accessibility Enhancement: LGTM!The addition of the
aria-labelattribute with the value "register" enhances the accessibility of the registration form by providing a clear and descriptive label for the submit button. This is particularly beneficial for users relying on assistive technologies, such as screen readers.The change aligns with the best practices for improving web accessibility and does not introduce any functional or visual changes to the button itself.
playground/playwright.config.ts (13)
15-21: LGTM!The
expecttimeout configuration is set to a reasonable value of 5000 milliseconds. This allows enough time for most conditions to be met while preventing tests from hanging indefinitely.
23-23: LGTM!The
forbidOnlyconfiguration is a good practice to prevent accidental commits of focused tests (test.only) to CI. It ensures that all tests are executed on CI and no tests are skipped unintentionally.
25-25: LGTM!The
outputDirconfiguration is set to a reasonable location (node_modules/.e2e/test-results/) to store test artifacts. It keeps the test artifacts separate from the source code and other project files.
27-74: LGTM!The
projectsconfiguration is set up with a 'chromium' project using the 'Desktop Chrome' device configuration. This is sufficient for initial testing. The commented-out projects provide options for testing on additional browsers and devices in the future. It's a good practice to start with a single browser for faster test execution and add more browsers as needed.
76-79: LGTM!The
reporterconfiguration includes a 'list' reporter for a concise summary of test results in the console and an 'html' reporter for generating an HTML report in the specified output folder. Using multiple reporters is a good practice for comprehensive test reporting.
81-81: LGTM!The
retriesconfiguration is set to retry failed tests twice on CI and zero times locally. This is a good practice to account for flaky tests or temporary environment issues on CI while preventing unnecessary retries during local development. The retry count of 2 is reasonable to balance test stability and execution time.
82-82: LGTM!The
testDirconfiguration is set to./__tests__/e2e, which is a common convention for storing end-to-end tests in a separate directory. It keeps the end-to-end tests separate from other types of tests (e.g., unit tests, integration tests).
84-84: LGTM!The
timeoutconfiguration is set to 30 seconds (30 * 1000 milliseconds), which is reasonable for most end-to-end test scenarios. It allows enough time for tests to complete while preventing tests from running indefinitely. If tests consistently exceed this timeout, it may indicate a performance issue or a need to optimize the tests.
86-95: LGTM!The
useconfiguration specifies shared settings for all the projects. TheactionTimeoutis set to 0, which means there is no limit on the time each action can take. This may need to be adjusted based on the specific requirements of the tests. ThebaseURLis set tohttp://localhost:5555, which is appropriate for running tests locally. Theheadlessmode is enabled only on CI systems, which is a good practice for faster execution on CI. Thetraceoption is set to'retain-on-failure', which is useful for debugging and understanding the cause of test failures.
98-102: LGTM!The
webServerconfiguration specifies the settings for running a local development server before starting the tests. Thecommandproperty usespnpm preview --port 5555for CI andpnpm devfor local environments, ensuring that the appropriate command is used based on the environment. Theportproperty is set to5555, which matches thebaseURLconfiguration. ThereuseExistingServerproperty is set totruefor local environments, allowing it to reuse an existing server if available, which can speed up test execution during local development. For CI,reuseExistingServeris set tofalse, ensuring that a fresh server instance is started for each test run.
105-105: LGTM!The
workersconfiguration specifies the number of concurrent worker processes for running tests. Setting the number of workers to 1 on CI ensures that tests are run sequentially, which can be useful for debugging and avoiding resource contention on CI machines. Leaving the number of workers undefined for local environments allows Playwright to determine the optimal number of workers based on the available system resources, which can speed up test execution during development.
14-14: LGTM!The
configobject is correctly declared as a constant of typePlaywrightTestConfig, which is imported from the@playwright/testpackage. This ensures type safety and autocompletion for the configuration properties and prevents accidental modifications to the configuration.
108-108: LGTM!The
configobject is correctly exported as the default export at the end of the file, after the configuration object is fully defined. This allows the configuration to be easily imported and usepackage.json (5)
57-57: LGTM!The change to the
reinstallscript looks good. Usingpnpm installis a standard way to install dependencies.
58-58: LGTM!The modification to the
test:unitscript looks good. Including therun --domoption forvitestis necessary for running tests that interact with the DOM, which aligns with the introduction of Playwright for e2e testing.
59-59: LGTM!The addition of the
test:e2escript looks good. Usingturbofor running end-to-end tests is a valid approach and aligns with the PR objective of introducing Playwright for e2e testing.
66-66: LGTM!The changes to the dependencies look good. Removing JSDOM-related dependencies and adding Playwright-related dependencies is consistent with the shift towards using Playwright for e2e testing, as mentioned in the PR objectives.
Also applies to: 83-83, 87-87
117-117: LGTM!The removal of the
playwrightdependency from theneverBuiltDependenciessection looks good. Building theplaywrightdependency is likely required for running e2e tests, which aligns with the introduction of Playwright for e2e testing.internal/lint-configs/eslint-config/src/custom-config.ts (1)
137-144: LGTM! The changes align with the PR objective.The new ESLint configuration rule specifically targets Playwright configuration files and disables certain rules to provide flexibility in writing those files. This aligns with the PR objective of introducing the Playwright end-to-end testing framework to the project.
The changes are isolated to Playwright configuration files and do not affect other parts of the codebase.
packages/effects/common-ui/src/ui/authentication/login.vue (1)
132-132: LGTM!The addition of the
aria-labelattribute with the value "login" enhances the accessibility of the button element. This change allows assistive technologies to accurately convey the purpose of the button to users.pnpm-workspace.yaml (5)
32-32: LGTM!The addition of the
@playwright/testpackage with version^1.47.2aligns with the PR objectives of introducing the Playwright e2e testing framework. The version constraint allows for minor and patch updates while ensuring compatibility.
106-106: Approve the replacement ofjsdomwithhappy-dom.The addition of the
happy-dompackage with version^15.7.4and removal of thejsdompackage suggests a replacement of the DOM testing library.happy-domis a lightweight alternative tojsdomfor testing web applications in Node.js environments. The version constraint allows for minor and patch updates while ensuring compatibility.
124-124: LGTM!The addition of the
playwrightpackage with version^1.47.2aligns with the PR objectives of introducing the Playwright e2e testing framework. The version constraint allows for minor and patch updates while ensuring compatibility.
Line range hint
1-1: Approve the removal of@types/jsdom.The removal of the
@types/jsdompackage is consistent with the removal of thejsdompackage. Sincejsdomhas been replaced withhappy-dom, the corresponding type definitions are no longer needed.
Line range hint
1-1: Approve the removal ofjsdom.The removal of the
jsdompackage and addition of thehappy-dompackage suggests a replacement of the DOM testing library.happy-domis a lightweight alternative tojsdomfor testing web applications in Node.js environments.docs/src/guide/essentials/development.md (2)
108-108: Verify ifpnpm bootstrapis required for the project setup.The change simplifies the reinstallation process by directly using
pnpm install. This should be sufficient if the project doesn't use a monorepo structure or require package linking.However, if the project uses a monorepo setup and requires package linking, removing
pnpm bootstrapcould potentially break the setup. In that case, please clarify the need for this change.
110-110: LGTM!The addition of the
--domflag to thevitestcommand enhances the unit testing capabilities by enabling tests that require a browser-like environment. This is useful for testing components that interact with the DOM.README.ja-JP.md (1)
136-138: LGTM!The addition of the "スター歴史" (Star History) section with the image link to display the repository's star history chart is a valuable enhancement to the README file. It provides insights into the project's popularity and growth over time.
The section is positioned appropriately within the document flow, and the image link syntax is correct, including the repository name to generate the chart dynamically.
docs/src/en/guide/essentials/development.md (2)
108-108: LGTM!The change from
pnpm bootstraptopnpm installin thereinstallscript is a minor modification that should not have any significant impact on the functionality. The removal of thebootstrapscript is consistent with this change.
110-110: LGTM!The addition of the
runcommand and the--domflag in thetest:unitscript specifies that the tests should be run in a DOM environment. This is important for testing components that interact with the DOM.playground/__tests__/e2e/common/auth.ts (1)
45-45: Verify the selector for the login buttonEnsure that the selector
await page.getByRole('button', { name: 'login' }).click();correctly identifies the login button. If the button's accessible name differs (due to localization or missingaria-label), this selector might fail.
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes
New Features
Accessibility Improvements
aria-labelattributes.Development Environment Updates
Testing Enhancements