Package redactors should load when testing interactively with devtools::test()

[MarkEdmondson1234]

Hello!

I've tried this:

You can also provide this function in inst/httptest/request.R, and any time your package is loaded (as when you run tests or build vignettes), this function will be called automatically.

With this code:

set_requester(function (request) {
  gsub_request(request, "https\\://(.+).googleapis.com/", "api/")
})

...but the mocks are still written to the long form folders, without substitution. Am I missing something?

Its for this repo https://github.com/ropensci/googleLanguageR/blob/master/inst/httptest/request.R and the mocks are here https://github.com/ropensci/googleLanguageR/tree/master/tests

[nealrichardson]

Hi Mark, A couple of ideas:

1. Most likely the issue is that you need to set a redactor to edit what is written in the mocks. That's what gets called in capture_requests(). The requester is used in with_mock_api() to alter the request before mapping it to a file. So if you're truncating URLs when recording mocks, it sounds like you want function (response) gsub_response(response, "https\\://(.+).googleapis.com/", "api/") in inst/httptest/redact.R and then this gsub_request() in your requester. See https://github.com/nealrichardson/pivotaltrackR/tree/master/inst/httptest for an example.

(Aside: I admit that the distinction is a little confusing, and it feels like you should be able to set the function once and have it work for recording and replaying. I had at least once case where I didn't want to assume the same logic when loading mocks than when recording them, or there were things that only made sense when recording, so I made them be distinct. But maybe that should be the special case and the default should be that one function works for both. I'll think about it. Would love to hear more about your experience once you get beyond this blocker so that I can make it better.)

2. httptest finds these package-level functions via system.file(), so they work when the package is installed, but I'm not sure about when they're loaded via devtools::load_all(). Maybe devtools has a shim that makes system.file() work as if it were truly installed, IDK, I haven't really looked into it. But if you still have trouble once you put the code in the right file, try installing the package.

3. Inside inst/httptest/request.R or redact.R, you don't need to call set_requester() or load library(httptest), you can just have the function. set_requester() and set_redactor() do both return the function, so it would still work if you did call it, but it's not necessary.

[MarkEdmondson1234]

Thanks Neal, I followed the suggestions above and it works!

...but only if I source the test files manually, and install from GitHub and not locally.

remotes::install_github("ropensci/googleLanguageR")
setwd("tests/testthat")
source("test_unit.R")

When I test via RStudio's test button e.g. devtools::test() it doesn't read or write to the expected directory. I tried testing via installing via remotes::install_package("ropensci/googleLanguageR") and locally via R CMD INSTALL via RStudio's Install and Restart, but I think its looking in the wrong version of the package related to your point 2).

It seems to work otherwise, on Travis and when I do a devtools::check() after I manually created the mock files. So basically it works if I don't use devtools::test()

[nealrichardson]

Ok, good to know. I can look into what devtools::test() does exactly and see if I can make it work in the interactive case. I'll rename this issue to focus on that.

As you note, it does work as expected in R CMD check (and hence Travis) because the package is "properly" installed--I even have a test that it finds the redactor from an installed package.

[MarkEdmondson1234]

Now for some reason the Travis tests are failing https://travis-ci.org/ropensci/googleLanguageR/builds/417955023

...but the local (via devtools::check()) do work. I'm confused why they should differ since its the same code in both cases.

sessionInfo()
R version 3.5.0 (2018-04-23)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS High Sierra 10.13.6

Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.5/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] compiler_3.5.0     magrittr_1.5       R6_2.2.2           assertthat_0.2.0   parallel_3.5.0    
 [6] tools_3.5.0        yaml_2.1.19        testthat_2.0.0     rlang_0.2.2        stringdist_0.9.5.1

[nealrichardson]

Looks like a github problem to me: The command "eval git fetch origin +refs/pull/43/merge: " failed 3 times.

[MarkEdmondson1234]

