Skip examples or tests that need credentials if not available#484
Skip examples or tests that need credentials if not available#484
Conversation
|
There is no issue for this PR |
|
The package is also too big to be sent via |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #484 +/- ##
==========================================
+ Coverage 85.65% 86.74% +1.09%
==========================================
Files 26 26
Lines 746 747 +1
==========================================
+ Hits 639 648 +9
+ Misses 107 99 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This seems to have a very negative effect on package coverage, does covr somehow skip these tests as well? It should have access to credentials since it worked just fine before! |
|
CI also has much more skips than expected, something is wrong with my helper and covr caught it! |
PietrH
changed the title
man/get_acoustic_detections.Rd
Outdated
| } | ||
| \examples{ | ||
| \dontshow{if (etn:::credentials_are_set()) withAutoprint(\{ # examplesIf} | ||
| # example code |
There was a problem hiding this comment.
Is this the literal comment # example code? If yes, then I would remove.
There was a problem hiding this comment.
I'm also curious how
dontshow{if (etn:::credentials_are_set()) withAutoprint(\{ # examplesIf}
Renders on the pkgdown website.
There was a problem hiding this comment.
Because the system that renders the pkgdown website has access to credentials (CI, or dev machine) it'll just run the examples. CRAN for example, will not.
Note the progress is still reported even though the function is not called interactively, maybe I should stop that from happening.
| @@ -1,3 +1,5 @@ | |||
| skip_if_no_authentication() | |||
There was a problem hiding this comment.
Does this need to be done outside a test_that() function?
There was a problem hiding this comment.
Same for other occurrences where the function is called at the beginning of the test file
There was a problem hiding this comment.
No, but this has the advantage of skipping all tests of the test-* file without me having to repeat it for every test. But it has the disadvantage of not counting the number of tests that are skipped.
I took the easiest route most of the time, do you prefer me switching to skipping in the test_that({}) context every time?
There was a problem hiding this comment.
Yeah, I think it's probably better to skip within the test (best practice).
There was a problem hiding this comment.
Alright, I'll switch that around and let you know.
Co-Authored-By: Peter Desmet <peter.desmet@inbo.be>
…o credentials known
PietrH
force-pushed
the
pass-win-devel
branch
from
040cc28 to
0b5a88c
Compare
|
Tested on the rstudio server to ensure it also skips when it's supposed to skip when there is db access. Made a few changes in the hopes to fix #494 |
2 participants
Some examples or tests need credentials to be stored in order to run. This is no big deal but will break CRAN submission or using
devtools::check_win_release(). This PR gets this out of the way.I add two helpers, one to check if credentials are set, this is used for conditional examples and for the other function: a testthat skip helper that skips a test if no credentials are set.
It should be easy to adapt these helpers to keyring later on. They may need no modification whatsoever.
Possible followup: Some tests are already skipped if there is no local database connection, in fact, these should also be skipped if there are no credentials set. I suppose I could repeat the skip or call it from within
skip_if_not_localdb().This is a nice to have, and not a blocker for the ETN API release.
I think I was done working on this, I can't see why I didn't open this for review earlier.