Skip to content

Convert tests from testthat to testit#625

Merged
keaven merged 33 commits into
mainfrom
convert-tests-to-testit
May 19, 2026
Merged

Convert tests from testthat to testit#625
keaven merged 33 commits into
mainfrom
convert-tests-to-testit

Conversation

@yihui
Copy link
Copy Markdown
Collaborator

@yihui yihui commented May 6, 2026

Summary

  • Replace all testthat tests with testit equivalents (60 R test files + 2 markdown snapshot files)
  • Snapshot tests (as_gt, as_rtf) converted to testit's markdown-based .md format with output embedded inline
  • DESCRIPTION updated: testthat (>= 3.0.0) replaced with testit
  • Key conversions: test_that()assert(), expect_equal()all.equal(), expect_identical()%==%, expect_error()has_error(), expect_true()(expr)

Motivation

> # testthat dependencies
> setdiff(unlist(tools::package_dependencies('testthat', recursive = TRUE)), xfun::base_pkgs())
 [1] "brio"      "callr"     "cli"       "desc"      "evaluate"  "jsonlite"  "lifecycle"
 [8] "magrittr"  "pkgload"   "praise"    "processx"  "ps"        "R6"        "rlang"    
[15] "waldo"     "withr"     "fs"        "glue"      "pkgbuild"  "rprojroot" "diffobj"  
[22] "crayon"   

> # testit dependencies (none)
> setdiff(unlist(tools::package_dependencies('testit', recursive = TRUE)), xfun::base_pkgs())
character(0)

More info: https://yihui.org/en/2026/05/testthat-to-testit/

yihui and others added 13 commits May 6, 2026 13:42
Replace the testthat test infrastructure with testit:
- test_that() -> assert()
- expect_equal() -> isTRUE(all.equal())
- expect_identical() -> %==%
- expect_true/false() -> (expr) / (!(expr))
- expect_error/warning() -> has_error/has_warning()
- Snapshot tests (as_gt, as_rtf) converted to testit's .md format
- Test runner: tests/test-all.R using test_pkg()
- DESCRIPTION: testthat dependency replaced with testit

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Upstream added as.data.frame() wrapping in test comparisons;
resolved by applying the same change in testit syntax.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use the new `message` parameter in has_error() to verify
specific error messages, matching the original testthat tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Since testit's assert() checks whether the value is TRUE,
all.equal() already returns TRUE on success, making isTRUE()
redundant. Replace (isTRUE(all.equal(a, b))) with (all.equal(a, b)).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion 2

testthat edition 2's expect_equal() passes if EITHER the relative OR
absolute difference is within tolerance. Add an all_equal() helper
that replicates this behavior, and use it across all test files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@yihui yihui requested review from LittleBeannie and jdblischak May 6, 2026 22:31
@yihui
Copy link
Copy Markdown
Collaborator Author

yihui commented May 7, 2026

@jdblischak @LittleBeannie This PR is ready for review.

Comment thread tests/testit/helper.R Outdated
Comment thread tests/testit/test-developer-ahr.R Outdated
For tests comparing exact values (integers, rounded data frames, NULL,
lists), use testit's %==% operator instead of all_equal(). This gives
better failure diagnostics via str() output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that the test coverage reported locally by covr::package_coverage() before and after is identical (93.12%) 🎉

A few ergonomic questions:

  • Why does {testit} need to be loaded in order to run test_pkg()? I couldn't find this in the docs
packageVersion("testit")
## [1] ‘0.18.5’
testit::test_pkg()
## Error from sys.source2(r, envir = env, top.env = ns)
## Error in assert("unstratified population, compared with old version",  :
##   could not find function "assert"
  • Is it possible to suppress the many error messages printed to the R console? I assume these are from tests that use has_error()
library("testit")
test_pkg()
# Lots of error messages printed to the console, eg
## Error: missing value where TRUE/FALSE needed
## Error: missing value where TRUE/FALSE needed
## Error: `times` (c(1, 2, 1)) must be positive and strictly increasing!
## Error: `survival` (c("0.5", "NA")) must be positive!
## Error: `survival` must be of same length as `times`
## Error: `survival` (c(0.5, -0.1)) must be positive!
## Error: `survival` must be non-increasing
## Error: `survival` must be non-increasing

