-
Notifications
You must be signed in to change notification settings - Fork 6.1k
chore: upgrade to Playwright 1.12 with its new test-runner #3590
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: upgrade to Playwright 1.12 with its new test-runner #3590
Conversation
@@ -413,7 +412,7 @@ export const isObject = <T extends object>(obj: T): obj is T => { | |||
* we don't have to set up a `vs` alias to be able to import with types (since | |||
* the alternative is to directly import from `out`). | |||
*/ | |||
const enum CharCode { | |||
enum CharCode { |
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.
Playwright's test-runner uses babel-plugin-transform-typescript which does not support const enums. See here.
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.
Awesome! 🎉
@@ -6,7 +6,7 @@ main() { | |||
cd test | |||
# We set these environment variables because they're used in the e2e tests | |||
# they don't have to be these values, but these are the defaults | |||
PASSWORD=e45432jklfdsab CODE_SERVER_ADDRESS=http://localhost:8080 yarn folio --config=config.ts --reporter=list "$@" | |||
PASSWORD=e45432jklfdsab CODE_SERVER_ADDRESS=http://localhost:8080 yarn 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.
I removed the two parameters there, to have all Playwright related configuration in its config file. Also playwright.config.ts
is easier to understand than config.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.
Agreed! playwright.config.ts
is easier to understand. Good call!
import { CodeServer } from "./models/CodeServer" | ||
|
||
export const test = base.extend<{ codeServerPage: CodeServer }>({ | ||
codeServerPage: async ({ page }, use) => { |
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.
This is a custom test-fixture. This means for every test a new one gets created. Since its using the page, which uses a context, Playwright does automatically take care of closing the context.
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.
Woah, that is super cool! That's fantastic! 🎉
Thanks for making your first contribution! 🙂 |
Codecov Report
@@ Coverage Diff @@
## main #3590 +/- ##
==========================================
+ Coverage 60.53% 60.67% +0.13%
==========================================
Files 35 35
Lines 1784 1790 +6
Branches 403 404 +1
==========================================
+ Hits 1080 1086 +6
Misses 562 562
Partials 142 142
Continue to review full report at Codecov.
|
Thanks a lot for your contribution! 😄 |
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.
Wow! Thank you so much for the surprise contribution @mxschmitt 🎉🎉
You've cleaned everything up and also improved our e2e test setup!
Re: CHANGELOG -> no need to update unless you want to! We copy those notes into the release notes and give credit to contributors. Totally up to you! 😄
@@ -6,7 +6,7 @@ main() { | |||
cd test | |||
# We set these environment variables because they're used in the e2e tests | |||
# they don't have to be these values, but these are the defaults | |||
PASSWORD=e45432jklfdsab CODE_SERVER_ADDRESS=http://localhost:8080 yarn folio --config=config.ts --reporter=list "$@" | |||
PASSWORD=e45432jklfdsab CODE_SERVER_ADDRESS=http://localhost:8080 yarn 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.
Agreed! playwright.config.ts
is easier to understand. Good call!
@@ -413,7 +412,7 @@ export const isObject = <T extends object>(obj: T): obj is T => { | |||
* we don't have to set up a `vs` alias to be able to import with types (since | |||
* the alternative is to directly import from `out`). | |||
*/ | |||
const enum CharCode { | |||
enum CharCode { |
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.
Awesome! 🎉
import { CodeServer } from "./models/CodeServer" | ||
|
||
export const test = base.extend<{ codeServerPage: CodeServer }>({ | ||
codeServerPage: async ({ page }, use) => { |
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.
Woah, that is super cool! That's fantastic! 🎉
Does it require changes in the changelog?
This upgrades it to the new Playwright test-runner.