testthat icon indicating copy to clipboard operation
testthat copied to clipboard

Consistency of info= argument

Open MichaelChirico opened this issue 3 years ago • 4 comments

Should all expect_* have an info= argument?

We have a linter for expect_equal(length(x), n) to recommend expect_length(x, n) instead, but it can't apply in cases where info= is passed to expect_equal() because expect_length() doesn't have the corresponding argument.

Should it? Here's a quick pass at which expect* functions in testthat don't have an info= argument:

 expect_length
 expect_setequal
 expect_silent
 expect_type
 expect_gt
 expect_less_than
 expect_invisible
 expect_more_than
 expect_snapshot_error
 expect_failure
 expect_equal_to_reference
 expect_snapshot_file
 expect_vector
 expect_visible
 expect_lte
 expect_success
 expect_lt
 expect_s3_class
 expectation
 expect_snapshot
 expect_snapshot_value
 expect_cpp_tests_pass
 expect_known_hash
 expect_snapshot_output
 expect_s4_class
 expect_mapequal
 expect_gte

It would be nice to avoid the maintenance overhead of writing exceptions for which expect* linters need to be aware of whether info= is used/available: https://github.com/r-lib/lintr/issues/952

MichaelChirico avatar Mar 21 '22 23:03 MichaelChirico

This also applies to the label= argument

MichaelChirico avatar Mar 21 '22 23:03 MichaelChirico

I think our position is that info is a remnant of an older time, and It's not something we support on newer expectations.

hadley avatar Apr 25 '22 13:04 hadley

OK... any plans to deprecate it in those expectations that currently have it?

MichaelChirico avatar Apr 28 '22 01:04 MichaelChirico

In principle I think we should deprecate it (at least in the 3rd edition, although even that ship might have sailed), but it's unlikely that I'll find the time to do it since it's a relatively small payoff.

hadley avatar Apr 28 '22 12:04 hadley