# But they all passed
.Last.value
## NULL
  • I observed that {testit} stops on the first error. Is there any plan to enable {testit} to collect test errors to spot patterns, or is this out of scope?

  • For quick feedback, I often use the argument filter of devtools::test(), eg devtools::test(filter = "npe"). Is it possible to do something similar with {testit}?

Comment thread .github/workflows/R-CMD-check.yaml Outdated
Comment thread tests/test-all.R
yihui and others added 4 commits May 7, 2026 16:08
- Tighten some all.equal() tests to use %==% for exact equality
- Extract lengthy LHS/RHS expressions of %==% and all.equal() into
  named variables (res, expected) for clarity
- Raise () assertions from inside loops to top-level using vapply()
- Break excessively long lines (>120 chars) into multiple lines
- Add parentheses for operator precedence with %==% (e.g., 3L * 5L)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@yihui yihui force-pushed the convert-tests-to-testit branch from 53a54f1 to 4d9628c Compare May 8, 2026 05:38
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@yihui yihui force-pushed the convert-tests-to-testit branch 5 times, most recently from 8f9ffd3 to 98e7a7c Compare May 8, 2026 06:52
@yihui
Copy link
Copy Markdown
Collaborator Author

yihui commented May 9, 2026

@jdblischak All ergonomic issues have been addressed in testit. May thanks to all your great suggestion! I think they improved the usability of this package by 10 times.

yihui and others added 2 commits May 8, 2026 23:45
The test "s2pwe fails to identify infinity value" used `times2` (c(1, NA)
from a previous test block) instead of `times3` (c(1, Inf)). It appeared
to pass because `s2pwe(times = c(1, NA), ...)` does error, but the intent
was to test Inf handling. With the correct variable, s2pwe(times = c(1,
Inf), ...) returns a valid result (Inf is numeric and positive), so the
test is invalid — there is no Inf validation in s2pwe.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Faithfully translate expect_error(expr, "message") from the original
testthat tests to has_error(expr, "message") in testit, preserving the
original message strings where the function still produces them.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the recent updates to {testit} (thanks @yihui!), I am supportive of this migration of the testing framework.

I observed a slight reduction in the code coverage locally via covr::package_coverage() (93.12% to 92.91%), but according to Codev the coverage is unchanged at 92.90%.

I would like to delay merging until 1) we switch to tagged versions of yihui/actions, and 2) the updated version of {testit} is uploaded to CRAN.

@yihui yihui force-pushed the convert-tests-to-testit branch from 218e0be to 9022c8a Compare May 11, 2026 22:01
@yihui yihui force-pushed the convert-tests-to-testit branch from 9022c8a to eb40106 Compare May 11, 2026 22:05
@yihui
Copy link
Copy Markdown
Collaborator Author

yihui commented May 11, 2026

@jdblischak Actions have been tagged. The CRAN release of testit can be made at any time if you don't have further suggestions or requests.

Copy link
Copy Markdown
Collaborator Author

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is ready for review. I'll release testit v1.0 to CRAN later today and switch to that version accordingly once it lands on CRAN.

To help reviewers understand the changes better, I need to clarify one extra thing I did in this PR, which I should have saved for another PR but I was not sure if it'd be worth torturing you one more time :) The extra thing was that I tightened the equality tests (i.e. expect_equal()) as much as possible for greater rigor. The logic is the following:

  1. Switch to identical() (or equivalently, %==% from testit) whenever possible. If the two objects are strictly identical to each other, we use %==% instead of testing for approximate equality.
  2. When we test things like nrow() or ncol() that are integers, we also try identical testing by changing the target to an integer, e.g., expect_equal(nrow(z1$`_footnotes`), 1) is changed to (nrow(z1$`_footnotes`) %==% 1L) (note the change from double 1 to integer 1L); this is because 1 is not identical to 1L although they are "equal".
  3. Then for the rest of equality tests, try all.equal() with default tolerance (i.e. sqrt(.Machine$double.eps)). If that works, we just use the default all.equal().
  4. If the default tolerance is too tight, we raise it to the nearly smallest tolerance possible to make the test pass, e.g., if the actual difference is 0.00085, we use 0.001. In our original tests, tolerances (if explicitly provided) are often too high, e.g., when 0.003 works but we used 0.01.
  5. When comparing probabilities, we usually use the argument scale = 1 for testing the absolute difference between two probabilities. This gives us a crystal clear idea about how much the two probabilities differ.

