search

r-lib / testthat

An R 📦 to make testing 😀
https://testthat.r-lib.org
Other
888 stars 318 forks source link

Proposal: add a custom signal class for skip(), then sort skip notes by class #1967

Open MichaelChirico opened 3 months ago

MichaelChirico commented 3 months ago

Really liking the recent-ish change to batch skip reasons all in one blob. One further improvement would be better sorting of these skips:

══ Skipped tests (58) ══════════════════════════════════════════════════════════
• fGarch cannot be loaded (1): test-stats.R:26:3
• mapdata cannot be loaded (2): test-maps.R:2:3, test-maps.R:27:3
• MSwM cannot be loaded (1): test-MSwM.R:4:3
• On CRAN (48): test-backcompat.R:2:3, test-base-infer.R:4:3,
  test-base-infer.R:17:3, test-base-infer.R:51:3, test-base.R:4:3,
  test-base.R:12:3, test-base_ts.R:4:3, test-basis.R:6:5,
  test-changepoint.R:4:3, test-changepoint.R:49:3, test-cluster.R:4:3,
  test-cluster.R:27:3, test-cluster.R:81:3, test-cluster.R:127:3,
  test-cluster.R:148:3, test-cluster.R:167:3, test-forecast.R:4:3,
  test-forecast.R:45:3, test-forecast.R:66:3, test-forecast.R:82:3,
  test-forecast.R:98:3, test-forecast.R:117:3, test-forecast.R:133:3,
  test-forecast.R:160:3, test-plotlib.R:4:3, test-plotlib.R:26:3,
  test-plotlib.R:98:3, test-plotlib.R:128:3, test-plotlib.R:150:3,
  test-plotlib.R:215:3, test-plotlib.R:226:3, test-stats-lm.R:7:3,
  test-stats-lm.R:27:3, test-stats-lm.R:40:3, test-stats-lm.R:202:3,
  test-stats-lm.R:363:3, test-stats-lm.R:526:3, test-stats-lm.R:546:3,
  test-stats-lm.R:561:3, test-stats-lm.R:570:3, test-stats.R:222:3,
  test-stats.R:319:3, test-stats.R:420:3, test-stats.R:495:3,
  test-stats.R:522:3, test-stats.R:582:3, test-stats.R:599:3, test-surv.R:238:3
• timeSeries cannot be loaded (6): test-ts.R:4:3, test-ts.R:23:3,
  test-ts.R:44:3, test-ts.R:63:3, test-ts.R:83:3, test-ts.R:128:3

As we see here, the timeSeries skip might be better off grouped with the fGarch, mapdata, and MSwM skips.

One natural way to accomplish this would be for the error signal emitted by skip() to take a custom class, e.g.

c("testthat_error_skip_not_installed", "testthat_error_skip", "error", "condition")
c("testthat_error_skip_on_cran", "testthat_error_skip", "error", "condition")

Then when printing the Skipped tests section, we iterated over skip classes.

Note that approaches building off the text in the skip message will be fraught, e.g. alphabetizing or other pattern matching. The custom signal class approach seems the most generalizable.

hadley commented 2 months ago

How about the ability to add a sort key to the skip object? Then we'd just sort by that, and you'd be free to define as you wish.

MichaelChirico commented 2 months ago

The downside is I'm looking at the output of someone else's suite, so it's not really in my control (unless I send them a PR, nor particularly scalable).