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

options(warn = 1) by default? #74

Open
HenrikBengtsson opened this issue Feb 25, 2021 · 5 comments
Open

options(warn = 1) by default? #74

HenrikBengtsson opened this issue Feb 25, 2021 · 5 comments

Comments

@HenrikBengtsson
Copy link
Contributor

WRE now (wch/r-source@3a3073b) says:

@item
Tests should use @code{options(warn = 1)} as reporting

@example
There were 22 warnings (use warnings() to see them)
@end example

@noindent
is pointless, especially for automated checking systems.

Is this something that tinytest could do by default, e.g. in run_test_file()?

PS. This is just a comment; I'm new to tinytest, so sorry if this has already been discussed or is out of the scope of the design/philosophy.

@markvanderloo
Copy link
Owner

Thanks, I was unaware of this WRE requirement.

It seems like a good default, especially for test_package as this is the command that is intended to be used to trigger tests in R CMD check.

@markvanderloo
Copy link
Owner

There are some other things I should set, e.g. make sure that the sorting locale is C like in R CMD check. cf #69. In the works.

@eddelbuettel
Copy link
Contributor

Interesting suggestion!

But I am not sure if always turning it on is right? Maybe making this is a toggle is better. I.e. in my for-work package, I warn on use of a deprecated-but-supported interface. That counts as a warning yet I know full well it is there. Making the test fails does not help.

Or maybe we add a per-file 'un-setter' of this default?

@eddelbuettel
Copy link
Contributor

Come to think about it, I could locally reset options(warn) before hitting the deprecated calls. But then I get dinged with a 'side-effects triggered'. Less bad than a fail, I suppose...

@markvanderloo
Copy link
Owner

For packages we can probably get away with adding the option setting to tests/tinytest.R, so

options(warn=1)
tinytest::test_package("foo")

I can add this to setup_tinytest() and ppl can do it explicitly if they run tests interactively. waddayathink?

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

No branches or pull requests

3 participants