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

devtools::test(...) passes but tests fail in covr::package_coverage(...) #487

Closed
hechth opened this issue Aug 18, 2021 · 18 comments
Closed

Comments

@hechth
Copy link

hechth commented Aug 18, 2021

Running my tests of recetox-xMSannotator using devtools::test() works completely fine, but calling covr::package_coverage() fails in finding local test data.

Could it be that some working directories are changed during the coverage measurement?

@jimhester
Copy link
Member

covr runs its tests with tools::testInstalledPackage(), which is closer to how the tests are run by R during R CMD check.

The first thing to do is see if you have the same failures using devtools::check() rather than test().

In terms of constructing paths in your tests the best practice is to rely on testthat::test_path() rather than trying to do it yourself.

@hechth
Copy link
Author

hechth commented Aug 18, 2021

Thanks for the quick reply! I'm currently also looking into the devtools::check() command but there are quite some other issues apparently - I will try to clean that up and see if the problem persists.

@hechth
Copy link
Author

hechth commented Aug 25, 2021

By now I am at a point where devtools::test(), devtools::check() and tools::testInstalledPackage('xMSannotator') all work, but covr::package_coverage()` still fails the tests - any hints?

@jimhester
Copy link
Member

If you are talking about hechth/recetox-xMSannotator@9b770b5 I still get test failures with devtools:test() and devtools::check() has lots of issues.

e.g.

Warning (test-advanced_annotation.R:5:1): (code run outside of `test_that()`)
The `code` argument to `test_that()` must be a braced expression to get accurate file-line information for failures.
Backtrace:
 1. patrick::with_parameters_test_that(...) test-advanced_annotation.R:5:0
 2. purrr::pmap(all_pars, build_and_run_test, desc = desc_stub, code = captured)
 3. patrick:::.f(...)
 4. testthat::test_that(completed_desc, rlang::eval_tidy(code, args))

Error (test-advanced_annotation.R:21:3): Advanced annotation works: qc_matrix
Error: the condition has length > 1
Backtrace:
 1. rlang::eval_tidy(code, args)
 2. xMSannotator::multilevelannotation(...) test-advanced_annotation.R:21:2

Error (test-advanced_annotation.R:18:3): Advanced annotation works: batch1_neg_hmdb
Error: cannot open the connection
Backtrace:
 1. rlang::eval_tidy(code, args)
 2. base::readRDS(peaks_filepath) test-advanced_annotation.R:18:2
 3. base::gzfile(file, "rb")

Error (test-advanced_annotation.R:18:3): Advanced annotation works: sourceforge
Error: cannot open the connection
Backtrace:
 1. rlang::eval_tidy(code, args)
 2. base::readRDS(peaks_filepath) test-advanced_annotation.R:18:2
 3. base::gzfile(file, "rb")

I can't really help you further until I can run the tests cleanly on my machine.

@hechth
Copy link
Author

hechth commented Aug 27, 2021

Yes, the most recent commit in which I changed the tests is here: hechth/recetox-xMSannotator@71b5854. On my machine it now all passes, also the codecov works since I altered the test condition a bit.

In the refactoring I did some changes which apparently cause few rows in the output dataframes to be different, but it seems to be minor. The rather interesting behaviour is that the differences are different, depending on whether I run devtools::test() or tools::testInstalledPackage(). Now with the more relaxed criterium it all passes, but still it is interesting that the two ways of testing would give different results. But as I am not the original developer of the package, I can't say that I understand everything what is happening inside the code

@hechth
Copy link
Author

hechth commented Oct 5, 2021

@jimhester I'm again at a point where devtools::test(...) and even devtools::check(...) pass, but the tests using tools::testInstalledPackage(...) fail - do you know someone who maybe has a better insight into the internals of R, who can explain why these ways of testing behave differently?

Thank you so much for the time and support, I really appreciate it and it helps me a lot in making our code better and cleaner :)

@jimhester
Copy link
Member

jimhester commented Oct 5, 2021

There are a number of different potential issues, hadley/r-pkgs#483 documents some of them.

Again I tried running your package (hechth/recetox-xMSannotator@3820ca3) tests and check locally and ran into failures with both.

You might try pulling the GitHub repo into a new directory and verifying the tests work on your machine afterwards.

I could probably diagnose the issue pretty quickly if I can run the tests myself.

@hechth
Copy link
Author

hechth commented Oct 5, 2021

@jimhester you need to fetch the test data (it is quite something, so we moved it to a different repo (should be publicly readable, just copy the content of the testdata/recetox-xMSannotator folder to the testdata folder in the repo.

On my machine (as well as a dev server), the tests using devtools::test() as well as devtools::check() pass.

Thank you so much for the help, I'll look into the issues you provided.

@privefl
Copy link

privefl commented Nov 19, 2021

I guess I have a similar issue with my package {bigsnpr}.

Could this be an issue with some outdated cache used by {covr}?

@dewittpe
Copy link

dewittpe commented Feb 1, 2022

I have ran into this issue with one of my packages, cpr.

I think it might have something to do with reading .rds files within a test, or rather, from where the .rds files are read.

Tests in version 03b0898 will pass locally and on several other OS and R versions, see the logs for the github actions R-CMD-check and test-coverage. In this version, there are two .rds files in the tests/testthat directory which are read during the evaluation of the tests. The coverage and one of the runners fail due to testing.

On the next commit, d1024f7, the tests tests/testthat/test-cpr.R and test/testthat/test-cnr.R were commented out and the R-CMD check and code coverage do not error.

Finally, on commit ce5021f I moved one of the .rds files from test/testthat/ to inst/example_objects and changed the what the testing script reads in the .rds file to use system.file. For this commit my local devtools::test, devtools::check, and covr::package_coverage() all pass. The github actions R-CMD-check and test-coverage also passed.

From these steps it seems like the file location might be part of the cause for the different behavior between checks, tests, and covr.

@privefl
Copy link

privefl commented Feb 2, 2022

Thanks for reporting your solution @dewittpe.

In the test file that fails in the coverage, I do read RDS files, but they are all located in inst/ and I read them with system.file() (cf. https://github.com/privefl/bigsnpr/blob/master/tests/testthat/test-1-readBGEN.R#L14-L21).
So I am still not sure what the problem is.

@dewittpe
Copy link

dewittpe commented Feb 2, 2022

@privefl -- I have run into the same again. test-coverage and R-CMD-check failed when I needed to read in a larger .rds file. I'm rethinking my testing objectives and writing out several smaller specific tests instead of the original idea of checking for equality with a large object. I'm at a bit of a loss here too. Some OS/R-version combinations work, some don't, and between the two github actions it appears that there is some inconsistency in behavior reading in the .rds files.

@hechth
Copy link
Author

hechth commented Feb 2, 2022

@dewittpe the tests in the project I mentioned above also involved comparisons of rather large files stored as .rds on disk, so there seems to be something in common.

privefl added a commit to privefl/bigsnpr that referenced this issue Feb 8, 2022
@dusadrian
Copy link

dusadrian commented Aug 28, 2022

I have the same issue, but not using any .rds test files. It fails for normal code, which might help pinpointing the problem.
To be more specific, it is about unexported code that calls a C level function.

This is the repository:
https://github.com/dusadrian/declared

Right now, all of devtools::check(), devtools::test() and covr::package_coverage() pass with no problems, but the offending test is:

library(declared)
library(testthat)
devtools::load_all()
test_that("tagged values work", {
    expect_true(hasTag_(makeTag_("-a"), "-a"))
    expect_true(hasTag_(makeTag_("-ab"), "-ab"))
})
# Test passed 🎉

This passes well at the command line, but as soon as I put this test in tests/testthat/test-internals.R, I get an error:

covr::package_coverage()
# Error: Failure in `/private/var/folders/43/n9gyx1_n6llbb4ds7tflndc40000gn/T/Rtmp6LwsHQ/R_LIBS81d6c63576f/declared/declared-tests/testthat.Rout.fail`
# f/declared/declared-tests/testthat/test-internals.R�test-internals.R:284:5): tagged values work ────────────────────────
# hasTag_(makeTag_("-a"), "-a") is not TRUE
# 
# `actual`:   FALSE
# `expected`: TRUE 
# ── Failure (test-internals.R:285:5): tagged values work ────────────────────────
# hasTag_(makeTag_("-ab"), "-ab") is not TRUE
# 
# `actual`:   FALSE
# `expected`: TRUE 
# 
# [ FAIL 2 | WARN 0 | SKIP 0 | PASS 393 ]
# Error: Test failures
# Execution halted

@D-Se
Copy link

D-Se commented May 10, 2023

For the small ask package I see a similar issue, without .rds files in test files.

  • pre-commit main passes R CMD, tinytest::test_package() and covr::package_coverage(), locally and remotely.
  • post-commit to change parent.frame to R_GetCurrentEnv, on branch parent_frame, tests pass locally and remotely, but package_coverage() fails.

Stepping through package_coverage() in an interactive session, the line that where results == 0L becomes results == 1L is covr:::add_hooks(pkg$package, install_path, fix_mcexit = FALSE), specifically the line setHook(packageEvent(pkg, \"onLoad\"), function(...) covr:::trace_environment(ns)).

Running in the main package directory, this gives 1L:

pkg <- covr:::as_package(".")
install_path <- covr:::temp_file("R_LIBS")
dir.create(install_path)
flags <- getOption("covr.flags")
out_dir <- file.path(install_path, pkg$package)

withr::with_makevars(flags, assignment = "+=", utils::install.packages(
	repos = NULL, 
	lib = install_path, ".", type = "source",
	INSTALL_opts = c("--example",  "--install-tests", "--with-keep.source", "--with-keep.parse.data", 
"--no-staged-install", "--no-multiarch"), quiet = FALSE))

covr:::add_hooks(pkg$package, install_path, fix_mcexit = FALSE)
libs <- covr:::env_path(install_path, .libPaths())

withr::with_libpaths(install_path, action = "prefix", {
	withr::with_envvar(
		c(R_DEFAULT_PACKAGES = "datasets,utils,grDevices,graphics,stats,methods", 
			R_LIBS = libs, R_LIBS_USER = libs, R_LIBS_SITE = libs, 
			R_COVR = "true", R_TESTS = file.path(
				R.home("share"), 
				"R", "tests-startup.R")), {
					result <- tools::testInstalledPackage("ask", types = "tests", lib.loc = install_path)
				})
	
})

but this passes (0L)

pkg <- covr:::as_package(".")
install_path <- covr:::temp_file("R_LIBS")
dir.create(install_path)
flags <- getOption("covr.flags")
out_dir <- file.path(install_path, pkg$package)

withr::with_makevars(flags, assignment = "+=", utils::install.packages(
	repos = NULL, 
	lib = install_path, ".", type = "source",
	INSTALL_opts = c("--example",  "--install-tests", "--with-keep.source", "--with-keep.parse.data", 
"--no-staged-install", "--no-multiarch"), quiet = FALSE))

libs <- covr:::env_path(install_path, .libPaths())

withr::with_libpaths(install_path, action = "prefix", {
	withr::with_envvar(
		c(R_DEFAULT_PACKAGES = "datasets,utils,grDevices,graphics,stats,methods", 
			R_LIBS = libs, R_LIBS_USER = libs, R_LIBS_SITE = libs, 
			R_COVR = "true", R_TESTS = file.path(
				R.home("share"), 
				"R", "tests-startup.R")), {
					result <- tools::testInstalledPackage("ask", types = "tests", lib.loc = install_path)
				})
	
})

I got stuck here, as simply replacing covr:::trace_environment with a local version where every call is annotated with covr::: somehow gives 0L:

trace_environment <- function (env) {
	covr:::clear_counters()
	the$replacements <- covr:::compact(c(covr:::replacements_S4(env), covr:::replacements_RC(env), 
																covr:::replacements_R6(env),
																lapply(ls(env, all.names = TRUE), 
																			 covr:::replacement, env = env)))
	lapply(the$replacements, covr:::replace)
}

Hope this helps to isolate issues

@jimhester
Copy link
Member

@D-Se R_GetCurrentEnv() is not a direct equivalent to parent.frame(). The latter is a safer way to ensure you are getting the correct environment, I would recommend sticking with that approach.

This issue seems to have become a dumping ground for disparate issues, so I am going to close it.

@hechth
Copy link
Author

hechth commented May 12, 2023

Yeah - I think the main reason by now for the difference is (as was already mentioned) that devtools::test(...) and covr run the tests differently and parallelism and some errors are just handled differently in these two ways, with covr being more sensitive from the seems of it.

@b-rodrigues
Copy link

b-rodrigues commented Jan 26, 2024

Sorry to dig up this old issue, but I'm facing the same problem but with a snapshot test. Here's the relevant code:

  testthat::announce_snapshot_file("find_rev/golden_Rprofile.txt")

  testthat::expect_snapshot_file(
              path = save_rix_init_test(),
              name = "golden_Rprofile.txt",
              )

works with devtools::test() and testthat::test_local(), doesn't work with covr::packages_coverage(). What's really surprising is that I have three other snapshot tests, essentially the same as the test above (but it's another function that generates another text file that I'm testing) and for these, no problems. Here's the error message:

══ Failed tests ════════════════════════════════════════════════════════════════
── Failure ('test-find_rev.R:110:3'): Snapshot test of rix_init() ──────────────
Snapshot of `save_rix_init_test()` to 'find_rev/golden_Rprofile' has changed
Run `testthat::snapshot_review('find_rev/')` to review changes

but when I look at the snapshots, or try snapshot_review(), I get No snapshots to update. Any ideas? The test is here: https://github.com/b-rodrigues/rix/blob/ccfe4fff120d785b748ee0377e2f3ee0f2457df7/tests/testthat/test-find_rev.R#L93

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

7 participants