BTW, if test coverage is a concern, I think we can easily address it in the next PR.

I'll also add a skill to give AI models instructions on how to write tests with testit in future.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The improvements to the tolerances are very welcome! These have long been a source of frustration for me.

Let's merge once the latest {testit} is available from CRAN.

Comment thread .claude/skills/write-tests.md
Comment thread .claude/skills/write-tests.md Outdated
Comment thread .github/workflows/R-CMD-check.yaml Outdated
Comment thread DESCRIPTION Outdated
Copy link
Copy Markdown
Collaborator

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@jdblischak
Copy link
Copy Markdown
Collaborator

Just noticed this NOTE. Could you please add .claude to .Rbuildignore?

* checking for hidden files and directories ... NOTE
Found the following hidden files and directories:
  .claude
These were most likely included in error. See section ‘Package
structure’ in the ‘Writing R Extensions’ manual.

@yihui
Copy link
Copy Markdown
Collaborator Author

yihui commented May 13, 2026

Sure. Done.

@jdblischak
Copy link
Copy Markdown
Collaborator

Reminder to please squash and merge this PR

@yihui
Copy link
Copy Markdown
Collaborator Author

yihui commented May 13, 2026

I'm not sure who are admins of this repo, but they can go to https://github.com/Merck/gsDesign2/settings and disable "merge commits" and "rebase merging", which is what I do for all my repositories since I rarely need the full commit history of a PR and I always squash and merge:

image

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request migrates the package’s test suite from testthat to testit, aiming to reduce dependency weight while keeping equivalent coverage for design/power utilities and snapshot-style outputs.

Changes:

  • Replaced testthat::test_that()/expect_*() usage with testit::assert() and testit-style comparisons (%==%, all.equal(), has_error()) across the test suite.
  • Moved snapshot-style checks to testit’s markdown snapshot format and removed legacy testthat snapshot artifacts.
  • Updated package metadata and CI workflows to reflect the new testing stack.

Reviewed changes

Copilot reviewed 102 out of 125 changed files in this pull request and generated no comments.

