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

Option to turn off helper loading #1169

Closed
wants to merge 4 commits into from

Conversation

pitakakariki
Copy link

I'd prefer not to load my testthat helpers with load_all.

I set a bunch of options like default number of simulations and progress bars which need to be different between interactive use and testing.

nb: Relevant issue is #1125

@codecov-io
Copy link

Current coverage is 41.91%

Merging #1169 into master will increase coverage by +0.17%

@@             master      #1169   diff @@
==========================================
  Files            86         86          
  Lines          4464       4464          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1863       1871     +8   
+ Misses         2601       2593     -8   
  Partials          0          0          
  1. File R/github.R (not in diff) was modified. more
    • Misses -8
    • Partials 0
    • Hits +8

Powered by Codecov. Last updated by c19c6f6...bded5f5

@jimhester jimhester added this to the 1.11.2 milestone May 31, 2016
@hadley
Copy link
Member

hadley commented Jun 2, 2016

I'd rather not add an argument for this, but instead provide some other way to scope your helpers so they work differently between formal and informal testing.

@@ -86,7 +87,7 @@
#' }
#' @export
load_all <- function(pkg = ".", reset = TRUE, recompile = FALSE,
export_all = TRUE, quiet = FALSE, create = NA) {
export_all = TRUE, helpers=TRUE, quiet = FALSE, create = NA) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please put spaces around =?

Copy link
Author

Choose a reason for hiding this comment

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

Spaces are there now.

@krlmlr
Copy link
Member

krlmlr commented Jul 8, 2016

#1256 (now merged) relies on the helpers being loaded already by load_all(). We'll need to use helpers = TRUE there, too, or use helpers = FALSE and revert #1256.

@krlmlr
Copy link
Member

krlmlr commented Jul 8, 2016

On that note, I think it would be great if helpers = TRUE added testthat to the search path, so that the testthat functions are available. @hadley: What do you think?

@hadley
Copy link
Member

hadley commented Aug 1, 2017

@pitakakariki any interest in merging and updating? We'll be working on devtools for the next few weeks.

@pitakakariki
Copy link
Author

@hadley will do. Should I wait till devtools is passing on Appveyor or is it okay to do it now?

@pitakakariki
Copy link
Author

I've sent a pull request to pkgload here r-lib/pkgload#41

@hadley hadley closed this Aug 2, 2017
@pitakakariki pitakakariki deleted the patch-1 branch August 2, 2017 19:31
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.

5 participants