Skip to content
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

Replace jest-puppeteer with Playwright #1884

Merged
merged 3 commits into from
Nov 6, 2024
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Nov 6, 2024

Description of proposed changes

Advantages of the new Playwright setup:

  1. It auto-detects whether a server is already running (no need for two separate npm scripts)
  2. Browser installation is done in a separate command. Previously, it was done on installation of puppeteer which became difficult to debug when it timed out on remote build environments.
  3. It requires much fewer dependencies (see diff of package-lock.json)

Related issue(s)

Checklist

Usage was removed in "Remove integration tests" (2047696).
@victorlin victorlin self-assigned this Nov 6, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-smoke-hnkmwg November 6, 2024 20:11 Inactive
Advantages of the new Playwright setup:

1. It auto-detects whether a server is already running (no need for two
   separate npm scripts)
2. Browser installation is done in a separate command. Previously, it
   was done on installation of puppeteer which became difficult to debug
   when it timed out on remote build environments.
3. It requires much fewer dependencies (see diff of package-lock.json)
The default parallelization of Playwright tests will parallelize across
multiple test files, but not within each file. There is currently only
one test file, so it makes sense to parallelize within the file.

<https://playwright.dev/docs/test-parallel#parallelize-tests-in-a-single-file>
@victorlin victorlin temporarily deployed to auspice-victorlin-smoke-hnkmwg November 6, 2024 20:16 Inactive
@victorlin
Copy link
Member Author

Jobs using npm ci without --ignore-scripts (build, smoke-test) now run in ~1m30s compared to the previous ~6m.

@victorlin victorlin marked this pull request as ready for review November 6, 2024 20:22
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Haven't tested thoroughly and it's been a while since I delved into puppeteer / smoke-testing in Auspice but this looks like a good move and CI looks good.

@victorlin victorlin merged commit 4e1ed7a into master Nov 6, 2024
25 checks passed
@victorlin victorlin deleted the victorlin/smoke-tests branch November 6, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI timing out during puppeteer browser download
3 participants