Storybook: move smoke-test deps into the storybook workspace - #81017
Conversation
|
Size Change: 0 B Total Size: 7.76 MB |
3c66091 to
bd8a0e2
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Declare @storybook/test-runner, playwright, http-server, concurrently and wait-on as devDependencies of @wordpress/storybook and run the smoke test via workspace scripts, removing the CI-time npm install and npx usage from the storybook-check workflow. Aligns root concurrently on v9 to match.
bd8a0e2 to
303c692
Compare
Mamaduka
left a comment
There was a problem hiding this comment.
Looks good.
I guess +1,743 lines to package lock isn't avoidable.
Yes, unavoidable due to a few new deps in the workspace. |
|
Flaky tests detected in 303c692. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/30631658542
|
Resolve storybook-check.yml against #81017, which moved the smoke-test deps into the storybook workspace: keep that workspace refactor, and install browsers through .github/setup-playwright with chromium only.
What?
Moves the Storybook smoke-test dependencies (
@storybook/test-runner,playwright,http-server,concurrently,wait-on) into the@wordpress/storybookworkspace so thestorybook-checkworkflow no longer needs ad-hocnpm installornpxat CI time.Why?
Dependencies installed ad hoc at CI time (
npm install --no-save,npx) are invisible to the tooling that governs the repo's dependency tree: they are not inpackage-lock.json, so they are not covered by security audits, Dependabot, syncpack version policies, or lockfile validation. They also offer no protection against supply-chain attacks — the workflow fetches whatever the registry serves at run time, without the lockfile's integrity pinning or the repo'smin-release-agesafeguard, and a compromised package would execute inside CI with access to its secrets. Declaring them in the workspace that owns the smoke test puts them under the same lockfile, audit, and update process as every other dependency.As a bonus, this also fixes the workflow under
install-strategy=linked(isolated dependencies - #75814), wherenpxno longer resolves hoisted binaries from the repo root: the workflow becomes a pair of plainnpm run --workspacecalls that work under both isolated and hoisted strategies.Recommendation: for the reasons above, we should ideally ban installing dependencies from within CI workflows (
npm install <pkg>,npx <pkg>, etc.) across the repo — every dependency a workflow needs should be declared in a workspacepackage.jsonand installed through the lockfile via the regularnpm cistep. CC: @desrosjHow?
@storybook/test-runner@0.24.2(itsstorybookpeer is already a devDependency of the workspace),playwright,http-server,concurrently, andwait-ontostorybook/package.jsondevDependencies.test:smoke(serve build + wait +test-storybook) andtest:smoke:setup(Playwright browser install) scripts to the workspace.npm run --workspace @wordpress/storybook test:smoke:setupand… test:smoke—npm runputs the workspacenode_modules/.binonPATH, so nonpxis needed.Testing Instructions
npm ci,npm run storybook:build, thennpm run --workspace @wordpress/storybook test:smoke.Note: this change was also validated with isolated dependencies (
install-strategy=linked) by temporarily basing it on #75814 — the smoke-test job passed there too (run 1, run 2).Use of AI Tools
Authored with the assistance of Claude Code; reviewed and tested by the PR author.