-
Notifications
You must be signed in to change notification settings - Fork 1k
ci(js-sdk): install Playwright Chromium without --with-deps
#1698
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | |||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -96,6 +96,17 @@ jobs: | ||||||||||
| path: ${{ matrix.os == 'windows-latest' && '~/AppData/Local/ms-playwright' || '~/.cache/ms-playwright' }} | |||||||||||
| key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} | |||||||||||
|
|
|||||||||||
| # `--with-deps` is deliberately omitted. It shells out to apt on Linux and | |||||||||||
| # to a DISM Media Foundation enable on Windows on every run, cache hit or | |||||||||||
| # not, which cost ~21m and ~4m30s respectively. The runner images already | |||||||||||
| # ship every shared library Chromium needs; the only packages | |||||||||||
| # `--with-deps` pulled in were CJK/Cyrillic fonts that the headless | |||||||||||
| # `browser` project never renders. Without it this step is a no-op once | |||||||||||
| # the cache above hits. | |||||||||||
| - name: Install Playwright Chromium | |||||||||||
| if: matrix.runtime == 'node' | |||||||||||
| run: pnpm run playwright:install | |||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional, and only if there's no plan to run headed or full-Chromium browser tests later:
That also shrinks the |
|||||||||||
|
|
|||||||||||
| # The unit bundle test and the Cloudflare deploy config fail in CI when | |||||||||||
| # the build output is missing. | |||||||||||
| - name: Test build | |||||||||||
|
|
|||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ | |
| "generate:volume-api": "openapi-typescript ../../spec/openapi-volumecontent.yml -x api_key --array-length --alphabetize --output src/volume/schema.gen.ts", | ||
| "generate:mcp": "json2ts -i ./../../spec/mcp-server.json -o src/sandbox/mcp.d.ts --unreachableDefinitions --style.singleQuote --no-style.semi", | ||
| "check-deps": "knip", | ||
| "pretest": "npx playwright install --with-deps chromium", | ||
| "playwright:install": "playwright install chromium", | ||
|
mishushakov marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing in the repo documents the new prerequisite. What a fresh clone now gets from that documented command (reproduced with Exit code 1, and Playwright's remedy line says |
||
| "test:bun": "bunx --bun vitest run --project unit --project connectionConfig --project template", | ||
| "test:cf": "vitest run --config tests/runtimes/cloudflare/vitest.config.mts", | ||
| "test:cf:deploy": "vitest run --config tests/runtimes/cloudflare-deploy/vitest.config.mts", | ||
|
|
||
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.
The
~21mfigure is the tail of a wide distribution rather than the steady cost, and this comment is what the next reader will trust. Measuredpretestduration on the fiveTest JS SDKruns before this branch (firstplaywright install --with-depslog line to the first> vitest runline):So the structural win is Windows, where the cost is paid on every run regardless of the cache, while Linux was a lottery between ~15s and ~21m. Something like "a deterministic ~3-4m on Windows, and anywhere from ~15s to ~21m on Linux depending on the Azure mirror" would age better than a single pair of numbers.
Small factual nit in the same sentence: of the 9 packages
--with-depsactually installed,fonts-freefont-ttf,fonts-unifont,fonts-tlwg-loma-otf(Thai) andxfonts-{utils,encodings,scalable}aren't CJK/Cyrillic. The conclusion is untouched — the one browser test renders the Latin stringHello World— but "font packages the headlessbrowserproject never renders" is the accurate version.