ropensci / vcr

Record HTTP calls and replay them
https://docs.ropensci.org/vcr
Other
77 stars 12 forks source link

CRAN message about broken reverse dependency checks #235

Closed jsta closed 3 years ago

jsta commented 3 years ago

Subject: CRAN packages archiveRetriever dbhydroR deepdep hlidacr intensegRid weathercan Today's vcr update 1.0.0 has broken the checks of these.

All tests pass locally for me (running devtools::test) but fail running as part of devtools::check. The error I get is the same as CRAN:

Error: No root directory found in /home/hornik/tmp/R.check/r-devel-gcc/Work/PKGS/dbhydroR.Rcheck/tests/testthat or its parent directories. Root criterion: contains a file "DESCRIPTION" with contents matching "^Package: "

I was able to pass check associated tests by removing the call to vcr_test_path in the autogenerated "setup" file and hardcoding the path to the fixtures directory.

jsta commented 3 years ago

dir = vcr::vcr_test_path("fixtures") -> dir = "../fixtures"

sckott commented 3 years ago

Thanks. Was going to send an email to people this morning. I did run revdep checks and didn't get any issues (and I don't personally use the function in my own http pkgs), but I should have emailed about this issue, sorry about that. Looking into it

dpprdan commented 3 years ago

👋🏻 My PR #230 probably caused this. Sorry for the trouble this is causing!

I can reproduce with https://github.com/ropensci/opencage/tree/devel. devtools::check()/R CMD check apparently moves the test dir outside the pkg dir:

.
+-- 00check.log
+-- 00install.out
+-- 00_pkg_src
|   \-- opencage
+-- opencage
|   +-- data
|   +-- DESCRIPTION
|   +-- doc
|   +-- extdata
|   +-- help
|   +-- html
|   +-- INDEX
|   +-- Meta
|   +-- NAMESPACE
|   +-- NEWS.md
|   \-- R
+-- opencage-Ex.pdf
+-- opencage-Ex.R
+-- opencage-Ex.Rout
+-- opencage-Ex.timings
\-- tests
    +-- startup.Rs
    +-- testthat
    +-- testthat.R
    +-- testthat.Rout.fail
    \-- vcr_cassettes

(vcr_cassettes is what is usually called fixtures)

I looking into ways how to catch this. Maybe rprojroot::has_dir("tests")?

dpprdan commented 3 years ago

Using rprojroot::has_dir("tests") in vcr_test_path() seems to work in general, i.e. both devtools::test() and devtools::check() run successfully with this change on https://github.com/ropensci/opencage/tree/devel.

However, since has_dir() is starting to look up the tests dir from ./tests/testthat/, it will use ./tests/testthat/tests/ or ./tests/tests/ as a "root" dir should those exist. The tests still run, though.

sckott commented 3 years ago

@dpprdan Thanks for the quick response

@jsta can you try again after installing the change @dpprdan made above remotes::install_github("dpprdan/vcr@2881b588fe3eabac2c7a371f199aaad407b7bddf")

sckott commented 3 years ago

(p.s. @dpprdan ultimately responsibility is with me as the lead maintainer, so no worries)

dpprdan commented 3 years ago

Thanks! Still sucks though to have this go to CRAN and causing revdeps to fail. Apparently I didn't run R CMD check on opencage since #230 got merged (or I didn't install that change locally?) or else I would have caught it. Strange.

jsta commented 3 years ago

Works with that fork!