Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use parameterized test on public API for implicit_integer_linter() #1693

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

MichaelChirico
Copy link
Collaborator

Part of #1692

@MichaelChirico MichaelChirico added internals Issues related to inner workings of lintr, i.e., not user-visible testing labels Oct 13, 2022
Copy link
Collaborator

@IndrajeetPatil IndrajeetPatil left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Btw, why do you use local() with {patrick} tests?
I didn't use it for the tests I wrote, and I can change that if I understand the reasoning here.

@MichaelChirico
Copy link
Collaborator Author

Btw, why do you use local() with {patrick} tests?

because of the extra variables associated with the test (cases, int_max, cases_int_max)... those are needed for the test but make the tests less "hermetic" by polluting the local environment (can possibly be seen by other tests, by mistake). local() ensures those are removed "on.exit" from the local() execution frame.

There's another approach that could work with {rlang} but I prefer the local() one for now...

r-lib/withr#205

@MichaelChirico MichaelChirico merged commit 1280a4e into main Oct 13, 2022
@MichaelChirico MichaelChirico deleted the int-tests branch October 13, 2022 06:42
@MichaelChirico
Copy link
Collaborator Author

(with plain {testthat} tests, this is encapsulated in test_that(), but we need this workaround for {patrick}... hmm maybe that could be improved upstream...)

@IndrajeetPatil
Copy link
Collaborator

make the tests less "hermetic" by polluting the local environment (can possibly be seen by other tests, by mistake)

I see. Thanks for clarifying!

My understanding was that objects created in each test file live in their own local environments and won't be accessible from other test files. But maybe there are contexts (like {patrick} tests) where this is possible?

@MichaelChirico
Copy link
Collaborator Author

My understanding was that objects created in each test file live in their own local environments

I think that's right, but there are also the other tests in the same file

PS filed: google/patrick#13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants