nealrichardson / httptest

A Test Environment for HTTP Requests in R

https://enpiar.com/r/httptest/

Other

80 stars 10 forks source link

Updates for testthat 3rd edition #38

Closed jonkeane closed 4 years ago

jonkeane commented 4 years ago

This adjusts all of the tests to pass the 3rd edition cleanly and without excess warnings/messages. This also (fully) resolves #34

I have punted on two tests involving expect_failure() within public(): these don't seem to trigger a failure like expected (though there are other examples where those two work just fine). We'll take this up in a followup.

codecov-io commented 4 years ago

Codecov Report

Merging #38 into master will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #38   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          435       435           
=========================================
  Hits           435       435

Impacted Files	Coverage Δ
R/expect-header.R	`100.00% <100.00%> (ø)`
R/trace.R	`100.00% <100.00%> (ø)`
R/mock-api.R	`100.00% <0.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 e63111f...822d14b. Read the comment docs.

dmenne commented 4 years ago

Time to merge? Thanks....

nealrichardson commented 4 years ago

Yeah I think this is about ready. @jonkeane would you mind adding to NEWS about this, noting in particular the code changes (expect_header etc.) that make the package work better on 3e?

jonkeane commented 4 years ago

Done!

maelle commented 4 years ago

I have another unsollicited comment, not related to this PR in particular: it seems several tests fail with devtools::test(). Is it a known problem? Would there be a way around this?

nealrichardson commented 4 years ago

@maelle I'm not aware of a general problem; #15 mentions devtools::test() but it's a bit murky as to what is going on. Could you please open an issue with details about that?

maelle commented 4 years ago

So when you run devtools::test() locally on httptest all tests pass? I'll open an issue tomorrow or on Thursday

nealrichardson commented 4 years ago

I don't personally do devtools::test() to run the tests, I always run in a fresh R process

nealrichardson commented 4 years ago

Ok, I squashed/rebased this on master after merging @maelle's latest PR, which conflicted with this. I think it's good now and will merge after CI passes, though I'll take one last pass through to make sure I didn't screw up the git shenanigans.