Show a summary per file
File Description
DESCRIPTION Replaces testthat with testit in Suggests.
tests/test-all.R New test runner entrypoint for testit (test_pkg("gsDesign2")).
tests/testthat.R Removed legacy testthat runner.
tests/testthat/test-independent-gs_design_combo.R Removed testthat version of these tests.
tests/testthat/test-independent-expected_time.R Removed testthat version of these tests.
tests/testthat/test-independent-expected_event.R Removed testthat version of these tests.
tests/testthat/test-independent-expected_accrual.R Removed testthat version of these tests.
tests/testthat/test-independent-check_arg.R Removed testthat version of these tests.
tests/testthat/test-independent_s2pwe.R Removed testthat version of these tests.
tests/testthat/test-independent_ppwe.R Removed testthat version of these tests.
tests/testthat/test-independent_as_rtf.R Removed testthat snapshot tests for RTF output.
tests/testthat/test-independent_as_gt.R Removed testthat snapshot tests for gt output.
tests/testthat/test-developer-gs_spending_bound.R Removed testthat developer test.
tests/testthat/test-developer-gs_b.R Removed testthat developer test.
tests/testthat/_snaps/independent_as_rtf/gs_design_wlr.rtf Removed testthat snapshot artifact.
tests/testthat/_snaps/independent_as_rtf/gs_design_rd.rtf Removed testthat snapshot artifact.
tests/testthat/_snaps/independent_as_rtf/gs_design_ahr.rtf Removed testthat snapshot artifact.
tests/testthat/_snaps/independent_as_rtf/fixed_design_ahr.rtf Removed testthat snapshot artifact.
tests/testthat/_snaps/independent_as_rtf/fixed_design_ahr_title.rtf Removed testthat snapshot artifact.
tests/testthat/_snaps/independent_as_rtf/fixed_design_ahr_footnote.rtf Removed testthat snapshot artifact.
tests/testit/helper.R Adds shared helper (gt_to_latex) for testit tests.
tests/testit/helper-support-as_rtf.R Adds helpers used by testit snapshot-style RTF tests.
tests/testit/helper-double-programming-expected_accrual.R Adds double-programming helper for accrual validation.
tests/testit/helper-double-programming-expected_event.R Adds double-programming helper for event validation.
tests/testit/helper-double-programming-expected_time.R Adds helper for expected_time validation.
tests/testit/helper-double-programming-gs_design_combo.R Adds helper for gs_design_combo validation.
tests/testit/helper-double-programming-gs_info_ahr.R Adds helper for gs_info_ahr validation.
tests/testit/helper-double-programming-ppwe.R Adds double-programming helpers for ppwe validation.
tests/testit/helper-old-version-tEvents_.R Adds old-version helper code used for regression-style comparisons.
tests/testit/helper-old-version-gs_power_ahr_.R Adds old-version helper code used for regression-style comparisons.
tests/testit/helper-old-version-gs_info_ahr_.R Adds old-version helper code used for regression-style comparisons.
tests/testit/helper-old-version-eEvents_.R Adds old-version helper code used for regression-style comparisons.
tests/testit/helper-old-version-eAccrual_.R Adds old-version helper code used for regression-style comparisons.
tests/testit/test-independent-wlr_weight.R Converts tests to testit for WLR weight utilities.
tests/testit/test-independent-utility_tbl.R Converts tbl utility tests to testit.
tests/testit/test-independent-to_integer.R Converts to_integer tests to testit.
tests/testit/test-independent-rmst.R Converts RMST tests to testit.
tests/testit/test-independent-hupdate.R Converts hupdate tests to testit.
tests/testit/test-independent-h1.R Converts h1 tests to testit.
tests/testit/test-independent-gs_update_ahr.R Converts gs_update_ahr tests to testit.
tests/testit/test-independent-gs_spending_combo.R Converts gs_spending_combo tests to testit.
tests/testit/test-independent-gs_spending_bound.R Converts gs_spending_bound tests to testit.
tests/testit/test-independent-gs_power_npe.R Converts gs_power_npe tests to testit.
tests/testit/test-independent-gs_power_combo.R Converts gs_power_combo tests to testit.
tests/testit/test-independent-gs_power_ahr.R Converts gs_power_ahr tests to testit.
tests/testit/test-independent-gs_info_combo.R Converts gs_info_combo tests to testit.
tests/testit/test-independent-gs_info_ahr.R Converts gs_info_ahr tests to testit.
tests/testit/test-independent-gs_design_combo.R Adds testit version of gs_design_combo tests.
tests/testit/test-independent-gs_design_ahr.R Converts gs_design_ahr tests to testit.
tests/testit/test-independent-gs_bound.R Converts gs_bound tests to testit.
tests/testit/test-independent-gs_b.R Converts gs_b tests to testit.
tests/testit/test-independent-gridpts.R Converts gridpts tests to testit.
tests/testit/test-independent-fixed_design.R Converts fixed design tests to testit.
tests/testit/test-independent-expected_time.R Adds testit version of expected_time tests.
tests/testit/test-independent-expected_event.R Adds testit version of expected_event tests.
tests/testit/test-independent-expected_accrual.R Adds testit version of expected_accrual tests.
tests/testit/test-independent-check_arg.R Adds testit version of argument-check tests.
tests/testit/test-independent_s2pwe.R Adds testit version of s2pwe tests.
tests/testit/test-independent_ppwe.R Adds testit version of ppwe tests.
tests/testit/test-independent_as_gt.R Adds basic non-snapshot as_gt-related tests under testit.
tests/testit/test-independent-ahr.R Converts ahr tests to testit.
tests/testit/test-independent-ahr_blinded.R Converts ahr_blinded tests to testit.
tests/testit/test-independent_gs_power_wlr.R Converts gs_power_wlr tests to testit.
tests/testit/test-independent_gs_info_wlr.R Converts gs_info_wlr tests to testit.
tests/testit/test-independent_gs_design_wlr.R Converts gs_design_wlr tests to testit.
tests/testit/test-developer-sequential_pval.R Converts sequential_pval developer tests to testit.
tests/testit/test-developer-pw_info.R Converts pw_info developer tests to testit.
tests/testit/test-developer-gs_spending_bound.R Adds testit version of gs_spending_bound developer test.
tests/testit/test-developer-gs_power_wlr.R Converts gs_power_wlr developer tests to testit.
tests/testit/test-developer-gs_power_rd.R Converts gs_power_rd developer tests to testit.
tests/testit/test-developer-gs_power_npe.R Converts gs_power_npe developer tests to testit.
tests/testit/test-developer-gs_info_wlr.R Converts gs_info_wlr developer tests to testit.
tests/testit/test-developer-gs_info_rd.R Converts gs_info_rd developer tests to testit.
tests/testit/test-developer-gs_design_rd.R Converts gs_design_rd developer tests to testit.
tests/testit/test-developer-gs_design_npe.R Converts gs_design_npe developer tests to testit.
tests/testit/test-developer-gs_cp.R Converts gs_cp developer tests to testit.
tests/testit/test-developer-gs_cp_simple.R Converts gs_cp_simple developer tests to testit.
tests/testit/test-developer-gs_cp_npe2.R Converts gs_cp_npe2 developer tests to testit.
tests/testit/test-developer-gs_cp_npe1.R Converts gs_cp_npe1 developer tests to testit.
tests/testit/test-developer-gs_b.R Adds testit version of gs_b developer test.
tests/testit/test-developer-gridpts.R Converts gridpts developer tests to testit.
tests/testit/test-developer-fastlag.R Converts fastlag developer tests to testit.
tests/testit/test-developer-expected_time.R Converts expected_time developer tests to testit.
tests/testit/test-developer-expected_event.R Converts expected_event developer tests to testit.
tests/testit/test-developer-as_gt.R Converts as_gt developer tests to testit.
tests/testit/test-developer-ahr.R Converts ahr developer tests to testit.
tests/testit/fixtures/simulation_test_data.Rdata Adds/updates fixture data used by tests.
tests/testit/fixtures/sim_gsd_pMaxCombo_exp1_H1_test.Rdata Adds/updates fixture data used by tests.
tests/testit/fixtures/sim_gsd_pMaxCombo_exp1_H0_test.Rdata Adds/updates fixture data used by tests.
.github/workflows/test-coverage.yaml Updates coverage workflow dependency setup consistent with testit migration.
.github/workflows/R-CMD-check.yaml Updates R CMD check workflow actions/dependency setup.
.Rbuildignore Ignores .claude artifacts.
.gitignore Minor formatting cleanup.
gsDesign2.Rproj Removes PackageUseDevtools setting.
.claude/skills/write-tests.md Adds repository guidance for writing testit-based tests.
Comments suppressed due to low confidence (4)

