test: track the nine orphaned vdiffr baselines; fix the .Rcheck ignore pattern - #186
Open
ehrlinger wants to merge 1 commit into
Open
test: track the nine orphaned vdiffr baselines; fix the .Rcheck ignore pattern#186ehrlinger wants to merge 1 commit into
ehrlinger wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #186 +/- ##
=======================================
Coverage 89.02% 89.02%
=======================================
Files 50 50
Lines 4618 4618
=======================================
Hits 4111 4111
Misses 507 507 🚀 New features to boost your workflow:
|
…e pattern
Nine SVGs under tests/testthat/_snaps/snapshots/ have never been tracked on
any branch, though they are baselines for nine live expect_doppelganger()
calls in test_snapshots.R. That left them in the worst of both states: not
protected by git, and not deliberately ignored either.
Two consequences, one loud and one quiet:
- Loud: a suite run with VDIFFR_RUN_TESTS unset prunes them as "unused",
and `git restore` cannot bring them back because git never had them.
On 2026-08-06 they survived only because a stale copy happened to be
left in ggRandomForests.Rcheck/00_pkg_src/. That is not a backup.
- Quiet: vdiffr writes a baseline and passes when none exists, so on any
fresh clone those nine visual-regression tests could never fail.
Tracking them is what the package's own convention already prescribes --
test_snapshots.R:18 says "Commit tests/testthat/_snaps/ to the repo" -- and
puts them on the same footing as their 40 committed siblings, which share
the same Aug 6 generation run.
Verified rather than assumed: NOT_CRAN=true VDIFFR_RUN_TESTS=true
devtools::test(filter = "snapshots") reports FAIL 0 | SKIP 0 | PASS 49
against main, with no .new.svg written, so these baselines match current
output and are not stale recoveries. lintr::lint_package() reports 0 lints.
Separately, .gitignore carried `..Rcheck` and `..Rcheck/*`. A dot is a
literal in gitignore syntax, so those matched only a path named `..Rcheck`,
which nothing creates -- an evident typo for `*.Rcheck`. The 12 MB
ggRandomForests.Rcheck/ was therefore never ignored while the file looked
like it handled it. Replaced with `*.Rcheck/` and dropped the redundant
second line. Confirmed it catches the directory and its contents without
matching R/ or tests/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ehrlinger
force-pushed
the
claude/lucid-pascal-05aa38
branch
from
August 11, 2026 18:06
ae1824a to
e58c198
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two repo-hygiene fixes that were both sitting in an ambiguous "neither protected nor deliberately ignored" state.
1. Nine vdiffr baselines were untracked on every branch
tests/testthat/_snaps/snapshots/holds 49 SVGs. Forty are tracked; nine never have been, on any branch (git log --allon them is empty — this is not a deletion to recover, they were simply never added).All nine correspond to live
expect_doppelganger()calls intest_snapshots.R— there are exactly 49 such calls and 49 SVGs, a 1:1 map.Two consequences:
VDIFFR_RUN_TESTSunset prunes them as "unused", andgit restorecannot recover them because git never had them. On 2026-08-06 they survived only because a stale copy happened to remain inggRandomForests.Rcheck/00_pkg_src/. That is not a backup and will not reliably be there.Tracking them is what the package's own convention already prescribes —
test_snapshots.R:18reads "Committests/testthat/_snaps/to the repo".2.
.gitignorenever actually ignored*.RcheckThe file carried
..Rcheckand..Rcheck/*. A dot is a literal in gitignore syntax, so those patterns matched only a path named..Rcheck, which nothing creates — an evident typo for*.Rcheck. The 12 MBggRandomForests.Rcheck/was therefore never ignored, while.gitignorelooked like it handled it. That false coverage is why a build directory sat exposed to a straygit add -A.Replaced with
*.Rcheck/; dropped the redundant second line.Verification
Rebased onto
mainafter #185 merged, and re-verified. #185 changedR/calc_roc.R,R/gg_roc.R,R/plot.gg_roc.R,R/gg_error.R,R/gg_vimp.RandR/plot.gg_rfsrc.R— the plotting code behind two of these nine baselines (gg-roc-multiclass-*) and a dozen of the tracked forty. GitHub reported the PRMERGEABLE / CLEANthroughout, but that is only textual; it says nothing about whether the SVGs still match. Re-ran rather than relied on it:NOT_CRAN=true VDIFFR_RUN_TESTS=true devtools::test(filter = "snapshots")against the post-fix: address Codex review items for 3.5.1 #185 base → FAIL 0 | WARN 22 | SKIP 0 | PASS 49, with no.new.svgwritten. So fix: address Codex review items for 3.5.1 #185's changes did not move any baseline, and these nine match current output — they are not stale recoveries, and are exactly as fresh as their 40 committed siblings (all 49 share one Aug 6 generation run).VDIFFR_RUN_TESTS=trueguard behaved as documented.lintr::lint_package()→ 0 lints (re-run after fix: address Codex review items for 3.5.1 #185, which also modifiedtest_lint.R).git check-ignore -vconfirms*.Rcheck/catchesggRandomForests.Rcheck/and its contents, and does not matchR/ortests/..Rcheckpath, so the new rule cannot orphan anything already in the index.Heads-up: this merge will abort your next
git pullThe nine files also exist as untracked copies in the primary local checkout. Git refuses to overwrite untracked files on checkout even when the content is byte-identical (verified empirically, not assumed), so pulling this will fail with:
Remedy — delete the local untracked copies first, then pull; the identical tracked versions arrive with the merge. From the repo root:
Not folded in (flagging, not fixing)
.Rbuildignorehas no.Rcheckentry. Low severity given the release gate builds from a cleangit archiveexport, but worth a separate look.🤖 Generated with Claude Code