Should .mockPaths() return different paths from tests/testthat and from the root of the folder

[maelle]

Context (hehe) for my issue: when debugging a test you might find yourself, I think, running use_mockapi() in your R session. But this won't use your existing mock files as they are in tests/testthat.

Therefore, should .mockPaths() work like testthat::test_path()?

[nealrichardson]

To put it in concrete terms, I think you're suggesting this, right?

diff --git a/R/mock-paths.R b/R/mock-paths.R
index a3f4f35..472082d 100644
--- a/R/mock-paths.R
+++ b/R/mock-paths.R
@@ -26,7 +26,7 @@
 #' @rdname mockPaths
 #' @export
 .mockPaths <- function (new) {
-    current <- getOption("httptest.mock.paths", default=".")
+    current <- getOption("httptest.mock.paths", default=testthat::test_path())
     if (missing(new)) {
         ## We're calling the function to get the list of paths
         return(current)
@@ -36,7 +36,7 @@
         invisible(current)
     } else {
         ## We're adding one or more paths
-        current <- unique(c(new, current))
+        current <- unique(c(testthat::test_path(new), current))
         options(httptest.mock.paths=current)
         invisible(current)
     }

[maelle]

Yes I think so, it'd work in both modes (tests, interactive test debugging)

[nealrichardson]

My only concern is that the vignette tooling (start_vignette(), change_state()) use .mockPaths() too, so they may need to be adapted--they take the vignettes dir as the current working directory, so test_path wouldn't work. (The test suite passes with this change, but as it turns out, the vignette tests also call .mockPaths() in the assertions, so they aren't actually testing that the vignette would work IRL.)

[maelle]

Ouch indeed more complex than I thought. Could .mockPaths detect it is run from knitr? Which wouldn't help if you're debugging vignette code outside of the vignette...

[nealrichardson]

Could .mockPaths detect it is run from knitr?

Sounds hacky; it would be less hacky for start_vignette and change_state just to use different code to set the paths. I'll think about this some more, see if there's a less bad way. Maybe .mockPaths() can check if the (relative) path exists and try test_path() if it does not?

[maelle]

But "." would be a special case?

[jonkeane]

The way that I “solved” this in dittodb was hacky, but has so far worked well enough: I set as default both “.” and “tests/testthat/“.

[nealrichardson]

Ah interesting, that could work well for loading mocks since if the file is not found in the first path it tries the next, but what about recording?

[jonkeane]

For recording, it defaults to the first path (so ”.”), which can be confusing (though in practice when I’m recording mocks during package development, I like that the recordings plop into the root directory so that I can look at them/move them over to tests/testthat).