Just a note I spent 30mins puzzling why this didn't work again until I read this thread, so may be worth adding a note to the website in the FAQ

[MarkEdmondson1234]

And still confusingly it still errors on Travis - I can do the steps above to create the mocks, but then they fail. Any hints appreciated - The travis build log and github commit

[nealrichardson]

Thanks for the nudge on this issue, I'll take another look. I'd rather add a solution than a note to the FAQ, which wouldn't prevent you or someone else from losing time on the issue.

Re: the travis build, the test failures all say "Invalid token", not an httptest error from not finding a mock. Maybe that is trapped somewhere in your code and returned as Invalid token? Or maybe it's something else.

[nealrichardson]

Alright, give that a try and see if it is resolved. I believe I reproduced the behavior faithfully in the test, but it's possible that there's some other corner I missed.

[MarkEdmondson1234]

I'm afraid it has not, with this commit I have the redact.R and request.R with this content:

function(response){
  require(magrittr, quietly=TRUE)
  response %>%
    httptest::gsub_response(".googleapis.com", "", fixed=TRUE)
}

function(request){
   require(magrittr, quietly=TRUE)
   request %>%
       httptest::gsub_response(".googleapis.com", "", fixed=TRUE)
}

I expect these redactions to turn language.googleapis.com into language etc.

• Set my INTEGRATION_TESTS <- TRUE flag in my "prep_tests.R" file that sets this line: with_mock_api <- httptest::capture_requests
• Make sure my authentication is on via GL_AUTH environment file.
• I "Install and Restart" in RStudio
• Click the "more > test package" button in RStudio
• wait for tests to run

The expected folders are not created - they stay the same as before with language.googleapis.com etc.

Then I think I have another issue, where the authentication token is not recognised after the first run - will investigate that a bit and open another issue if necessary.

[nealrichardson]

Ok. I'll have to look into what "Install and Restart" does then. In the meantime, instead of doing that step, could you try just pkgload::load_all()?

[ramiromagno]

hi @nealrichardson:

I am also having the same trouble. The mocks files are written to the long form folders, without substitution... I might be doing something wrong too as I have just started attempting at shortening those long URLs because of the "non-portable file paths" during R CMD check. Still, if you could have a look at my code it would be great!

https://github.com/ramiromagno/gwasrapidd/tree/master/inst/httptest

P.S.: @MarkEdmondson1234 's last post includes the code for both redact.R and request.R but both use the function gsub_response. Should not he be using gsub_request in request.R instead?

Many thanks!

[ramiromagno]

Now I am seeing the same pattern as @MarkEdmondson1234. Travis builds fail, devtools::check() also fails but running devtools::test() works without any fail...

[ramiromagno]

Well, closing and opening RStudio now made the writing of the mock files work...

Using redact.R from “gwasrapidd”

Attaching package: ‘magrittr’

The following objects are masked from ‘package:testthat’:

    equals, is_less_than, not

Writing /home/rmagno/sci/code/R/pkg/gwasrapidd/tests/testthat/gc/metadata.json

So the current status is:

1. devtools::test() runs my tests using the old long form folders, and that works locally/interactively because I still have those folders (I have not yet deleted them); so it seems devtools::test() is ignoring httptest/request.R
2. devtools::check() gives a bunch of errors because seemingly it is trying to use the new shorted folders but because I have only one yet, i.e, the tests/testthat/gc/metadata.json, so most tests fail.

So for me the question is really why isn't devtools::test() using httptest/request.R ?

BTW: devtools is version 2.0.1 and httptest 3.2.0.

[nealrichardson]

Hi @ramiromagno, For (1), could you try with the github version of httptest? devtools::test() ignoring the request preprocessor sounds like it could be the pkgload::load_all() issue I fixed most recently.

