Skip to content

Conversation

@jonthegeek
Copy link
Contributor

Name snap_dir arg for testthat::local_snapshotter() in test_coverage_active_file()

Fixes #2630

Name `snap_dir` arg for `testthat::local_snapshotter()` in `test_coverage_active_file()`

Fixes r-lib#2630
@jonthegeek
Copy link
Contributor Author

It's a little ironic for this not to have test coverage, but the tests here are pretty light, and it's tricky to add them. I could probably get some things in place to at least throw errors if something like this happens again if you'd like.

@fabiandistlerkb
Copy link

Thank you. This has been annoying me for the past weeks. I got a local quickfix. Is there a chance to get this into the dev version quickly?

@hadley
Copy link
Member

hadley commented Jan 8, 2026

Hmmm, I haven't noticed this because I haven't updated devtools for a long time 😬

@hadley
Copy link
Member

hadley commented Jan 8, 2026

Let me also fix this in testthat, since I'm about to release it. We'll still want this change because it's definitely better practice to use a named argument here.

hadley added a commit to r-lib/testthat that referenced this pull request Jan 8, 2026
hadley added a commit to r-lib/testthat that referenced this pull request Jan 8, 2026
Copy link
Member

@jennybc jennybc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sort of nitpicky/meta but sort of legit and I know you are resilient enough @jonthegeek to take it 😁 but can you back out the Air-ish formatting changes, so this only has the meaningful change? I think it's just the addition of snap_dir =?

(I need to do a holistic re-formatting of devtools as a whole. And it really does make it easier to see what's in this PR / what the problem is.)

@jonthegeek
Copy link
Contributor Author

This is sort of nitpicky/meta but sort of legit and I know you are resilient enough @jonthegeek to take it 😁 but can you back out the Air-ish formatting changes, so this only has the meaningful change? I think it's just the addition of snap_dir =?

(I need to do a holistic re-formatting of devtools as a whole. And it really does make it easier to see what's in this PR / what the problem is.)

I almost did this earlier today because I didn't intentionally make them (air and/or "Strip trailing horizontal whitespace when saving" did). It's a little tricky to save WITHOUT applying those things but I can work it out!

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.

test_coverage_active_file() fails with testthat 3.3

4 participants