diff --git a/.dev/compose.yml b/.dev/compose.yml index 230eae71c..4204d208f 100644 --- a/.dev/compose.yml +++ b/.dev/compose.yml @@ -1,6 +1,9 @@ --- # Simplified Docker Compose configuration for JetThoughts Hugo development # Optimized for fast feedback loops with minimal complexity +# +# Host docker = colima (not Docker Desktop). Gotcha: credsStore: osxkeychain +# needs `brew install docker-credential-helper` (colima ships no osxkeychain helper). services: hugo: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 309d7cf77..38e626582 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -83,7 +83,7 @@ jobs: with: ref: ${{ github.event_name == 'workflow_dispatch' && github.ref || '' }} - - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@5ee298fbc3c1e070c78e3d83862ffd3228204214 + - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@db591a2b7d483f39985e39db1c648b48e44abfb9 with: ruby-version: '4.0' cache-apt-packages: true @@ -131,6 +131,8 @@ jobs: HUGO_DEFAULT_PATH: _dest/public-test # Record mode intentionally dirties the fixtures as it runs. ALLOW_DIRTY_SCREENSHOTS: '1' + # record=:all refuses under CI; this dispatch is the reviewed bulk accept. + CI: '' # always(): freshly recorded baselines must be committed even when a # test failed mid-run (a red test does not invalidate the OTHER pages' @@ -167,7 +169,7 @@ jobs: - name: Upload screenshot report if: failure() - uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@5ee298fbc3c1e070c78e3d83862ffd3228204214 + uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@db591a2b7d483f39985e39db1c648b48e44abfb9 with: name: screenshots report-path: test/fixtures/screenshots diff --git a/.okf/build/ci-gates.md b/.okf/build/ci-gates.md index fe6236fb6..09e084c46 100644 --- a/.okf/build/ci-gates.md +++ b/.okf/build/ci-gates.md @@ -251,7 +251,7 @@ a reviewer sees the pixels change alongside the change that moved them. Two gotchas the first record run hit (both fixed; evidence: [run 30629929407](https://github.com/jetthoughts/jetthoughts.github.io/actions/runs/30629929407) - 104 screenshots compared clean, 4 test failures, commit step never ran). Keep in mind for any new CI test job: - **Draft fixtures**: screenshot tests visit the draft post `/blog/codeblock-styles-fixture/`; local builds pass `--buildDrafts` but `bin/hugo-build` only does so when `BUILD_DRAFTS` is set - test.yml sets `BUILD_DRAFTS: '1'` on its setup-hugo step. A CI test build without it 404s the fixture and fails the codeblock tests on every run. -- **fail_if_new in CI**: snap_diff hard-errors on missing baselines when `ENV["CI"]` is set. Record mode (`FORCE_SCREENSHOT_UPDATE=true`) disables `fail_on_difference` AND `fail_if_new` (setup_snap_diff.rb) so pages added since the last recording can get their FIRST baseline. +- **Missing baselines / record mode (2.0 semantics, 2026-08-25)**: strictness is `SnapDiff.config.record = :none` in setup_snap_diff.rb - a missing baseline ALWAYS fails (2.0 default is `:once`, which records and compares the rest; 1.x used `fail_if_new`). Record mode (`FORCE_SCREENSHOT_UPDATE=true`) sets `record = :all` (re-record every screenshot, compare nothing), which creates FIRST baselines for pages added since the last recording. The gem REFUSES `:all` under `ENV["CI"]` by design, so the CI record step unsets CI (test.yml, `CI: ''`) - that dispatch IS the human-reviewed bulk accept. Two more, hit re-recording Linux baselines from CI on 2026-08-20 (see [test-gates.md](test-gates.md) for why CI, not local `bin/dtest`, is the only honest place to record them): - **"no checks reported" has TWO causes - check `mergeable_state` FIRST**: (a) the bot's baseline commit carries `[ci skip]`, so a record dispatch leaves the PR with no new run; (b) far more silent, an UNMERGEABLE PR produces ZERO checks at all - `pull_request` runs are built against a merge ref GitHub cannot compute, so it creates nothing rather than erroring. On 2026-08-20 (b) was the real blocker and (a) was wrongly blamed for 25 minutes; `gh api repos/OWNER/REPO/pulls/N --jq .mergeable_state` returned `dirty`. A baseline record takes ~20 min while master keeps moving, and the record commit plus any `.okf/log.md` edit conflicts easily - merge master and the checks appear. Never read missing checks as "still running" or "passing." diff --git a/.okf/build/rendering-stack.md b/.okf/build/rendering-stack.md index 5f68be817..3b63a5593 100644 --- a/.okf/build/rendering-stack.md +++ b/.okf/build/rendering-stack.md @@ -101,8 +101,10 @@ is cheap, boring, and would have ended it in a day. cannot pin (an x86 CPU cannot run arm64 Chrome), so `bin/setup-test-env` warns instead. Caught by review on #589, not by any gate - no suite fails when the only x86 host is one nobody has yet. -- A green visual run that prints no `[snap_diff] N screenshots compared` line - compared nothing - see [test-gates](/build/test-gates.md). +- A green visual run whose `[snap_diff]` line shows `0 verified` (or prints no + line) compared nothing - since 2.0 the line is `[snap_diff] N verified, N + changed, N new (not verified).` and prints on every run - see + [test-gates](/build/test-gates.md). ## arm64 is THE stack (Paul, 2026-08-22) - shipped, not on trial diff --git a/.okf/build/test-gates.md b/.okf/build/test-gates.md index fda75a998..c66f557e1 100644 --- a/.okf/build/test-gates.md +++ b/.okf/build/test-gates.md @@ -184,7 +184,7 @@ tolerance 0. Corroborating tell in any failure payload: the reported `region` never exceeds the viewport height (e.g. `[0.0,42.0,1920.0,1080.0]` for `services/_footer`). -There is a second tolerance-independent axis: `Capybara::Screenshot::Diff.perceptual_threshold = 2.0` +There is a second tolerance-independent axis: `SnapDiff.config.perceptual_threshold = 2.0` (`test/support/setup_snap_diff.rb:25`) means the vips driver only counts a pixel as differing once it is more than CIE dE00 2.0 from the baseline pixel. A recolour that stays under that contributes ZERO differing pixels, so it @@ -299,7 +299,7 @@ Minitest under `test/`, driven by `Rakefile` (`Rake::TestTask`). **The two readers disagree on what counts as set** (identified 2026-08-21, NOT fixed - known wart): `bin/qtest:209` skips its restore on ANY truthy - value, while `test/support/setup_snap_diff.rb:28` enters record mode only + value, while `test/support/setup_snap_diff.rb:33` enters record mode only on the literal string `"true"`. So `FORCE_SCREENSHOT_UPDATE=1` on qtest gets the worst of both - the suite still compares and fails, and the restore that would have cleaned up is skipped. Spell it `=true`, or better, @@ -613,12 +613,12 @@ gone rather than patched a fifth time: reading the block has none of those holes recording locally. Say so in the PR rather than letting a reader assume both legs were run. -- **Quote the compared COUNT, not just "0 failures"** (2026-08-21). A suite - that compared nothing and a suite that compared everything both print - `0 failures`. The distinguishing line is - `[snap_diff] N screenshots compared, no failures.` - cite the N. #518 cites - `53 screenshots compared`, which is what makes its green legible as - evidence rather than as an absence of errors. +- **Quote the verified COUNT, not just "0 failures"** (2026-08-21; format + updated 2026-08-25 for snap_diff 2.0). A suite that compared nothing and a + suite that compared everything both print `0 failures`. The distinguishing + line is now `[snap_diff] N verified, N changed, N new (not verified).` - + cite the N (verified). #518 cites `53 screenshots compared`, which is what + makes its green legible as evidence rather than as an absence of errors. - **A rendered-output sweep can look thorough and check almost nothing** (2026-08-20). Count the DISTINCT values a sweep actually resolves before @@ -913,13 +913,15 @@ such line at all. **Two fixes, each verified by breaking it:** -- `Capybara::Screenshot::Diff.fail_if_new = true` (test/support/setup_snap_diff.rb). - Outside CI the gem defaulted this to false, so an unresolvable baseline - passed silently - that default is what made the no-op invisible rather than - loud. Probe with a screenshot name absent from git: `1 runs, 1 assertions, - 1 failures`. A genuinely new page now fails its first run until recorded, - which is the same run-to-fail -> inspect -> commit flow this file already - documents. +- `SnapDiff.config.record = :none` (test/support/setup_snap_diff.rb) - the + 2.0 spelling of the old `Capybara::Screenshot::Diff.fail_if_new = true` + (`record` replaces `fail_if_new`, which 2.1 removes). The 2.0 default is + `:once` (record a missing baseline and compare the rest) outside CI, so an + unresolvable baseline still passed silently - that default is what made the + no-op invisible rather than loud. Probe with a screenshot name absent from + git: `1 runs, 1 assertions, 1 failures`. A genuinely new page now fails its + first run until recorded, which is the same run-to-fail -> inspect -> commit + flow this file already documents. - `bin/dtest` now MAKES git work from a worktree instead of refusing to run: it mounts the common git dir (`git rev-parse --path-format=absolute --git-common-dir`) at `/gitcommon` and sets `GIT_DIR=/gitcommon/worktrees/ @@ -929,9 +931,13 @@ such line at all. Verified by re-injecting the red body: `[snap_diff] 55 screenshots compared, 10 failures`, with `desktop/contact_us` and `mobile/contact_us` among them. -**Reading rule: a visual run that does not print `[snap_diff] N screenshots -compared` compared nothing.** Check for that line before believing green - -counting `0 failures` is not the same as counting comparisons. +**Reading rule: a visual run whose `[snap_diff]` summary line shows +`0 verified` (or prints no line at all) compared nothing.** Since 2.0 the line +- `[snap_diff] N verified, N changed, N new (not verified).` - prints on EVERY +run, passing or failing, and `0 verified` is the shout (the gem appends +`NOTHING WAS VERIFIED` when nothing was compared and nothing was re-recorded). +Check the verified count before believing green - counting `0 failures` is not +the same as counting comparisons. # arm64 dtest and amd64 CI genuinely disagree on text-heavy pages diff --git a/.okf/log.md b/.okf/log.md index 6afd94b7f..c537f72ba 100644 --- a/.okf/log.md +++ b/.okf/log.md @@ -251,11 +251,14 @@ the Linux side". That was worthless: it was green because it was not testing. Paul's main checkout has a real .git, so his dtest DID compare - which is why he saw 31 modified baselines from a red run and I could never reproduce one. -Root cause is not worktree-specific: outside CI the gem defaults `fail_if_new` -to false, so a baseline it cannot RETRIEVE is treated as a new screenshot and -passes. Now `true`, verified by a probe with a name absent from git. The tell to -remember is a missing line - a real run ends with `[snap_diff] N screenshots -compared`; the no-op run printed none, while still reporting `0 failures`. +Root cause is not worktree-specific: outside CI the 2.0 default `record = +:once` treats a baseline it cannot RETRIEVE as a new screenshot and passes +(1.x: `fail_if_new` defaulted to false; `SnapDiff.config.record = :none` now +restores strictness in setup_snap_diff.rb, verified by a probe with a name +absent from git). The tell to remember is the summary line - a real run ends +with `[snap_diff] N verified, N changed, N new (not verified).`; the no-op run +shows `0 verified` (or printed nothing at all in 1.x), while still reporting +`0 failures`. ## 2026-08-22 - closing three of the fault-injection gaps, each broken before it was trusted @@ -869,7 +872,7 @@ Concept updated: [build/test-gates.md](build/test-gates.md). **Known wart, identified and NOT fixed:** the two `FORCE_SCREENSHOT_UPDATE` readers disagree on what counts as set - `bin/qtest:209` skips its restore on -any truthy value, `test/support/setup_snap_diff.rb:28` enters record mode only +any truthy value, `test/support/setup_snap_diff.rb:33` enters record mode only on the literal `"true"`. `=1` on qtest therefore gets the worst of both: the suite still compares and fails, and the cleanup is skipped. That is the mechanism behind the older "the flag appears to be ignored on qtest" note. diff --git a/.okf/workflows/autonomous-loops.md b/.okf/workflows/autonomous-loops.md index 573950f77..275df5692 100644 --- a/.okf/workflows/autonomous-loops.md +++ b/.okf/workflows/autonomous-loops.md @@ -92,7 +92,8 @@ failed dedup. A two-exit loop would have written both. The loop re-reads the prompt every iteration, so it is the cheapest place to put gate rules that would otherwise decay. The 1a.4 prompt carried "never cite `bin/dtest` from this worktree - it is vacuous-green", "quote the -`[snap_diff] N screenshots compared` count", and "`--strict` EXITS 1 by design", +`[snap_diff] N verified` count" (2.0 summary format; the old wording was +`[snap_diff] N screenshots compared`), and "`--strict` EXITS 1 by design", and none of those was violated across the whole run. # The delivery contract that wraps this diff --git a/Gemfile b/Gemfile index 940c07592..19208180b 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,8 @@ gem "ostruct" gem "sanity-ruby" -gem "capybara-screenshot-diff", github: "snap-diff/snap_diff-capybara" +# Same gem as capybara-screenshot-diff; install one, never both. +gem "snap_diff-capybara", "2.0.0.beta4" gem "simplecov", require: false gem "vips" diff --git a/Gemfile.lock b/Gemfile.lock index cee96f223..ef755deab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,10 +1,3 @@ -GIT - remote: https://github.com/snap-diff/snap_diff-capybara.git - revision: 5ee298fbc3c1e070c78e3d83862ffd3228204214 - specs: - capybara-screenshot-diff (1.12.0) - capybara (>= 2, < 4) - GEM remote: https://rubygems.org/ specs: @@ -171,6 +164,8 @@ GEM rubyzip (>= 1.2.2, < 4.0) websocket (~> 1.0) simplecov (1.1.1) + snap_diff-capybara (2.0.0.beta4) + capybara (>= 2, < 4) standard (1.56.0) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.0) @@ -210,7 +205,6 @@ PLATFORMS DEPENDENCIES bigdecimal capybara - capybara-screenshot-diff! faraday html-proofer httparty @@ -225,6 +219,7 @@ DEPENDENCIES sanity-ruby selenium-webdriver simplecov + snap_diff-capybara (= 2.0.0.beta4) standard vips diff --git a/README.md b/README.md index f4f4595b9..75d508a8d 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ JetThoughts website is a sophisticated static site generator project that showca gem "minitest" gem "capybara" gem "selenium-webdriver" -gem "capybara-screenshot-diff" +gem "snap_diff-capybara", "2.0.0.beta4" # API & HTTP gem "httparty" diff --git a/bin/dtest b/bin/dtest index 8107e0dbd..9714485a8 100755 --- a/bin/dtest +++ b/bin/dtest @@ -52,7 +52,7 @@ fi # NOTHING: captures are written over the baselines and every run is green. # Measured 2026-08-22 - a contact page with `body { background: red }`, # difference_level 0.68, passed with 0 failures, and the only tell was a -# missing `[snap_diff] N screenshots compared` line. +# missing `[snap_diff] N verified` line. # # Mount the COMMON git dir (the main repo's .git, which holds objects/refs and # every worktree's entry) and point GIT_DIR at this worktree inside it. The diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 445a0c8b8..e17de9ffb 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -23,7 +23,7 @@ # an unscoped glob let dirty macos/ baselines abort the linux/ leg - bin/dtest # refusing to start on a Mac mid-review was never the intent. unless ENV["ALLOW_DIRTY_SCREENSHOTS"] - os_dir = "test/fixtures/screenshots/#{Capybara::Screenshot::Os.name}" + os_dir = "test/fixtures/screenshots/#{SnapDiff::Os.name}" dirty = `git status --porcelain #{os_dir} 2>/dev/null`.lines if dirty.any? warn "Screenshot fixtures are dirty (a previous run rewrote baselines):" @@ -69,8 +69,8 @@ def visit_via_menu(menu_text, submenu_text = nil) class ApplicationSystemTestCase < Minitest::Test include Capybara::DSL include Capybara::Minitest::Assertions - include CapybaraScreenshotDiff::DSL - include CapybaraScreenshotDiff::Minitest::Assertions + include SnapDiff::DSL + include SnapDiff::Minitest::Assertions include NavigationHelpers # Floor for calls that pin no tolerance of their own. 0.02 let ~41,472 pixels diff --git a/test/support/setup_snap_diff.rb b/test/support/setup_snap_diff.rb index ccbbd6a8c..4bd49f8bc 100644 --- a/test/support/setup_snap_diff.rb +++ b/test/support/setup_snap_diff.rb @@ -5,48 +5,32 @@ # - FORCE_SCREENSHOT_UPDATE: Set to 'true' to regenerate all baselines # -require "capybara_screenshot_diff/minitest" -require "capybara_screenshot_diff/reporters/html" +require "snap_diff/integrations/minitest" +require "snap_diff/reporters/html" -Capybara::Screenshot.save_path = "test/fixtures/screenshots" +SnapDiff.config.save_path = "test/fixtures/screenshots" # The HTML reporter only writes when a run HAS failures, so a green run would # otherwise leave the previous red run's report on disk - reviewing it shows # diffs that no longer exist. Drop it up front; every entry point (bin/test, # bin/qtest, bin/dtest, rake test:*) loads this file. File.delete("test/fixtures/screenshots/snap_diff_report.html") if File.exist?("test/fixtures/screenshots/snap_diff_report.html") -Capybara::Screenshot.add_os_path = true -Capybara::Screenshot.window_size = nil -Capybara::Screenshot.stability_time_limit = ENV.fetch("SCREENSHOT_STABILITY_TIME", "0.1").to_f -Capybara::Screenshot.disable_animations = true -Capybara::Screenshot.root = Dir.pwd +SnapDiff.config.add_os_path = true +SnapDiff.config.window_size = nil +SnapDiff.config.stability_time_limit = ENV.fetch("SCREENSHOT_STABILITY_TIME", "0.1").to_f +SnapDiff.config.disable_animations = true +SnapDiff.config.root = Dir.pwd -Capybara::Screenshot::Diff.driver = :vips -Capybara::Screenshot::Diff.perceptual_threshold = 2.0 -Capybara::Screenshot::Diff.delayed = true +# No driver line: 2.1 removes the knob; :auto resolves to vips. +SnapDiff.config.perceptual_threshold = 2.0 +SnapDiff.config.delayed = true -# A screenshot whose baseline cannot be RETRIEVED must fail, not pass. The gem -# resolves baselines with `git show HEAD:`, so anything that breaks git -# silently turns the whole visual gate into a no-op: captures are written, -# nothing is compared, every run is green. -# -# Measured 2026-08-22: run from a git WORKTREE, `.git` is a file pointing at an -# absolute host path under the main repo, which the container does not mount - -# so git inside it reports "not a git repository", every baseline lookup -# returns nothing, and a contact page with a `background: red !important` body -# (difference_level 0.68) passed with 0 failures. Defaulting fail_if_new to -# false outside CI is what made that invisible. -# -# Consequence: a genuinely NEW page fails its first run until recorded. That is -# the same run-to-fail -> inspect -> commit flow .okf/build/test-gates.md -# already documents for re-records; record mode below still bypasses it. -Capybara::Screenshot::Diff.fail_if_new = true +# Missing baselines must FAIL: record = :none (2.0 spelling of fail_if_new = +# true). Anything that breaks git would otherwise pass green silently +# (measured 2026-08-22, git worktree - see .okf/build/test-gates.md). +SnapDiff.config.record = :none if ENV["FORCE_SCREENSHOT_UPDATE"] == "true" - Capybara::Screenshot::Diff.fail_on_difference = false - # Record mode must be able to create FIRST baselines for new pages: with - # ENV["CI"] set, snap_diff's fail_if_new defaults to hard-error, which made - # the CI re-record run fail on pages added since the last recording - # (2026-07-31: vibe_code_rescue had no linux/ baseline yet). - Capybara::Screenshot::Diff.fail_if_new = false + # Bulk re-record; gem refuses :all under CI - test.yml unsets it for this step. + SnapDiff.config.record = :all end