vcr_test_path() fix: look for 'tests' dir instead of DESCRIPTION

[dpprdan]

• DESCRIPTION is not available for tests run by R CMD check, #235

Both devtools::test() and devtools::check() seem to run successfully with this change.

Caveat: If vcr_test_path() is run from a file setup-pkg.R in ./tests/testthat/ (i.e. the recommended setup, AFAIU), has_dir() is starting to search for the tests dir from ./tests/testthat/ and go further up the dir tree. As a result, it will use ./tests/testthat/tests/ or ./tests/tests/ as a "root" dir should those exist and the test path will start from there.

[sckott]

Thanks!

I'll ping some of the authors of the other affected pkgs and see if any of them can test this out.

[dpprdan]

@sckott Is this sufficient, given aforementioned caveat, or should we consider alternatives?

Possible alternatives:

1. rprojroot::is_testthat ("Root criterion: directory name is "testthat" (also look in subdirectories: tests/testthat, testthat)"): Similar caveat as rprojroot::has_dir("tests"), works only with {testthat}. (Doesn't find the package root, but testthat dir within ./tests)
2. rprojroot::has_file("testthat.R") ("Root criterion: contains a file "testthat.R"): similar to 1, probably lower risk to match on a dir further down the path (assuming that users might use multiple dirs called tests more often than using multiple testthat.R files).
3. Go back to using relative paths with file.path(), possibly normalizePath()ing them. We'd have to find a better solution for #225, then.

Looking at the function description, has_dir("tests") probably comes closes to what vcr_test_path() is intended to do:

This function, similar to testthat::test_path(), is designed to work both interactively and during tests, locating files in the tests/ directory.

So I could just add a warning to the docs, 'This might not work if you have more than one directory named "tests" in your package'?

Somewhat relatedly: Should vcr be compatible with test suites other than testthat, e.g. tinytest? IIUC tinytest's test files are located in /inst/tinytest/, so the cassettes probably should be, too?

[sckott]

I guess considering we want vcr to be useable with other testing pkgs, we probably don't want 1 or 2, and I don't think we want 3 given the #225 issue

The solution in this PR works for me in all scenarios I've tested.

As a result, it will use ./tests/testthat/tests/ or ./tests/tests/ as a "root" dir

I don't see this when I tried it locally. I went from the root of a pkg and ran the two lines https://github.com/ropensci/vcr/pull/236/files#diff-3f0a5d7d447e7c5c839094838571d9e1f38b5ecfa19e057193ba32c5959c6851R19-R20, then went into tests/ and ran them again, and also within tests/testthat - and those two lines all gave back the correct path (e.g., when run for my zbank pkg within tests/testthat/ I get "/Users/sckott/github/ropensci/zbank/tests/fixtures")

So I could just add a warning to the docs, 'This might not work if you have more than one directory named "tests" in your package'?

Seems unlikely, right? So we probably don't have to worry about that situation very much

---

We do have an issue already for tinytest/testit https://github.com/ropensci/vcr/issues/162 - I don't think we want to address this now - it should be thoroughly tested, and we don't want to rush it i think

[dpprdan]

I don't see this when I tried it locally.

Sorry, I did not describe this clearly.

it will use ./tests/testthat/tests/ or ./tests/tests/ as a "root" dir should those exist

Emphasis added now. So if there is a second tests dir within the tests dir (the one in the package root) it will use the one further down the dir tree (though depending on the current working directory, I suppose). E.g. if I have a tests dir within /tests/testthat and I run devtools::test() with the vcr setup in `/tests/testthat/setup-mypackage.R´ I'll end up with

.
+-- [...]
+-- tests
|   +-- testthat
|   |   +-- setup-mypackage.R
|   |   +-- [...]
|   |   +-- tests
|   |   |   \-- fixtures
|   |   |       +-- some_vcr_cassette.yml
|   |   |       +-- [...]
|   \-- testthat.R
\-- vignettes
    +-- [...]

instead of

.
+-- [...]
+-- tests
|   +-- testthat
|   |   +-- setup-opencage.R
|   |   +-- [...]
|   |   \-- tests
|   |-- testthat.R
|   \-- fixtures
|       +-- some_vcr_cassette.yml
|       +-- [...]
\-- vignettes
    +-- [...]

Then again, how likely is it that users will put tests within tests?

it should be thoroughly tested, and we don't want to rush it i think

Definitely! My idea behind the tinytest question was that with tinytest the cassettes shouldn't live in /tests but in /inst/tests (because that's where the tinytest test files are AFAIK). So the has_dir("tests") solution does not work in that case. But we can think about that, when we tackle tinytest.

[sckott]

@dpprdan Thanks for the explanation. I'll see if I can have a quick look to answering this tests within tests question

[sckott]

library(crul)
library(glue)

con <- HttpClient$new("https://api.github.com",
  headers = list(Authorization = paste0("token ", Sys.getenv("GITHUB_PAT"))))

# vcr reverse dependencies
revdeps <- c("bold", "chirps", "circle", "citecorp", "crminer", "dbhydroR", "deepdep", "finbif", "fulltext", "gfonts", "hlidacr", "intensegRid", "jaod", "magmaR", "microdemic", "natserv", "pangaear", "qualtRics", "rbace", "rbhl", "rbison", "rcol", "rcoreoa", "rcrossref", "rdatacite", "rdryad", "rerddap", "rgbif", "ritis", "rnoaa", "rnpn", "rorcid", "rphylopic", "rplos", "rredlist", "rromeo", "rstac", "rvertnet", "Rwtss", "spocc", "taxize", "weathercan", "webmockr", "wikitaxa", "worrms", "zbank")

# get latest tree sha
tree_sha <- function(pkg, org = "cran", branch = "master") {
  z <- con$get(glue("repos/{org}/{pkg}/branches/{branch}"))
  res <- jsonlite::fromJSON(z$parse("UTF-8"))
  res$commit$commit$tree$sha
}

# get tree
tree <- function(sha, repo, org = "cran") {  
  path <- glue("repos/{org}/{repo}/git/trees/{sha}")
  w <- con$get(path, query = list(recursive = "true"))
  temp <- jsonlite::fromJSON(w$parse("UTF-8"))
  temp$tree$path
}

# any test(s) dir within tests?
any_nested_tests <- function(paths) {  
  paths_in_test <- grep("^tests/", paths, value = TRUE)
  any(grepl("^tests/testthat/tests?/", paths_in_test))
}

# run all pkgs
res <- list()
for (i in seq_along(revdeps)) {
  ts <- tree_sha(revdeps[i])
  tr <- tree(ts, revdeps[i])
  res[[ revdeps[i] ]] <- any_nested_tests(ts)
}
any(unlist(res))
#> FALSE

And I tested it with a repo I created https://github.com/sckott/testintest to make sure a path tests/testthat/tests/ is indeed found

So at least none of the current revdeps have tests/testthat/tests/ - could run over all of cran pkgs to see if any pkgs do this

[sckott]

@dpprdan I think we just need to add to the vcr_test_path docs about the tests within tests thing and then we're good to go

[dpprdan]

@sckott Sorry I did not follow up! I was busy with other stuff and overlooked this. Feel free to ping me next time! Great that this is solved for now.

[sckott]

Thanks for helping get this fixed