fix(devx): make bin/test green + fast - root-relative builds, restore-on-green baselines - #424
Conversation
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (12)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChangesScreenshot test flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant bin/qtest
participant bin/test
participant ScreenshotTests
participant Git
bin/qtest->>bin/test: run --all
bin/test->>ScreenshotTests: execute tests
ScreenshotTests-->>bin/test: success or failure
bin/test->>Git: restore screenshot fixtures after success
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two coupled workflow bugs made bin/test look wholesale broken: - bin/test's R2 fast path baked BASE_URL=http://localhost:1314 into the built HTML while Capybara boots Puma on a RANDOM port (TEST_SERVER_PORT is only set in Docker). Every external stylesheet and every clicked link hit a dead port: 49/49 screenshots diffed (half-styled pages on a dark-mode UA background) and click-navigation tests landed on ERR_CONNECTION_REFUSED. Build with baseURL "/" instead - the same convention CI (.github/actions/setup-hugo), bin/qtest, and Hugo#precompile already use. Ports no longer matter. - Every green run rewrites the baseline PNGs in place (snap_diff design), so dtest's sub-tolerance Rosetta drift (~0.004%/run) left ~54 linux PNGs dirty after every pass and the dirty-fixture guard then blocked every subsequent bin/test/bin/qtest until a manual reset. Green runs now restore test/fixtures/screenshots automatically (bin/test, bin/dtest, bin/qtest); red runs still keep candidates + diffs for inspection; FORCE_SCREENSHOT_UPDATE skips the restore so re-records survive. qtest's --all escalation now routes through bin/test to inherit the same behavior. Also: bin/dtest reuses a warm build via the same mtime staleness check as bin/test (~12s saved per rerun). Co-Authored-By: Claude Fable 5 <[email protected]>
Recorded with the fixed root-relative build (full test:system, 67 runs, 0 failures). These are the pages whose content actually changed since the last macos recording - blog index/pagination (Wave G posts), course landing/chapter, testimonials, special posts, mobile nav - mirroring the linux re-record CI committed in 2edb94d. Visually verified: blog index, course landing, homepage services render fully styled. Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
a33cc4d to
e594b19
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.okf/build/test-gates.md:
- Around line 50-68: In .okf/build/test-gates.md lines 50-68, add source links
for every factual assertion, including the test scripts, baseURL diagnosis,
observed failures, and retained verification output. In .okf/log.md lines
340-353, add citations for the verification output and baseline re-recording
evidence; ensure both artifacts contain no unsupported factual claims.
In `@bin/test`:
- Around line 59-67: Update the screenshot cleanup in bin/test (lines 59-67),
bin/dtest (lines 40-42), and bin/qtest (lines 192-194) to restore fixtures
explicitly from Git HEAD rather than the index, preserving intentionally staged
content. Ensure every runner guards cleanup on Git availability so Docker or
dtest environments without Git skip the restore safely.
🪄 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: 727d0f88-e957-4944-8479-2e15ff268e38
⛔ Files ignored due to path filters (12)
test/fixtures/screenshots/macos/desktop/blog/index.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/desktop/blog/index/_pagination.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/desktop/blog/special/youtube_post.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/desktop/clients/_testimonials.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/desktop/course/chapter.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/desktop/course/landing.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/blog/index.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/blog/index/_pagination.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/blog/special/code_highlight_post.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/course/chapter.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/nav/hamburger_menu.pngis excluded by!**/*.pngtest/fixtures/screenshots/macos/mobile/nav/hamburger_menu/services.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.okf/build/test-gates.md.okf/log.mdbin/dtestbin/qtestbin/test
…ack) Co-Authored-By: Claude Fable 5 <[email protected]>
Summary
Host
bin/testfailed 49/49 screenshots (~11 min per red run) whilebin/dteststayed green, and every passing run left the fixture tree dirty, blocking the next run. Two root causes, both fixed:BASE_URL=http://localhost:1314, but Capybara boots Puma on a random port on the host (TEST_SERVER_PORTis only pinned in Docker). Every external stylesheet and clicked link hit a dead port: all screenshots diffed (half-styled pages on a dark-mode UA background) and click-navigation tests landed onERR_CONNECTION_REFUSED. Test builds now usebaseURL "/"- the same convention CI (setup-hugo),bin/qtest, andHugo#precompilealready used.bin/test/bin/dtest/bin/qtestnow auto-restoretest/fixtures/screenshots; red runs keep candidates + diff artifacts;FORCE_SCREENSHOT_UPDATEre-records are exempt.Commits
88bfd59bfix(devx): root-relative test builds + restore-on-green baselines - the two fixes above, plus:bin/qtest --allescalation routes throughbin/test(inherits build-skip + restore), andbin/dtestreuses a warm Hugo build via the same mtime staleness check asbin/test(~12s/rerun).a7d58a40chore: re-record 12 macos screenshot baselines - recorded with the fixed build (fulltest:system: 67 runs, 0 failures). Only pages with real content changes since the last macos recording (blog index/pagination with the Wave G posts, course landing/chapter, testimonials, special posts, mobile nav) - mirroring the linux re-record CI committed in2edb94d9. Brightness-audited (an earlier record attempt under the port bug saved Chrome error pages as baselines; caught and discarded) and visually spot-checked: blog index, course landing, homepage services all render fully styled.a33cc4d3docs(okf): test-gates - both caveats captured in.okf/build/test-gates.md+ log.Verification
bin/dtest: 34 runs, 0 failures; tree clean afterwards (restore verified on two independent runs, including Paul's own).bin/test(host): 34 runs, 0 failures, 53 screenshots matched, 5:01 warm; tree clean afterwards. Previously: 29 failures + 2 errors in ~11 min.bin/dtestthenbin/test: no dirty-fixture guard trip.🤖 Generated with Claude Code
Summary by CodeRabbit