Repository navigation
test(bulma-ui): Storybook smoke gate — every story, light + dark, no console errors - #315
Conversation
…console errors Wires @storybook/test-runner (#283): each story must render without throwing and without console errors, once as-is and once with data-theme="dark" stamped on the root, closing the dark-mode blind spot jsdom can't see (the #257 residual-defect class). - .storybook/test-runner.ts: preVisit stamps the theme and collects console/page errors per page; postVisit fails the story on any. Resource errors from RFC 2606 `.invalid` hosts are ignored — that's how stories demo broken-image fallbacks (Avatar). - scripts/test-storybook-ci.mjs: serves storybook-static with node's http module (no http-server/wait-on/concurrently deps) and runs the light and dark passes. - ci.yml: Playwright Chromium install + smoke step after the Storybook build; the build step's changed-files condition is dropped — it never matched on PRs (changed_files is an integer count, not a path list). - pnpm-workspace.yaml: @swc/core build scripts stay blocked (platform binaries ship as optional deps, the postinstall is only a fallback). - Image stories: fix /logo.png → /img/logo.png (staticDirs maps images to /img) — a real 404 the new gate caught on its first run. First full run: 89 suites / 1089 stories green in both passes. Closes #283 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughStorybook CI now builds unconditionally and runs Playwright smoke tests against static output in light and dark themes. Test-runner hooks capture page errors, image stories use the corrected asset path, and CI installs Chromium with required dependencies. ChangesStorybook smoke testing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant StorybookBuild
participant SmokeScript
participant Playwright
participant StorybookStatic
CI->>StorybookBuild: build Storybook
CI->>SmokeScript: run CI smoke script
SmokeScript->>StorybookStatic: serve static output
SmokeScript->>Playwright: run light pass
Playwright->>StorybookStatic: visit stories
SmokeScript->>Playwright: run dark pass
Playwright->>StorybookStatic: visit stories with dark theme
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Preview DeploymentPreview URL: https://d3ec562e.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 1 finding
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | Correctness | "Light" pass inherits an ambient STORYBOOK_THEME, so a stray export STORYBOOK_THEME=dark silently turns it into a second dark pass |
bulma-ui/scripts/test-storybook-ci.mjs:74 |
Overall: This is a solid, well-scoped test-infrastructure PR and I could not find a blocking defect. I verified the parts most likely to be wrong: the dark mechanism (data-theme="dark" on <html>) exactly matches the library's own Theme component (Theme.tsx:781) and Bulma 1.0.4's theming, so the dark pass really does apply dark CSS rather than being a silent no-op; the per-page console sink is correct against the actual test-runner v0.24 lifecycle (no per-story navigation, preVisit runs before render, and the "execution context destroyed" retry re-invokes preVisit, re-arming the sink on a reset page); the .invalid filter matches Avatar's fallback convention; the /img/logo.png fix is real and consistent with every other story; playwright 1.61.1 matches the test-runner's bundled playwright-core; and the static server's normalize()-then-strip-leading-slash guards against path traversal. The one thing worth the human's attention is a scoping judgment, not a bug: this is a console/pageerror smoke gate, so it catches dark-mode CSS bugs only when they surface as a thrown error or console message — purely visual dark-mode regressions (contrast, invisible text, broken layout) still won't be caught, which is a narrower guarantee than "dark-mode CSS bugs" might suggest. That's by design (visual regression lives elsewhere); just worth naming so expectations are calibrated. The Minor finding is a latent robustness gap, not a live CI failure.
🏄 Clean sets rolling in on this one, dude — the dark-mode paddle-out actually lines up with how the library flips themes, the broken-image gnar is filtered right, and nothing's gonna wipe out on jsdom's blind spot anymore. Just tuck that stray env var back in the wax pocket and you're good to drop in.
Deep-review finding on #315: with STORYBOOK_THEME=dark exported in the surrounding shell, the light pass silently became a second dark pass. The env is now built explicitly for both passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Preview DeploymentPreview URL: https://0fd7088b.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
70-86: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCorrect ordering and unconditional build fix confirmed; consider caching Playwright browsers.
The unconditional Storybook build plus install/smoke-test sequence is ordered correctly. As an optional follow-up, caching
~/.cache/ms-playwright(keyed on theplaywrightversion) would avoid re-downloading Chromium every run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 70 - 86, Optionally add GitHub Actions caching for ~/.cache/ms-playwright around the Storybook smoke-test sequence, using a cache key derived from the installed Playwright version. Preserve the existing unconditional “Build Storybook (bulma-ui)”, “Install Playwright Chromium”, and “Storybook smoke test (light + dark)” ordering and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bulma-ui/scripts/test-storybook-ci.mjs`:
- Around line 64-80: Add an error listener to the child process created in
runPass so spawn failures resolve the promise with a nonzero exit code, while
preserving the existing close-handler behavior for normally launched processes.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 70-86: Optionally add GitHub Actions caching for
~/.cache/ms-playwright around the Storybook smoke-test sequence, using a cache
key derived from the installed Playwright version. Preserve the existing
unconditional “Build Storybook (bulma-ui)”, “Install Playwright Chromium”, and
“Storybook smoke test (light + dark)” ordering and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d4582d1-1687-4f58-9f69-93b55f9a140a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (6)
.github/workflows/ci.ymlbulma-ui/.storybook/test-runner.tsbulma-ui/package.jsonbulma-ui/scripts/test-storybook-ci.mjsbulma-ui/src/elements/Image.stories.tsxpnpm-workspace.yaml
CodeRabbit finding on #315: runPass only resolved via 'close', so a failed spawn (ENOENT) would hang the CI job indefinitely. An 'error' handler now logs and resolves with exit 1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Preview DeploymentPreview URL: https://e774f501.bestax.pages.dev |
There was a problem hiding this comment.
Deep review — 0 findings
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| — | — | — | No blocking defects found. | — |
Overall: The change is sound. I traced the single most load-bearing claim — that data-theme="dark" stamped in preVisit actually reaches the rendered story — through the installed @storybook/test-runner: defaultPrepare navigates to iframe.html once per page, and each story renders via an in-page __test(id) SPA remount rather than a fresh navigation, so the attribute set before each __test persists into render. The .invalid allowlist regex is anchored correctly (example.invalid.com is not swallowed), the static server can't escape storybook-static/ (absolute pathnames make normalize drop stray ../), the /logo.png → /img/logo.png fix matches the staticDirs ../images → /img mapping and a real logo.png, and the prior round's ambient-STORYBOOK_THEME leak is already fixed by building the child env explicitly per pass. One non-blocking note for the human: if a story both throws in __test and logged console errors, postVisit throws its own message and masks the original render stack — the story still fails, only the diagnostic is less specific. The two script/config files sit outside the 99% jest coverage scope by design (tooling, not shipped src/).
🏄 Total glassy conditions on this one, dude — the dark-mode pass actually catches the wave jsdom always paddles past, the traversal and
.invalidreefs are all buoyed off, and last round's rogue env current already got patched. Clean ride, send it. 🌊
|
🎉 This PR is included in version 5.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.3.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Closes #283 — Tier-2 item 1 from #263, done as a session PR (touches
.github/**andpnpm-workspace.yaml, so it's outside the AI loop by design).What
A CI smoke gate over the built Storybook: every story must render without errors and without console errors, once in light and once with
data-theme="dark"stamped on the root. Dark-mode-only CSS bugs are invisible to jsdom (the #257 residual-defect class survived four text-only reviews) — this is the first automated coverage for them. No screenshot baselines — it's a smoke gate, not visual regression (visual-regression.ymlexists separately).How
bulma-ui/.storybook/test-runner.ts— test-runner hooks:preVisitstamps the theme (whenSTORYBOOK_THEME=dark) and maintains a per-page console/pageerror sink;postVisitfails the story if anything landed. Resource errors from RFC 2606.invalidhosts are exempt — that's the convention stories use to demo broken-image fallbacks (Avatar'shttps://example.invalid/missing.jpg).bulma-ui/scripts/test-storybook-ci.mjs— servesstorybook-static/with node'shttpmodule and runs both passes. Deliberately avoids addinghttp-server/wait-on/concurrently(three deps) for what is ~40 lines of stdlib.ci.yml— Playwright Chromium install + smoke step after the Storybook build. The build step's oldcontains(github.event.pull_request.changed_files, 'bulma-ui/**')condition is dropped:changed_filesis an integer count, so it never matched and Storybook was silently not being built on PR CI at all.test-storybook(dev server),test-storybook:dark,test-storybook:ci(static, both passes).Supply-chain notes
@storybook/test-runner@^0.24.4(supports Storybook 10, published 2026-05 — clears the 3-day cooldown) andplaywright(its CLI is invoked directly, so it's declared per the no-phantom-deps rule; it's already a transitive dep of the test-runner).pnpm-workspace.yaml:@swc/core: false— its postinstall is only a fallback native build; the platform binary arrives as an optional dependency, verified working with scripts blocked.It caught real bugs on its first run
Two Image stories pointed at
/logo.png, butstaticDirsmaps images to/img/— genuine 404s in every published Storybook until now. Fixed to/img/logo.pngin this PR.Verification
pnpm all,gen:catalog:check,check:conformanceall green.🤖 Generated with Claude Code
Summary by CodeRabbit