Skip to content

Fix snapshot_accept/review filtering with variant snapshots - #2332

Open
cderv wants to merge 2 commits into
r-lib:mainfrom
cderv:fix/snapshot-meta-variant-filter
Open

Fix snapshot_accept/review filtering with variant snapshots#2332
cderv wants to merge 2 commits into
r-lib:mainfrom
cderv:fix/snapshot-meta-variant-filter

Conversation

@cderv

@cderv cderv commented Apr 24, 2026

Copy link
Copy Markdown

Fixes #2325.

I ran into this while working on the tufte package, where snapshot tests use variants for different pandoc/citeproc versions. After a test failure, snapshot_review("html") was reporting "No snapshots to update" even though a .new. file clearly existed.

I stepped through snapshot_meta() and the issue is in the filtering logic. For a variant snapshot like _snaps/my-variant/html.md, the name column ends up as "my-variant/html.md", so matching against c("html", "html.md") fails. The documented behavior for the files argument says it can be "a snapshot name (e.g. foo or foo.md)", which I believe should work regardless of whether the snapshot lives in a variant subdirectory.

The fix adds a basename(out$name) check to the filter. I kept the existing out$name %in% files check because it's still needed for fully qualified paths like snapshot_accept("my-variant/html.md").

I also added tests for filtering by test name with variants, including the case with multiple variant directories.

@cderv

cderv commented Apr 24, 2026

Copy link
Copy Markdown
Author

The CI failures are unrelated to this PR — they're about a new R4.7 variant that doesn't have snapshots recorded yet:

── Failure ('test-snapshot-file.R:35:3'): expect_snapshot_file works with variant ──
Adding new file snapshot: 'tests/testthat/_snaps/R4.7/snapshot-file/version.txt'
── Failure ('test-snapshot.R:2:3'): variants save different values ─────────────
Adding new snapshot for variant 'R4.7':
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant