Record json-encoded bodies

[aaronwolen]

Apologies for the barrage PRs, I'm working on adding mocked-tests to osfr 🙂.

This is related to the issue addressed in webmockr PR#82, which prevented json-encoded bodies from being recorded to cassettes.

I added a new test file that basically mirrors tests/testthat/test-ause_cassette_match_requests_on.R albeit with JSON encoded requests. This might be overkill.

It'd be nice to have an exported version of pluck_body() somewhere that could be used in both packages but I'm not sure where that should go...

Let me know if you have any questions/suggestions.

[sckott]

thanks - and glad to see more usage! First to address the pluck_body issue:

I think it makes sense to export pluck_body - since webmockr is in Imports in vcr and vcr is only in Suggests in webmockr I think we move the function to webmockr, export it from there, then vcr can use it from webmockr - sound good?

[aaronwolen]

Totally agree. Will update the PRs accordingly.

[sckott]

thanks

[aaronwolen]

Still working on this...

Currently the following test fails

out4 <- use_cassette("httr_post_upload_file", {
  b <- POST("https://httpbin.org/post",
    body = list(y = httr::upload_file(system.file("CITATION"))))
})
...
expect_match(strj$data, "bibentry\\(")

because webmockr::pluck_body() string-encodes form_file objects.

[sckott]

I think i got webmockr and vcr working locally - for vcr:

add if (inherits(x, "form_file")) return(FALSE) as the first line in this function https://github.com/ropensci/vcr/blob/master/R/request_class.R#L171-L175

and in webmockr in RequestSignature:

in the print method, replace cat_foo(self$body) with

if (inherits(self$body, "form_file"))
    cat(paste0("     ",
        sprintf("type=%s; path=%s", self$body$type, self$body$path)),
        sep = "\n")
else
    cat_foo(self$body)

and replace the to_string method at the bottom of the RequestSignature file with:

to_string <- function(x) {
  if (inherits(x, "list") && all(nchar(names(x)) > 0)) {
    tmp <- paste0(paste(names(x), x, sep = ": "), collapse = ", ")
  } else if (inherits(x, "list") && any(nchar(names(x)) == 0)) {
    tmp <- paste0(paste(names(x), x, sep = ": "), collapse = ", ")
  } else if (inherits(x, "form_file")) {
    tmp <- sprintf("type=%s; path=%s", x$type, x$path)
  } else {
    tmp <- paste0(x, collapse = ", ")
  }
  return(sprintf("{%s}", tmp))
}

---

With those changes, everything works locally for me, thoughts?

[aaronwolen]

I'll check it out!

Should I apply these patches to the respective master branches or on top of the current PRs?

[sckott]

In the current prs

On Thu, Dec 19, 2019 at 08:14 Aaron Wolen notifications@github.com wrote:

I'll check it out!

Should I apply these patches to the respective master branches or on top of the current PRs?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ropensci/vcr/pull/130?email_source=notifications&email_token=AAENBBACAIKWGEJ64JIEIDDQZOMWJA5CNFSM4J2WPX22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHKDP3Q#issuecomment-567556078, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAENBBDHAPL5ZM2JI2XEKWDQZOMWJANCNFSM4J2WPX2Q .

[aaronwolen]

Looks like that did the trick! 🎉

Of course we're being robbed of the satisfaction of passing tests because the minimum required version of webmockr is now 0.5.1.92 but I verified everything checks out in a fresh docker container.

[sckott]

thanks for all your work on this!