Start using testthat 3e

[MichaelChirico]

No description provided.

[MichaelChirico]

We have a bunch of migrations to do here..

[MichaelChirico]

Finished the first pass here. Notes:

• Couldn't finish all the with_mock() migrations. I don't understand how to use mockery() for some of them. I raised a comment on a similar mockery issue here: Stubbing out a function called within S3 dispatch mockery#43. Any ideas?
• It looks like the code tested test-cache has been refactored -- the with_mock() appear to be no-ops, so I just removed them. Let's see.
• Also in test-cache, expect_equal(e1, e2) became the much uglier expect_identical(as.list(e1), as.list(e2)). Raised Documentation request: how to compare environments? waldo#127 about it.
• Also did some more general clean-ups of tests, e.g. preferring withr::local_* functions over withr::with_* and using withr more comprehensively
• The tests in test-get_source_expressions.R are in incomprehensible mess IMO (and I've only made it worse with the expect_identical() shift) -- we should file a follow-up to refactor it to a more maintainable approach

[AshesITR]

Looks fine so far

[hadley]

If you use this approach, I think you need all.names = TRUE to verify the property implied by the test title.

OTOH I'm not sure what "non-hidden" means as I would have expected hidden to imply a variable starting with ., but that doesn't appear in the test.

Additionally, I'd use expect_equal() here because expect_identical() just implies exact numerical comparisons, which I don't think is important here.

[MichaelChirico]

that's not my reading of the docs (?expect_identical):

expect_identical() is the baseline; expect_equal() relaxes the test to ignore small numeric differences.

i.e., only use expect_equal to ignore numerical differences

[hadley]

Hmmm. I guess it missing another sentence that says something like: "Since in most cases tiny numeric differences aren't important, we recommend using expect_equal()."

[MichaelChirico]

🙈 our internal policy encourages expect_identical() now and we've basically switched all the expect_equal() tests already, except where ignore*= or tolerance= are needed. I have come to quite like it TBH. It encourages stronger tests.

[MichaelChirico]

Hmm @AshesITR the new helper function uses rlang as suggested in the testthat vignette: https://testthat.r-lib.org/articles/custom-expectation.html

That means we need a Suggests for rlang, but testthat already Imports rlang, so our total dep tree doesn't really change. WDYT?

[AshesITR]

Nice work, thanks!