Skip to content

Comments

chore: build project before playwright test#777

Merged
danielroe merged 5 commits intonpmx-dev:mainfrom
mihkeleidast:build-before-playwright
Feb 3, 2026
Merged

chore: build project before playwright test#777
danielroe merged 5 commits intonpmx-dev:mainfrom
mihkeleidast:build-before-playwright

Conversation

@mihkeleidast
Copy link
Contributor

Effectively does the same thing, but builds for playwright browser tests as a pre-step, so playwright won't have to wait for the build to succeed, rather just the webserver starting, which should be fast.

Lowered the timeout to the default value, but left it in the config, as it might still be useful to see/change later on.

@vercel
Copy link

vercel bot commented Feb 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 3, 2026 0:02am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 3, 2026 0:02am
npmx-lunaria Ignored Ignored Feb 3, 2026 0:02am

Request Review

package.json Outdated
"test": "vite test",
"test:browser": "playwright test",
"test:browser:ui": "playwright test --ui",
"test:browser:ui": "pnpm build:playwright && playwright test --ui",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can also move it to a separate step in the CI workflow - that could give us more visibility into if/how the build time changes over time. But kept it here now so the local browser tests run would be the same DX, just run the same command.

"scripts": {
"build": "nuxt build",
"build:lunaria": "node ./lunaria/lunaria.ts",
"build:playwright": "NODE_ENV=test pnpm build",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure where the NODE_ENV matters, kept it for both for now

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

this shouldn't make any difference in CI but it does enable writing tests more easily locally - e.g you build server, then iterate on it with playwright.... so I think it's a good thing

thank you!: ❤️

@mihkeleidast
Copy link
Contributor Author

From what I understood is that the main issue currently is that playwright has to wait for the build to complete before running browser tests - this should affect CI positively in that regard, i.e. it doesn't matter if the build takes 1-2 minutes longer (depending on flakyness in CPU/memory availability for example), playwright only starts after the build completes, and only has to wait for the local webserver to start.

@danielroe
Copy link
Member

playwright has to wait for the build to finish one way or another - either before playwright starts, or as part of the playwright process - so this is the same as simply adding a very high timeout for that start command

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

A dedicated build:playwright script was added and CI now runs pnpm build:playwright before browser tests. test:browser was changed to run the new build:playwright and then test:browser:prebuilt; test:browser:prebuilt was added. test:browser:ui and test:browser:update now invoke the build before running the prebuilt test command. start:playwright:webserver was changed to NODE_ENV=test pnpm preview --port 5678 (no in-place build). Playwright webServer.timeout was shortened from 240_000 ms to 60_000 ms.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description directly addresses the changeset: moving the build step before Playwright tests and lowering the timeout, which aligns with the file modifications in .github/workflows/ci.yml, package.json, and playwright.config.ts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

@danielroe danielroe added this pull request to the merge queue Feb 3, 2026
@danielroe danielroe removed this pull request from the merge queue due to a manual request Feb 3, 2026
@danielroe danielroe enabled auto-merge February 3, 2026 12:00
@danielroe danielroe added this pull request to the merge queue Feb 3, 2026
Merged via the queue into npmx-dev:main with commit 3114183 Feb 3, 2026
14 checks passed
taskyliz pushed a commit to taskyliz/npmx.dev that referenced this pull request Feb 7, 2026
Co-authored-by: Daniel Roe <daniel@roe.dev>
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