-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: refactor e2e tests to playwright #5080
base: master
Are you sure you want to change the base?
chore: refactor e2e tests to playwright #5080
Conversation
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.
Ideally, we need to rewrite existing tests on playwright
+ some refactor our tests, because we have a lot of code duplication
@alexander-akait thanks for your comment, for participating in the gsoc-2024, is this PR a valid patch requirements? if not could you introduce an issue which I can start working on? I'm a master student and a junior developer. regarding the proposal, is there anything specific things to be mentioned on or do you have a template from the previous years? also, if there is a discord of slack channel which I can share the draft version of proposal and receive early feedback, I appreciate it. |
/cc @evenstensberg |
I think this should work for a gsoc project. (90h project) |
Yes, I agree. |
WIP: will refactors tests one by one and rename the e2e-playwright folder to e2e after its done
WIP: will rename the test and clean it up after i received a feedback from maintainers
WIP: will refactors tests one by one and rename the e2e-playwright folder to e2e after its done
package.json
Outdated
@@ -41,6 +41,7 @@ | |||
"test:coverage": "npm run test:only -- --coverage", | |||
"test:watch": "npm run test:coverage --watch", | |||
"test": "npm run test:coverage", | |||
"test:e2e": "npx playwright test", |
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.
"test:e2e": "npx playwright test", | |
"test:e2e": "playwright test", |
const isCI = process.env.CI === "true"; | ||
|
||
/** @type { import('@playwright/test').PlaywrightTestConfig} */ | ||
module.exports = { |
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.
Ideally you use defineConfig + '// @ts-check'
testIgnore: "**/*.ignore.*", | ||
testDir: "./test/e2e", | ||
snapshotPathTemplate: "./test/e2e/__snapshots__/{testFilePath}/{arg}{ext}", | ||
fullyParallel: false, |
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.
Ideally you enable fullyParallel - didn't it work with having it enabled? Might be a potential follow-up.
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.
yes, it didn't work at first since we need to set ports, we planed to benefit from it later on.
playwright.config.js
Outdated
name: "chromium", | ||
use: { | ||
browserName: "chromium", | ||
launchOptions: { |
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.
Since you set ignoreHTTPSErrors
in line 26, you can remove the launchOptions
lock.
test/helpers/playwright-test.js
Outdated
], | ||
}); | ||
|
||
module.exports = { test: mergeTests(customTest) }; |
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.
in theory no need to merge
module.exports = { test: mergeTests(customTest) }; | |
module.exports = { test: customTest }; |
…r-to-playwright # Conflicts: # package-lock.json
c7510cf
to
3105fe8
Compare
Final Report GSoC 2024
Project
The project has been to substitute Puppeteer framework with MS Playwright in webpack-dev-server project and refactor tests afterward.
I have set up Playwright and configured it in a way to be similar to the existing code, I have extended the default
test
object and added a custom matcher for snapshot comparison of objects and arrays. Since previously Jest was used as Puppeteer's test runner and Playwright has its own runner, I have added Sinon.js as a mock / stubbing library.Tests are ran over Linux, Mac and Windows using node 18, 20 and 22. The code coverage is calculated using
nyc
andinstanbul
and it's babel plugin and pushed to CodeCov.Constraints and Future Improvement
Since by using the Playwright we could benefit from its flaky test marking mechanism, some tests are labeled as flaky which we may want to refactor them later.
Additionally, some tests were failing which I decided to do a root-cause analysis and figured out that by replacing
default-gateway
npm package, it could pass them successfully. (link to pr)Thank you
I want to sincerely thank Webpack team member @evenstensberg for taking the time to review my proposal for the project in advance. Special thanks to my mentors @snitin315 and @alexander-akait which helped me when I ran into any issues or had questions.
WIP: will refactors tests one by one and rename the e2e-playwright folder to e2e after its done
For Bugs and Features; did you add new tests?
Motivation / Use-Case
It will introduce playwright in the project to replace the existing puppeteer end to end framework.
Breaking Changes
Additional Info