Skip to content

fix: the HTML report lists new screenshots - #5

Closed
oboxodo wants to merge 1 commit into
masterfrom
fix/report-new-screenshots
Closed

oboxodo wants to merge 1 commit into
masterfrom
fix/report-new-screenshots

Conversation

@oboxodo

@oboxodo oboxodo commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Fork-only draft for review before opening upstream.

Problem

Measured on 7bb159c with a real Minitest run, real git baselines, capture stubbed
(test/fixtures/summary_line_case.rb):

Run Terminal Report (before) Report (after)
new only, record: :once 0 verified, 0 changed, 1 new none written new listed; pills 0 failed · 0 passed · 0 total · 1 new
changed + new, :once 1 verified, 1 changed, 1 new changed only; 1 failed · 0 passed · 1 total changed, then new; … · 1 new
new only, record: :none (CI) before 0 new, after 1 new; test fails with the same message none written new listed with its capture (embedded on CI)

Screenshots

Real Minitest runs, the report opened in headless Chrome.

Before (master): changed + new After: same run
00-before-mixed 02-mixed-new
Only changed is listed; the new capture is missing. new is listed after the failures: the capture, and the command to accept it.

CI (record: :none), only a new screenshot: master writes no report; now the report is written, with the capture embedded:

08-ci-none-embedded

The command is selected with one click, even when zoomed in (here 200%):

05-zoom200-select

Mechanism

  • Reporting's new/re-recorded tallies keep each captured path; SnapDiff::Reporting.uncompared
    returns them as frozen values (name, path, reason :new/:rerecorded), complete by
    finalize. The reporter contract is unchanged: any reporter reads it in finalize; the HTML
    reporter does, so its new is the terminal's. Documented in docs/snapdiff.md and
    docs/reporters.md.
  • The matcher tallies (and warns) once, after a successful capture and after the
    disappeared-baseline check (Fiber and thread safety: remaining decisions and hardening after the v2 core work snap-diff/snap_diff-capybara#217); image + git add -- <root-relative, shell-escaped path>
    only for a file on disk at render time (The new-screenshot error prints a path that will never exist in CI snap-diff/snap_diff-capybara#260).
  • Fork-parallel: the entries travel with the counts in Reporting's fragment (new uncompared
    key, read with a default; the name lists stay), so the parent's list is the whole run's.
  • Page: existing views; empty Baseline side uses the existing .img-empty message with the
    command (one click selects it; clicking it no longer toggles Annotated); new badges and
    pill in --accent.

Not changed

The reporter contract (record/finalize/summary) and reporters' state handoff; failure messages;
failure entries' rendering; view modes and shortcuts. Terminal output under :once/:all,
except: a capture that raised is no longer announced as new, and the same-name race below.
Under :none a new screenshot now prints the failure, 1 new and the existing "had no
committed baseline … : new" roll-up line — intended: it is the same line :once prints.

Adjacent fixes

Known limitations

Keyed by name (as the count already is). Under record: :all a brand-new screenshot counts
as re-recorded. One-process-per-worker runners: last writer wins, now also for a new-only process.

Evidence

  • Mutation checks (each reverted alone → named test red): list / entries not frozen, reset
    keeping entries, fragment without paths, merge ignoring them / guessing an unknown reason /
    names overriding them /
    re-recorded merged as new, every sighting treated as first, finalize condition, re-recorded
    listed, tally before the session check, existence check, template DATA,
    shell escape, -- separator, :none fall-through, no -w warning.
  • Full suite in the repo's container image (cuprite): 855 runs, 0 failures (master: 826).
  • JRuby 10.1 (local container): bin/rake test:unit 812 runs, 0 failures (PR CI runs MRI 4.0 only).
  • Size: lib/ (incl. template) +178/−54, tests +417/−17, docs/README/CHANGELOG +54/−8.
  • bin/standardrb clean.

Follow-up

PR B: list re-recorded (record: :all) screenshots with a before/after against the previous
baseline from git.

A screenshot captured with no committed baseline was only counted by name
for the summary line; reporters receive compared assertions only. So the
report left it out, a run whose only finding was a new screenshot wrote no
report at all, and under record: :none (CI's default) the raise came before
the tally, so the summary said `0 new` while the capture sat on disk.

Reporting's tallies of new and re-recorded screenshots now keep each
one's captured path, and SnapDiff::Reporting.uncompared returns them as
frozen Reporting::Uncompared values (name, path, reason :new or
:rerecorded): the entries behind the summary line's counts, complete by
the time reporters finalize. They travel with the counts in the
fork-parallel fragment; the name lists every version writes stay, and a
name only an older worker sent is still listed, with no capture. An
entry with a reason no version writes is a bug and raises instead of
being guessed at. The reporter contract (record/finalize/summary) is unchanged: any reporter,
the bundled HTML one included, reads the list in finalize, so the
report's `new` is the terminal's by construction, reset_run_totals!
clears it with the counts, and no reporter can alter what another reads.

The matcher now tallies a new screenshot once, after a successful capture
and after the session's disappeared-baseline check (snap-diff#217), and warns from
there; only "was a file already there" is still read before the capture.
So a listed path is a file that was captured (snap-diff#260), a failed capture is
not reported as new, and a baseline git handed over but a same-name test
moved away is never new, in any mode: every mode raises the session's
"disappeared" error. Before, record: :none told the user to `git add` a
screenshot whose baseline was committed.

The report builds the image and `git add -- <path>` only for a file on
disk at render time; `--` keeps a name starting with a dash from reading
as an option. In the page, a new screenshot goes through the existing views: the
empty Baseline side explains itself with the existing .img-empty message
and the command, which stays selectable (neither a click nor a zoomed-in
drag on it toggles or pans). Badges and the `N new` pill use the
palette's accent colour, neither pass nor fail. Re-recorded screenshots
are counted in a pill when present but not listed; listing them with a
before/after against the previous baseline is a separate follow-up.

Not changed: the reporter contract and every reporter's state handoff;
terminal output under :once and :all except that a failed capture is no
longer announced as new; other failure messages; how failure entries
render. Also fixes README's legacy
`capybara_screenshot_diff/reporters/html` require.

ScreenshotAssertion#display_path drops its `rescue ArgumentError`, like
Reporters::Default's did: a path Pathname can't relate to the root is a
misconfiguration to surface, not hide; the report's copy has none either.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oboxodo

oboxodo commented Oct 13, 2026

Copy link
Copy Markdown
Owner Author

Superseded by snap-diff#307.

@oboxodo oboxodo closed this Oct 13, 2026
@oboxodo
oboxodo deleted the fix/report-new-screenshots branch October 14, 2026 01:03
@oboxodo
oboxodo restored the fix/report-new-screenshots branch October 15, 2026 14:54
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.

1 participant