For (2), I looked for a failing Travis build of yours and saw https://travis-ci.org/ramiromagno/gwasrapidd/jobs/519796371, and the failure makes sense because as you note, the request preprocessor is running but you don't have the mocks in the new location (according to what I see in the corresponding commit, https://github.com/ramiromagno/gwasrapidd/tree/2f7f79dc314d3a1559d8096792f0a155178af689/tests/testthat).

Interesting observation about Mark's request.R using gsub_response() instead of gsub_request(). That could affect whether request body and query parameters are properly handled, but both apply to $url, which is where the path shortening will happen. (See https://github.com/nealrichardson/httptest/blob/master/R/redact.R#L88-L112) So it could cause problems, but probably not these problems.

[ramiromagno]

Hi @nealrichardson,

Note: The inconsistent behavior :point_down: is gone with latest version of devtools!

After installing the github version of httptest I am getting a very interesting behavior... Depending on where I call devtools::test() it uses the request preprocessor or not, and not consistently. Sometimes, hitting Shift+Ctrl+d that runs devtools::test() "at the top right" pane I get tests passing, meaning using the shortened mock paths, while running devtools::test() directly from the console won't work, i.e. it is still trying to test using the long paths... Some other times is the other way around! From the console it works but runnning devtools::test() via the shortcut does not...

Here's a screenshot illustrating the first case: devtools::test() ran from the console does not use the shortened paths but seemingly devtools::test() ran via the Shift+Ctrl+d does work... I am very confused.

[ramiromagno]

BTW: How do I do the substitutions for multiple patterns? I tried this but it's not working:

inst/httptest/redact.R:

httptest::set_redactor(
  list(
    function (response) { httptest::gsub_response(response, "https\\://www.ebi.ac.uk/gwas/rest/api/", "gc/") },
    function (response) { httptest::gsub_response(response, "https:\\://www.ebi.ac.uk/efo/", "efo/") },
    function (response) { httptest::gsub_response(response, "https:\\://www.ebi.ac.uk/ols/api/ontologies/efo", "ols/") }
  )
)

inst/httptest/request.R:

httptest::set_requester(
  list(
    function (request) { httptest::gsub_request(request, "https\\://www.ebi.ac.uk/gwas/rest/api/", "gc/") },
    function (request) { httptest::gsub_request(request, "https:\\://www.ebi.ac.uk/efo/", "efo/") },
    function (request) { httptest::gsub_request(request, "https:\\://www.ebi.ac.uk/ols/api/ontologies/efo", "ols/") }
  )
)

[nealrichardson]

set_requester/set_redactor take a single function, so more like:

set_redactor(function (response) {
    response %>%
        gsub_response("https\\://www.ebi.ac.uk/gwas/rest/api/", "gc/") %>%
        gsub_response("https:\\://www.ebi.ac.uk/efo/", "efo/") %>%
        gsub_response("https:\\://www.ebi.ac.uk/ols/api/ontologies/efo", "ols/")
})

[ramiromagno]

@nealrichardson: Thanks!

And if I have two patterns where one is more specific than the other...? which one should come first in the chain?

Specific pattern earlier

set_redactor(function (response) {
    response %>%
        gsub_response("https\\://www.ebi.ac.uk/gwas/rest/api/singleNucleotidePolymorphisms/", "snps/") %>%
        gsub_response("https\\://www.ebi.ac.uk/gwas/rest/api/", "gc/")
})

Specific pattern later

set_redactor(function (response) {
    response %>%
        gsub_response("https\\://www.ebi.ac.uk/gwas/rest/api/", "gc/") %>%
        gsub_response("https\\://www.ebi.ac.uk/gwas/rest/api/singleNucleotidePolymorphisms/", "snps/")
})

[nealrichardson]

They evaluate in order, just as if you were calling gsub() on a string, so in your "Specific pattern later" case, I wouldn't expect the second regexp to match anything because the first one will have replaced all "https://www.ebi.ac.uk/gwas/rest/api/" with "gc/"

[ramiromagno]

@nealrichardson Thank you for all the help! I've got everything working now :)