Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/js_sdk_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The ~21m figure is the tail of a wide distribution rather than the steady cost, and this comment is what the next reader will trust. Measured pretest duration on the five Test JS SDK runs before this branch (first playwright install --with-deps log line to the first > vitest run line):

  • Linux: 16s, 29s, 190s, 232s, 811s — entirely mirror-dependent (one run fetched 18.4 MB at 139 kB/s, the cited one at 16.1 kB/s).
  • Windows: 184s, 192s, 194s, 204s, 252s — the DISM Media Foundation enable really is a deterministic ~3-4m every run.

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-deps actually installed, fonts-freefont-ttf, fonts-unifont, fonts-tlwg-loma-otf (Thai) and xfonts-{utils,encodings,scalable} aren't CJK/Cyrillic. The conclusion is untouched — the one browser test renders the Latin string Hello World — but "font packages the headless browser project never renders" is the accurate version.

# 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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: playwright install --only-shell chromium is all this suite needs. The project launches chromium_headless_shell, not full Chromium — I moved ~/.cache/ms-playwright/chromium-1193 out of the way and the project still passed (1 passed). Measured on the same machine:

command cold time on disk
playwright install chromium 6.5s 920 MB (594 MB Chromium + 321 MB headless shell + 4.9 MB ffmpeg)
playwright install --only-shell chromium 2.7s 326 MB

That also shrinks the actions/cache artifact roughly 3x, which is the part paid on every run (the restore was 5.1s in the run I sampled) rather than only on a version bump. Note it would invalidate the existing cache entries once, since the key is keyed on the Playwright version alone.


# The unit bundle test and the Cloudflare deploy config fail in CI when
# the build output is missing.
- name: Test build
Expand Down
2 changes: 1 addition & 1 deletion packages/js-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Comment thread
mishushakov marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing in the repo documents the new prerequisite. CONTRIBUTING.md is 2 lines and DEV.md is 3 (neither mentions tests), so the only in-repo instruction is AGENTS.md/CLAUDE.md's "Create or update tests covering affected codepaths and run them using pnpm run test" — and root pnpm run test is pnpm test --recursive --if-present, which reaches js-sdk's vitest run including the browser project.

What a fresh clone now gets from that documented command (reproduced with PLAYWRIGHT_BROWSERS_PATH pointed at an empty dir) is not a clean skip:

⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.
Error: browserType.launch: Executable doesn't exist at .../chromium_headless_shell-1193/chrome-linux/headless_shell
║     pnpm exec playwright install                                        ║

Exit code 1, and Playwright's remedy line says pnpm exec playwright install, not pnpm run playwright:install. Two ways to close it: one line in AGENTS.md/CLAUDE.md next to the pnpm run test instruction, or #1609's pretest:browser shape, which keeps the install automatic for whoever runs the browser suite without putting it back on the default pnpm test.

"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",
Expand Down
Loading