Skip to content

test: carry the snapshot fixes onto main ahead of the CI migration - #48

Merged
eliotmcintire merged 4 commits into
mainfrom
fix/snapshot-tests-on-main
Sep 4, 2026
Merged

test: carry the snapshot fixes onto main ahead of the CI migration#48
eliotmcintire merged 4 commits into
mainfrom
fix/snapshot-tests-on-main

Conversation

@eliotmcintire

@eliotmcintire eliotmcintire commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Cherry-picks (with -x) the four test commits already merged into development via #45, #46 and #47.

Commit here From What
4f9ebd1 57600bb (#46) skip_if_not_installed("raster") in the 9 snapshot-taking test_that() blocks
0d86dff b9a1ce6 (#45) the rePlot snapshot gets its own name
ab7d126 2724b9d (#45) the three missing reference images
9d365b2 cb4637e (#47) the four accepted Windows antialiasing images

Why this needs to happen before, not after, #44

main and development differ in tests/testthat/test-Plot-snapshots.R by exactly one thing — res = → resolution = (3b84c9c, development only). main carries all four of the same snapshot bugs, and has since long before the CI work started.

They have simply never been visible: the current R-CMD-check.yaml on main triggers on [master, development], and this repo has no master branch, so R-CMD-check has never once run against main. #44 changes the triggers to [main, development]. The moment it lands, main gets its first R-CMD-check in the repo's history and it reproduces:

  • a hard ERROR on every Linux and Windows leg (duplicate snapshot name)
  • a missing-fixture failure on every Linux leg
  • four antialiasing failures on every Windows leg
  • FAIL 11 on the new nosuggests leg

Landing this first means main's first-ever CI run is green.

Verification

Two things are checked, and one cannot be:

  • ✅ tests/testthat/_snaps/ on this branch is byte-identical to development's (git diff empty).
  • ✅ test-Plot-snapshots.R differs from development's only by the pre-existing res = / resolution = split and two blank lines — no cherry-pick drift.
  • ⚠️ No R-CMD-check will run on this PR. main's current workflow does not trigger on PRs into main, so only pkgdown runs here. The same four commits are green on development — run 33921573171, all 9 legs plus coverage.

Suggested order

  1. Merge this.
  2. Re-run ci: move every workflow to the org reusable templates #44's CI against the updated main. It should go from FAIL 5/FAIL 2/FAIL 11 to green across the whole new matrix — that is the real verification.
  3. Merge ci: move every workflow to the org reusable templates #44.

Not included

3b84c9c ("fix partial argument match warning", res = → resolution =) is still development-only. It is unrelated to CI — R only warns on a partial argument match under warnPartialMatchArgs, which neither the tests nor R CMD check enable — so it is left for the next release merge rather than folded in here.

🤖 Generated with Claude Code

https://claude.ai/code/session_011kpoQV6nNqcknk16R3RzBB

eliotmcintire and others added 4 commits September 4, 2026 15:52
@eliotmcintire @claude
test: skip image snapshots when raster is not installed
4f9ebd1 
The image snapshots in test-Plot-snapshots.R were recorded with the full
Suggests set installed, and they are not portable to a run without it --
not because the rendering differs, but because the *names* do.

Several tests build a `rasts` list and then double its length inside
`if (requireNamespace("raster", quietly = TRUE))`. Snapshot file names come
from `fn()` (helper-testInit.R), which pulls from a running `counter` over
those lists. Drop `raster` and every list shortens, so each name after the
first raster block lines up against a different plot. "internal functions
in Plot" additionally computes `val <- (testNum - 1) %% (length(rasts) / 2) + 1`,
hard-coding the doubling, so without raster it renders plots 1 and 2 twice
and never renders 3 or 4.

This is exactly the `_R_CHECK_DEPENDS_ONLY_` leg that PredictiveEcology/actions'
shared R-CMD-check matrix runs: 11 failures there, all in this file, all of
them a reference image compared against the wrong plot.

Maintaining a second set of reference images for that configuration would be
a lot of pixels for no extra signal, so skip the image comparisons when
raster is absent. The nosuggests leg keeps its real purpose -- the package
loads and its non-image tests pass without Suggests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011kpoQV6nNqcknk16R3RzBB
(cherry picked from commit 57600bb)
@eliotmcintire @claude
test: give the rePlot snapshot its own name
0d86dff 
`test_that("rePlot doesn't work")` took two snapshots under one name: it
called `announce_snapshot_file()` + `expect_snapshot_file()` twice with the
same `fil`, `unlink()`ing the file in between. testthat rejects a duplicate
snapshot name outright, so the second call errored and aborted the test:

  Error ('test-Plot-snapshots.R:612:5'): rePlot doesn't work
  Snapshot file names must be unique. "rePlot_doesn't_work1Linux.png" has
  already been used.

This is a plain ERROR on every Linux and Windows leg (macOS is unaffected --
`oses` is c("Win", "Linux"), so no snapshot runs there), and it predates the
CI migration: the same failure is in the development-branch run from
2026-08-29, run 33278110145.

The second snapshot now gets a name derived from the first rather than a
second `fn()` call, because `fn()` bumps the shared `counter` and that would
renumber every snapshot after this one. `unlink(fil)` goes with it -- it only
existed to clear the path for reuse.

Two new reference images follow in the next commit, recorded from CI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011kpoQV6nNqcknk16R3RzBB
(cherry picked from commit b9a1ce6)
@eliotmcintire @claude
test: record the three missing snapshot fixtures
ab7d126 
All three are taken from this branch's own CI run 33897686977, not rendered
locally -- expect_snapshot_file() compares PNG bytes exactly and those bytes
depend on the runner's cairo and fontconfig.

  rePlot_doesn't_work0Win-rePlot.png     (windows-latest, release)
  rePlot_doesn't_work1Linux-rePlot.png   (ubuntu-latest, release)
  Plot_listsggplotVnewer2Linux.png       (ubuntu-latest, release)

The two rePlot images are byte-identical to their non-rePlot counterparts
already in the repo -- which is exactly the assertion the original code was
reaching for with "same file for snapshot b/c basename is same as previous":
rePlot() reproduces the plot pixel for pixel. It just could not express that
through a snapshot name testthat had already seen.

Plot_listsggplotVnewer2Linux.png closes a pre-existing gap unrelated to this
branch: only the ...2Win.png variant had ever been committed, so
test-Plot-snapshots.R:703 reported "Adding new file snapshot" on every Linux
leg, including in the development run from 2026-08-29.

As a side check, the Linux and Windows artifacts from that run reproduce the
already-committed rePlot_doesn't_work1Linux.png and rePlot_doesn't_work0Win.png
byte for byte, so those legs are stable and the new files are recorded against
the same rendering.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011kpoQV6nNqcknk16R3RzBB
(cherry picked from commit 2724b9d)
@eliotmcintire @claude
test: accept the four Windows antialiasing snapshots
9d365b2 
  Issue_20;_arr_working0Win.png                  4 px changed
  Issue_32_Plot_factors_lower_case_id0Win.png    1 px changed
  Issue_32_Plot_factors_lower_case_id2Win.png    1 px changed
  Issue_32_Plot_factors_lower_case_id4Win.png    1 px changed

Seven pixels out of 1,920,000, max channel delta 58/255, and in every case
the changed pixels sit inside a text glyph -- a glyph of the plot title
`maps$forestAge` for Issue 20, the legend label `A` for the three Issue 32
images. No mark, axis, colour ramp or raster cell moved. This is the
windows-latest runner's font rasteriser rounding an antialiased edge
differently than it did when the references were recorded.

Taken from run 33899141206, which produced byte-identical renders on all four
Windows legs (release, devel, oldrel-1, oldrel-2), so one accepted set clears
all four. The same bytes also came back from runs 33779375229 and 33897686977,
i.e. the new rendering is stable, not a flake.

expect_snapshot_file() compares PNG bytes exactly, so a one-pixel change in a
font rasteriser is indistinguishable from a real regression. A custom
`compare =` with a small per-pixel budget would keep the signal and drop this
class of failure; not attempted here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011kpoQV6nNqcknk16R3RzBB
(cherry picked from commit cb4637e)
@eliotmcintire
eliotmcintire merged commit 9c8b3dd into main Sep 4, 2026
2 checks passed
@eliotmcintire
eliotmcintire deleted the fix/snapshot-tests-on-main branch September 4, 2026 23:02
@eliotmcintire eliotmcintire mentioned this pull request Sep 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@eliotmcintire