Preserve snapshots for tests that were not run

[hadley]

Fixes #2066

@jennybc please kick the tires on this.

[claude]

Claude finished @hadley's task —— View job

---

Code Review Complete

• Read and analyze changed files
• Check for potential bugs or issues
• Provide review feedback

Review Findings

Issue: The fix preserves snapshots for tests that were not run when filtering with desc. The core logic looks sound, but found a few areas for improvement:

1. Missing parameter documentation in R/snapshot-reporter.R:229: The new desc parameter in local_snapshotter() lacks roxygen documentation. Add @param desc documentation.

2. Potential edge case in R/snapshot-reporter.R:49: The startsWith() logic assumes test names don't contain / characters, but there's no validation. Consider adding a check or clearer documentation about this limitation.

3. Function signature consistency: test_description() now takes an optional parameter but existing calls assume it's parameterless. Good defensive programming to maintain backward compatibility.

Code quality: Clean implementation with appropriate test coverage. The filtering logic correctly preserves order by setting matched elements to NULL rather than removing them.

Test coverage: Good - the new test "can filter with desc" validates the core functionality properly.

LGTM overall. The minor documentation issue is worth addressing.

[jennybc]

LGTM! I checked with my original reprex and also checked that this new test fails when applied to main. I pushed one small commit with another skip, which seems consistent with what I see going on elsewhere (unrelated but came across my radar while reviewing).

[hadley]

@jennybc FWIW testthat should now automatically skip suggested packages that are not installed.

[jennybc]

FWIW testthat should now automatically skip suggested packages that are not installed.

I saw a different result, but I can't say what version of testthat I had installed at that moment. I only made R 4.5 my default version very recently and am still discovering packages I haven't installed yet on R 4.5. I think I noticed the snapshot change (due to not having R7 installed) when I did the preliminary R CMD check right after checking out this PR, but before I actually installed it. So ... older testhat was being used to test newer testhat, I guess 🤔

[jennybc]

Or does the automatic skip maybe only apply on CRAN?

[hadley]

@jennybc oh yeah, CRAN only.