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

WISH: Add option/argument to run through all tests although some give errors #140

Closed
HenrikBengtsson opened this issue Jan 9, 2016 · 5 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@HenrikBengtsson
Copy link
Contributor

I'd like to suggest to add an option/argument so that covr keeps running through all tests instead of stopping whenever a test fails. Something like run_tests(..., on_error="warn").

Why? I once in a while find that covr gives errors when R CMD check doesn't. If it occurs on my local machine, I can often either troubleshoot and find a fix, or simple skip the test when covr is loaded. However, when I cannot reproduce the problem locally and it appears on, say, Travis CI, it is complicated and tedious to troubleshoot via trial and error commit/pushing. Having an option to have covr run through all tests would help, because then you could see what other tests pass/fail, which helps out rule things and narrow in on the troubleshooting. See for instance https://travis-ci.org/HenrikBengtsson/future/jobs/101287469#L2564-L2567 (where the covr tests just stops which I assume is due to a test error).

@jimhester jimhester added the bug an unexpected problem or unintended behavior label Jan 14, 2016
@jimhester
Copy link
Member

I assume this is happening regardless of use_try setting? If so it is a bug of some kind I will have to look into it.

@HenrikBengtsson
Copy link
Contributor Author

Thanks for pointer, using use_try=FALSE definitely gives a more informative error message with a call trace, e.g. https://travis-ci.org/HenrikBengtsson/future/jobs/102397272#L5179-L5239. That is certainly helpful for troubleshooting.

Next, the reason for tests failing under covr (pun not intended, but i's a good one) and not with R CMD check is with covr::package_coverage() the package I'm tested (future) is not available/installed when spawning off background R session via parallel::makeSOCKcuster() and then that session tries to library('future'). I assume it's happening because this manually spawned-off background R session does not see the same .libPaths() as the main covr session does. It seems like R CMD check does setup the same .libPaths() for spawned of R sessions but covr doesn't, but it might be more complicated than that. Also, I can indeed reproduce this locally if I use temporary .libPaths() where only covr et al. is installed. This problem reveals itself on Travis because of how (at least we/I) test R packages on Travis.

Please feel free to create a separate issue for the above.

Regarding my original post; what I would like to see is an option for use_try=TRUE / try(...) that applies to the individual source() calls rather than on the whole directory of tests.

HenrikBengtsson added a commit to HenrikBengtsson/future that referenced this issue Jan 16, 2016
@jimhester
Copy link
Member

I just ran into the .libPaths() inheritance issue with another package, I opened #147 to track it.

@jimhester
Copy link
Member

@HenrikBengtsson Could you verify https://github.com/jimhester/covr/tree/libpaths fixes the issue you were seeing with the libpaths not being passed to the spawned process (I believe it should). Thanks!

@HenrikBengtsson
Copy link
Contributor Author

Thanks for this. I rerun the exact same build on Travis CI and it seems to go a bit further now. However, there might be a related problem with happens with using PSOCK cluster nodes with covr. In https://travis-ci.org/HenrikBengtsson/future/jobs/109668979#L2606, I get:

Error : there is no package calledfuture

The package test tests/demo.R creates a "multisession" cluster, which is a localhost PSOCK cluster created as cl <- parallel::makeCluster(2L). Then it runs demo("mandelbrot", package="future"). As part of this, it tries to load the future package on the cluster nodes via expression future::plan(future::eager). I suspect this is where the error occurs. If so, this is a further indication that there is a library path issue.

If my guesses are correct, something like the following would be a minimal example (but I don't have time to try it out right now):

library("parallel")
cl <- makeCluster(2L)  ## homogeneous=TRUE
clusterCall(cl, fun=function() { loadNamespace("<pkg-being-tested>") })
stopCluster(cl)

I'm not sure if covr can handle this automagically; my guess is that parallel:::defaultClusterOptions$rlibs needs to reflect .libPaths() of the main process and I don't know if that options already does that now or not.

I have to leave it at this for now, but thought it would be better to report at least something at this point. I'll let you know when/if I learn something new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants