Skip to content

test: exclude screenshot-gen from default E2E run; wire pdf-server tests#537

Merged
ochafik merged 1 commit intomainfrom
worktree-agent-a06f8386
Mar 6, 2026
Merged

test: exclude screenshot-gen from default E2E run; wire pdf-server tests#537
ochafik merged 1 commit intomainfrom
worktree-agent-a06f8386

Conversation

@ochafik
Copy link
Contributor

@ochafik ochafik commented Mar 6, 2026

Bug 1: Screenshot-generation spec runs on every CI E2E invocation

tests/e2e/generate-grid-screenshots.spec.ts is meant to be run only via npm run generate:screenshots. But playwright.config.ts had testDir: "./tests/e2e" with no testIgnore, so it ran in the CI E2E job on every push, writing examples/*/grid-cell.png as a side effect.

Fix: Added testIgnore gated on GENERATE_SCREENSHOTS env var. Note: Playwright's testIgnore is stronger than explicit CLI file arguments — a plain ignore would have broken npm run generate:screenshots (verified: 0 tests found). The env var is passed through the docker invocation.

playwright test --list diff

- Total: 82 tests in 3 files   (included 17 generate-grid-screenshots tests)
+ Total: 65 tests in 2 files   (servers.spec.ts + security.spec.ts only)

GENERATE_SCREENSHOTS=1 npx playwright test tests/e2e/generate-grid-screenshots.spec.ts --list17 tests in 1 file (docker path still works)

Bug 2: examples/pdf-server/server.test.ts never runs

Root package.json had "test": "bun test src" — the 448-LOC pdf-server test file was orphaned.

Fix: Changed to "test": "bun test src examples". Verified only one .test. file exists under examples/, so no accidental noise pickup. Tests were verified passing standalone before wiring.

npm test diff

- 87 pass, 0 fail across 3 files
+ 121 pass, 0 fail across 4 files  (+34 pdf-server tests)

Files changed

  • playwright.config.ts — added conditional testIgnore (+7 lines)
  • package.jsontest script scans examples/; generate:screenshots passes -e GENERATE_SCREENSHOTS=1 to docker

No changes to .github/workflows/ci.yml needed — npx playwright test (line 124) and npm test (lines 94, 159) pick up the fixes automatically.

Bug 1: generate-grid-screenshots.spec.ts was running on every CI E2E
invocation because playwright.config.ts had no testIgnore. This spec
writes examples/*/grid-cell.png as a side effect — it's meant to be
invoked only via npm run generate:screenshots.

Added testIgnore gated on GENERATE_SCREENSHOTS env var (Playwright's
testIgnore is stronger than explicit CLI file args, so a plain ignore
would break generate:screenshots). Passed the env var through the
docker invocation in generate:screenshots.

  Before: npx playwright test --list → 82 tests in 3 files
  After:  npx playwright test --list → 65 tests in 2 files
  GENERATE_SCREENSHOTS=1 ... --list → 17 tests in 1 file (still works)

Bug 2: examples/pdf-server/server.test.ts (448 LOC, 34 tests) was
never run — root script was 'bun test src'. Changed to 'bun test src
examples'. Only one .test. file exists in examples/ so no noise.

  Before: npm test → 87 pass across 3 files
  After:  npm test → 121 pass across 4 files
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 6, 2026

Open in StackBlitz

@modelcontextprotocol/ext-apps

npm i https://pkg.pr.new/@modelcontextprotocol/ext-apps@537

@modelcontextprotocol/server-basic-preact

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-preact@537

@modelcontextprotocol/server-basic-react

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-react@537

@modelcontextprotocol/server-basic-solid

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-solid@537

@modelcontextprotocol/server-basic-svelte

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-svelte@537

@modelcontextprotocol/server-basic-vanillajs

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-vanillajs@537

@modelcontextprotocol/server-basic-vue

npm i https://pkg.pr.new/@modelcontextprotocol/server-basic-vue@537

@modelcontextprotocol/server-budget-allocator

npm i https://pkg.pr.new/@modelcontextprotocol/server-budget-allocator@537

@modelcontextprotocol/server-cohort-heatmap

npm i https://pkg.pr.new/@modelcontextprotocol/server-cohort-heatmap@537

@modelcontextprotocol/server-customer-segmentation

npm i https://pkg.pr.new/@modelcontextprotocol/server-customer-segmentation@537

@modelcontextprotocol/server-debug

npm i https://pkg.pr.new/@modelcontextprotocol/server-debug@537

@modelcontextprotocol/server-map

npm i https://pkg.pr.new/@modelcontextprotocol/server-map@537

@modelcontextprotocol/server-pdf

npm i https://pkg.pr.new/@modelcontextprotocol/server-pdf@537

@modelcontextprotocol/server-scenario-modeler

npm i https://pkg.pr.new/@modelcontextprotocol/server-scenario-modeler@537

@modelcontextprotocol/server-shadertoy

npm i https://pkg.pr.new/@modelcontextprotocol/server-shadertoy@537

@modelcontextprotocol/server-sheet-music

npm i https://pkg.pr.new/@modelcontextprotocol/server-sheet-music@537

@modelcontextprotocol/server-system-monitor

npm i https://pkg.pr.new/@modelcontextprotocol/server-system-monitor@537

@modelcontextprotocol/server-threejs

npm i https://pkg.pr.new/@modelcontextprotocol/server-threejs@537

@modelcontextprotocol/server-transcript

npm i https://pkg.pr.new/@modelcontextprotocol/server-transcript@537

@modelcontextprotocol/server-video-resource

npm i https://pkg.pr.new/@modelcontextprotocol/server-video-resource@537

@modelcontextprotocol/server-wiki-explorer

npm i https://pkg.pr.new/@modelcontextprotocol/server-wiki-explorer@537

commit: d221428

@ochafik ochafik marked this pull request as ready for review March 6, 2026 16:27
@ochafik ochafik requested a review from jonathanhefner March 6, 2026 16:28
@ochafik ochafik merged commit 1ea1e9e into main Mar 6, 2026
20 checks passed
@ochafik ochafik deleted the worktree-agent-a06f8386 branch March 6, 2026 17:14
ochafik added a commit that referenced this pull request Mar 10, 2026
Changes since 1.2.0:
- fix: bundle SDK+zod in react-with-deps (was byte-identical to ./react) (#539)
- fix(build): copy schema.json to dist and externalize zod (#534)
- fix: skip debug log for high-frequency tool-input-partial notifications (#546)
- fix(deps): drop @hono/node-server override to patch GHSA-wc8c-qw6v-h7f6 (#535)
- fix(readme): use picture element for theme-aware logo (#545)
- fix(ci): require maintainer association for /update-snapshots trigger (#532)
- fix: pre-commit stages only originally-staged files; add .npmrc (#538)
- ci: use npm ci with caching, validate typedoc links, align Node versions (#533)
- test: exclude screenshot-gen from default E2E run; wire pdf-server tests (#537)
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.

2 participants