Skip to content

[dev]: switch from jest-playwright to playwright test #3115

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

Closed
jsjoeio opened this issue Apr 12, 2021 · 8 comments · Fixed by #3133
Closed

[dev]: switch from jest-playwright to playwright test #3115

jsjoeio opened this issue Apr 12, 2021 · 8 comments · Fixed by #3133
Assignees
Labels
enhancement Some improvement that isn't a feature testing Anything related to testing
Milestone

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Apr 12, 2021

The Playwright team has reached out and suggested we use playwright-test instead of jest-playright as our test runner. The APIs look similar but they offered to help/guide the migration.

Hi @jsjoeio! Would you consider trying playwright-test instead of jest-playwright? We at Playwright team believe that with the focused test runner it is possible to deliver better experience for e2e tests. If you are open to this, I can guide/help with the migration, as well as address the feedback on the test runner side 😄 Let me know if you are interested!

Originally posted: #3016 (comment)

Note: I'll probably start on this sometime this week and post thoughts and ask questions as I go here.

@jsjoeio jsjoeio added the enhancement Some improvement that isn't a feature label Apr 12, 2021
@jsjoeio jsjoeio added this to the v3.9.4 milestone Apr 12, 2021
@jsjoeio jsjoeio self-assigned this Apr 12, 2021
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 13, 2021

Migration Process WIP

  • add @playwright/test and remove jest-playwright
  • write test and run to ensure working as expected
  • migrate jest.e2e.config.ts to config.ts for playwright-test
  • fix globalSetup
  • migrate e2e script
  • migrate tests over
    • browser.test.ts
    • globalSetup.test.ts
    • login.test.ts
    • loginPage.test.ts
    • logout.test.ts
    • openHelpAbout.test.ts
    • update .gitignore with /test-results
    • update ci to upload /test-results as artifact

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 13, 2021

Feedback

  • docs don't show alternative to running with npx
  • cross-browser API feels awkward
    test.runWith(new ChromiumEnv(options), { tag: 'chromium' });
    test.runWith(new FirefoxEnv(options), { tag: 'firefox' });
    test.runWith(new WebKitEnv(options), { tag: 'webkit' });
    • I would rather do something like
    const options = {
       browsers: ["chromium", "firefox", "webkit"],
    };
  • like that the test runner has a retry for failing tests!
  • docs only show npm install. show yarn too
  • config shouldn't be required to run test runner.
    npx folio --test-match "foo.spec.ts"
    Error: Configuration file not found. Either pass --config, or create folio.config.(js|ts) file
    • README.md doesn't mention config being required
  • test-match doesn't exist error? but --help said it was a valid flag.
  • test-runner output tells you nothing compare to jest-playwright :(
    Running 1 test using 1 worker
     1 passed (6s)
    • How do I know what test passed?
    • How many tests were there in total?
    • What browser was used?
  • test runner flags aren't intuitive: --workers=5 does 5 mean something? Is it 5 workers (guessing yes)? Why not use --runInParallel?
  • like the retain-on-failure option
  • options example should have type annotation PlaywrightOptions
  • not sure what to put in setConfig({}) vs options. feels awkward
  • test.describe feels awkward but guessing it's because of potential namespace collisions with Jest/Mocha?
  • types not working out of the box? i see a .d.ts file included when installing the package, but not sure how to reference in test file. How to reference types? microsoft/playwright-test#239
  • browserName global? jest-playwright provided a global to get the name of the currently running browser.
  • no global setup? Global setup phase microsoft/playwright-test#177

UPDATE:
Additional feedback:

  • 'const' enums are not supported. getting this when importing a variable from an external file that uses an enum.
  • Hmm...Seems like Playwright is reading my test files before running the globalSetup defined in my config? 1. define process.env.STORAGE in globalSetup then use in a foo.spec.ts
  • PlaywrightTestOptions type isn't exported which means I have to use any?
  • wish the docs explained how to resetContext before running a test (ensure cookies are cleared)
  • firefox is failing on login.test.ts like before ([BUG] Firefox browser.close throws Protocol error (Browser.removeBrowserContext): Browser closed microsoft/playwright#6020 (comment))
    • workaround is to use dependency resolution
    • wish @playwright/test@next used playwright@next under the hood. (maybe it does already and I just did it wrong)
  • wish docs explained how the three play together — playwright, playwright-test and folio (folio is kinda just introduced in playwright-test and doesn't do a great job explaining how the two work together)
  • looks like folio may provide a way to create different environments for specific tests. this may be really helpful. we need a way to set different environment variables for different tests. i.e. run one test with PASSWORD=password123 then run another where we set AUTH=none
  • config option to choose name of video directory - I want them to be stored under /videos
  • tell me how to fix/diagnose a slow test
    Slow test: openHelpAbout.test.ts (17s)

@dgozman
Copy link

dgozman commented Apr 13, 2021

Thank you for the feedback! I have a few comments/questions, but we'll address most of the items.

  • The config file is a deliberate decision. I understand that it might feel unfamiliar, but some documentation is already there and we'll expand it. I hope that it will feel much better once you'll get to use any flexibility that it provides.

  • We are not providing any globals, instead you should be able to import whatever you need - this is a deliberate decision as well. This allows us to isolate tests, run them separately or together, rerun flaky failures and more.

  • How do I know what test passed?

What specifically do you want to see? All the tests with green checkmarks? Try running as npx folio --reporter=list. There are a few built-in reporters available, and the default one (--reporter=line) is good for large test suites - for example we use it for our test runs with 1800 tests, where spamming the terminal is not an option. There is an initial example about using reporters, feel free to play with it.

  • How many tests were there in total?

It says Running 1 test...

  • What browser was used?

This depends on your config - most likely all three browsers. The test name should be prefixed with something like [chromium] when it runs under Chromium. Do you want to see just the browser or something else?

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 14, 2021

Thanks for the response @dgozman!

we'll expand it. I hope that it will feel much better once you'll get to use any flexibility that it provides

I think you're right. I'm already starting to see some of the benefits of the flexibility provided by the config file.

This allows us to isolate tests, run them separately or together, rerun flaky failures and more.

Awesome! I figured out how to get the browserName luckily, but some docs on that would be nice.

Try running as npx folio --reporter=list

That worked! Thanks!

What specifically do you want to see?

Honestly, I wish folio had --reporter=jest so that I could get a jest-like reporter out of the box. I like the green checkmarks and the PASS.

How many tests were there in total?

Sorry, this was vague. Basically, I wish it was more like Jest.

Test Suites: 1 skipped, 11 passed, 11 of 12 total

The test name should be prefixed with something like

Oh yeah, that's what I'm looking for, but I'm not seeing it.

image

P.S. I added more feedback to my comment above under UPDATE

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Apr 15, 2021

@dgozman we've migrated over! Thanks so much for your help.

@dgozman
Copy link

dgozman commented Apr 15, 2021

@jsjoeio Great! Thank you for your feedback. We'll keep addressing it, and I will comment here with an update.

@jsjoeio jsjoeio added the testing Anything related to testing label May 14, 2021
@dgozman
Copy link

dgozman commented May 25, 2021

@jsjoeio Thank you for trying the previous experimental version, your feedback helped a lot! We've been working on the next version, and a new alpha is ready. If you enjoy @playwright/test so far, you should give this one a try. The new alpha is versioned 0.1112.0-alpha1.

Here is a preview of the upcoming documentation and migration guide. However, since you have tried the experimental version (thank you!), migration will be a bit different for you:

  • Configuration file format has changed, please see an example in the documentation above. Overall, it should be a bit easier now. More details about configuration.
  • Instead of passing options as a second parameter to the test('title', options, func), you should now call test.use(options) to declare options for a file or a describe block.
  • globalSetup / globalTeardown now lives in a separate file. More details about global hooks.
  • If you have started using custom environments, you'll have to switch to fixtures. That's somewhat involved, and reading Folio documentation is necessary.

Let me know if you are willing to try this, and send me all the questions.


Answering some of the older questions, based on the new version:

  • 'const' enums are not supported. getting this when importing a variable from an external file that uses an enum.

    I guess this is a babel thing. We have bumped the babel version, let me know if this still happens.

  • wish the docs explained how to resetContext before running a test (ensure cookies are cleared)

    This is done by default. Each test is isolated from others.

  • config option to choose name of video directory - I want them to be stored under /videos

    This is not done yet. You should get videos per test in test-results/test-file-test-name directories.

  • docs only show npm install. show yarn too

    Not done yet.

  • Honestly, I wish folio had --reporter=jest so that I could get a jest-like reporter out of the box. I like the green checkmarks and the PASS.

    No significant progress on reporters, but we'll focus on them soon.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented May 25, 2021

Awesome! Thanks so much for the reply @dgozman - I'll open a separate issue to track upgrading to the new alpha version and tag you with questions after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature testing Anything related to testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants