Skip to content

Conversation

@Tyriar
Copy link
Member

@Tyriar Tyriar commented Sep 3, 2021

Fixes #3446
Fixes #3326

Changes summary:

  • Use playwright test runner instead of plain playwright with mocha
  • Move from deprecated equals to strictEquals
  • Use JSHandle for most things instead of raw page.evaluate(string) to strongly type renderer-side test code
    • The main thing to be careful with here is not to use async/await inside the page callback as the compiled __awaiter won't be on the page
  • Add missing test for onBinary event
  • Use () => {} for all tests
  • Not using the string for page.evaluate allows us to avoid the \\ hack
  • Pretty much all write/term calls now use await and can guarantee the action is complete (eg. writes are parsed) by the time it returns in node, so we can avoid using pollFor for most tests
  • Events are now proxied, making it more convenient to test with

Remaining:

  • Migrate addon tests over
  • Delete old .api tests
  • Get all tests passing in CI

@Tyriar Tyriar added this to the 4.14.0 milestone Sep 3, 2021
@Tyriar Tyriar self-assigned this Sep 3, 2021
displayName: 'Integration tests (Chromium)'
- script: xvfb-run --auto-servernum -- bash -c "yarn test-api-firefox --headless --forbid-only"
displayName: 'Integration tests (Firefox)'
- script: yarn test-playwright --forbid-only
Copy link

@mxschmitt mxschmitt Sep 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: In the config you can add: forbidOnly: !!process.env.CI that's how we do it, then you can drop the --forbid-only CLI argument everywhere.

@Tyriar Tyriar removed this from the 4.17.0 milestone Feb 3, 2022
- script: xvfb-run --auto-servernum -- bash -c "yarn test-api-firefox --headless --forbid-only"
displayName: 'Integration tests (Firefox)'
- script: npx playwright install
displayName: 'Install PlayWright browsers'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
displayName: 'Install PlayWright browsers'
displayName: 'Install Playwright browsers'

It's Playwright 🙂

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mxschmitt now that you're here, any idea how to solve this? Installing didn't see to fix:

  6) [Firefox Stable] › ../../test/playwright/CharWidth.test.ts:20:5 › CharWidth Integration Tests › getStringCellWidth › ASCII chars 

    browserType.launch: Executable doesn't exist at /home/vsts/.cache/ms-playwright/firefox-1335/firefox/firefox
    ╔═════════════════════════════════════════════════════════════════════════╗
    ║ Looks like Playwright Test or Playwright was just installed or updated. ║
    ║ Please run the following command to download new browsers:              ║
    ║                                                                         ║
    ║     npx playwright install                                              ║
    ║                                                                         ║
    ║ <3 Playwright Team                                                      ║
    ╚═════════════════════════════════════════════════════════════════════════╝

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely because you have playwright and @playwright/test inside your package.json with different versions. I suggest to remove playwright and only have @playwright/test with the latest version.

(everything from playwright is also exported from @playwright/test node/package-wise)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another idea is since you are using yarn, to use yarn run playwright install instead.

@Tyriar Tyriar added the reference A closed issue/pr that is expected to be useful later as a reference label Aug 1, 2023
@Tyriar
Copy link
Member Author

Tyriar commented Aug 1, 2023

This is too stale, it's available for reference when we want to try move to @playwright/test in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reference A closed issue/pr that is expected to be useful later as a reference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adopt @playwright/test and use JSHandle integration test script seriously broken

3 participants