-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(dark-light-modes): reusable test code #934
Conversation
Playwright test resultsDetails 790 tests across 134 suites Skipped testssrc/components/table/table/test/expandable-row-autocollapse/expandable-row-autocollapse.e2e.ts › tds-table-expandable-row-autoCollapse › NEEDS FIXING: expanding one row collapses the others when autoCollapse is true |
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going through all of the new screenshots PRs that have been posted using this reusable code as a base, it seems like there are still residual tests in light mode (specifically primary light mode if the component has mode variants) that produce duplicates of screenshots. This has happened all throughout the PRs. Before moving forward with this idea I think it would be good to align on how we want to tackle this problem, if we want to remove the original tests entirely or if this script should be tweaked to not produce results in (primary) light mode. Or, if there is another way to go about this and still get the result of dark mode tests. Having a larger discussion within the dev group sounds like a good idea to me.
451a8ac
to
e3b8ced
Compare
b20d19b
to
a9db16b
Compare
* test(tooltip): use reusable test code for lightmode and darkmode screenshot tests * test(tooltip): move non-screenshot tests to not run for all style configurations
* test(toggle): add screenshot tests using reusable test code * test(toggle): move non-screenshot tests to not run for all style configurations * test(toggle): remove old screenshots
…tests, with reusable test code (#942)
a9db16b
to
3852406
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes in PR are touching too large amount of files to provide detailed code review. However, idea of having configuration file and less lines of code to test both dark and light mode and its submodes looks good to me.
Looking in some components we could further optimize this by just taking screenshots using "beforeEach" trick done here and moving other logic outside - for example checking if disabled cursor is disabled. That would be a step in next improvement.
Overall great work!
I have updated the title of PR, just a bit shorter. |
I think the idea was that everything in this PR has been merged from other PRs that has been approved, so I don't think any of the files in this PR are necessary to review as they have already been approved, except for testConfiguration.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have went through every sub-pr before they where merge into this larger PR, and it looked good to me.
Quality Gate passedIssues Measures |
Describe pull-request
This PR introduces reusable test code, that allows for easily generating tests for any component, based on their existing tests and dynamically modifying mode (lightmode, darkmode) and mode variant (primary, secondary).
In this PR I have also applied this dynamic test for almost all components, so now they have screenshot tests for modes and mode variants.
Issue Linking:
The following tickets are addressed in this PR
How to test
Provide detailed steps for testing, including any necessary setup.
npm run test
Checklist before submission
npm run build-all
without errorsSuggested test steps
Screenshots
None
Additional context
How to use the reusable test code
In .e2e.ts test files, extract the tests within the
test.describe.parallel
that have atoHaveScreenshot
and put them below one of the followingor
and make sure to close the functions with curly braces and parentheses. Also insert code from existing (if there is any)
test.beforeEach
to this one, and also removing thepage.goto
lines in every test in the file.