tests/testit/test-independent_gs_power_wlr.R:100

  • This compares the Z boundaries with %==% (exact identical()), but these are floating-point values coming from numerical routines and were previously checked with a tolerance. Using exact comparison here is likely to make the test flaky across platforms/R versions; consider switching back to all.equal(..., tolerance = ...) (e.g., the prior 0.1 tolerance).
    tests/testit/test-independent-ahr_blinded.R:27
  • These assertions use %==% (exact identical()) for ahr, theta, and info0, but the expected values are computed via floating-point ops and the old test used tolerances. To reduce cross-platform flakiness, use all.equal(..., tolerance = ...) for these numeric comparisons (and keep %==% for the integer event count if desired).
    tests/testit/test-developer-gs_info_rd.R:60
  • res and expected here are computed from floating-point arithmetic, so comparing with %==% (exact identical()) is brittle and may fail due to minor numerical differences. Use all.equal(res, expected, tolerance = ...) (similar to the surrounding assertions) for these checks.

This issue also appears on line 84 of the same file.
tests/testit/test-developer-gs_info_rd.R:90

  • Same issue as above: this compares a floating-point result with %==% (exact identical()), which is likely to be flaky. Prefer all.equal(res, expected, tolerance = ...) for numeric equality here.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@keaven keaven merged commit e9e2206 into main May 19, 2026
11 checks passed
@LittleBeannie LittleBeannie deleted the convert-tests-to-testit branch May 19, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants