add with_mock_dir

[maelle]

Fix #40

[maelle]

is there any write-up of how httptest tests work? asking because of my wondering how I'd add one.

[nealrichardson]

Some notes. For testing, one thought is that you could break this into two cases: one where the directory exists, so we use mocks, and one where the directory does not, so we record. For the former, if you didn't want to cheat and do with_mock_dir(testthat::test_path("."), ...) you could create a temp dir, copy testthat::test_path("httpbin.org") into that temp dir, and test that you read from that (and that outside of the context, the mockPath is reset).

Testing the recording side is a little trickier if you want to mock what you're recording (🐢 s all the way down) but it's doable. https://github.com/nealrichardson/httptest/blob/master/tests/testthat/test-capture-requests.R uses this capture_while_mocking helper (https://github.com/nealrichardson/httptest/blob/master/tests/testthat/setup.R#L10), which could be refactored to be used here too. Or you could just have one request, assert that the mock is recorded to the right place, and have it skip_if_disconnected().

Keeping those two cases distinct means you can test the use of mocks even if you choose to make the recording test only work with a live connection.

[nealrichardson]

I would delete the other hacky answers, they're no longer recommended if this function exists IMO

[maelle]

the hacky answers are for a different use case though? When you want to turn off using httptest entirely? à la VCR_OFF (an environment variable in vcr).

[maelle]

which I would actually like to recommend in the book, run tests against the real service with a scheduled CI run.

[codecov-io]

Codecov Report

Merging #48 (f2fa8cd) into master (541a48c) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #48   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        18    +1     
  Lines          447       451    +4     
=========================================
+ Hits           447       451    +4     

Impacted Files	Coverage Δ
R/mock-dir.R	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 541a48c...f2fa8cd. Read the comment docs.

[maelle]

(thanks for the notes on testing!)

[maelle]

Not the most elegant thing ever... but otherwise things get messy when testing this function. I had no better idea.

[maelle]

I think this is still an important question.

[nealrichardson]

I'll take a look, and if I can come up with anything better for testing, I'll push a commit to your branch.

[nealrichardson]

I simplified the code a bit and likely broke the interactive use case, but I think that needs a more general solution (#50). I'll work on that next.

The other thing to do here is make a pass over the docs and make sure with_mock_dir is discussed everywhere it should be.

[nealrichardson]

I did wonder whether there was a use case where replace=FALSE would make sense (i.e. whether replace should go in the signature of with_mock_dir).

[maelle]

ah! Good question, but better to add it to the signature then.

[maelle]

Thanks for the review & improvements!

[maelle]

Should you add a sentence or two about the other ways to get some sort of automatic recording? Or link to an FAQ? I can't remember where you documented the use of a environment variable in setup.R to replace with_mock_api() with capture_requests().