Support release by llrs-roche · Pull Request #567 · insightsengineering/cards

llrs-roche · 2026-05-22T11:08:34Z

This PR makes tests more robust: there are snapshosts but the core functionality/output is also tested outside of it.

It also helps with some documentation checks for CRAN.

This is to help with the release of the package started on #562

Pre-review Checklist (if item does not apply, mark is as complete)

All GitHub Action workflows pass with a ✅
PR branch has pulled the most recent updates from master branch: usethis::pr_merge_main()
If a bug was fixed, a unit test was added.
Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): devtools::test_coverage()
Request a reviewer

Reviewer Checklist (if item does not apply, mark is as complete)

If a bug was fixed, a unit test was added.
Run pkgdown::build_site(). Check the R console for errors, and review the rendered website.
Code coverage is suitable for any new functions/features: devtools::test_coverage()

When the branch is ready to be merged:

Update NEWS.md with the changes from this pull request under the heading "# cards (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (see NEWS.md for examples).
All GitHub Action workflows pass with a ✅
Approve Pull Request
Merge the PR. Please use "Squash and merge" or "Rebase and merge".

Optional Reverse Dependency Checks:

Install checked with pak::pak("Genentech/checked") or pak::pak("checked")

# Check dev versions of `cardx`, `gtsummary`, and `tfrmt` which are in the `ddsjoberg` R Universe
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2L, repos = c('https://ddsjoberg.r-universe.dev', 'https://cloud.r-project.org'))"

# Check CRAN reverse dependencies but run tests skipped on CRAN
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')"

# Check CRAN reverse dependencies in a CRAN-like environment
Rscript -e "options(checked.check_envvars = c(NOT_CRAN = FALSE), checked.check_build_args = '--as-cran'); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')"

…_row tests.

github-actions · 2026-05-22T11:13:34Z

Code Coverage Summary

Filename                       Stmts    Miss  Cover    Missing
---------------------------  -------  ------  -------  ----------------------------------------------
R/add_calculated_row.R            53       6  88.68%   51-56
R/apply_fmt_fun.R                116       0  100.00%
R/ard_attributes.R                46       1  97.83%   57
R/ard_formals.R                   13       0  100.00%
R/ard_hierarchical.R             104      12  88.46%   88-93, 187-192
R/ard_identity.R                  13       0  100.00%
R/ard_missing.R                   70       7  90.00%   45-50, 61
R/ard_mvsummary.R                 54       7  87.04%   76-81, 100
R/ard_pairwise.R                  50       0  100.00%
R/ard_stack_hierarchical.R       276       0  100.00%
R/ard_stack.R                     97       0  100.00%
R/ard_strata.R                    38       0  100.00%
R/ard_summary.R                  200       8  96.00%   87-92, 226-227
R/ard_tabulate_rows.R             12       0  100.00%
R/ard_tabulate_value.R            78       7  91.03%   51-56, 73
R/ard_tabulate.R                 423      16  96.22%   108-113, 264, 487-491, 498-499, 676, 709
R/ard_total_n.R                   14       0  100.00%
R/as_card_fn.R                     8       0  100.00%
R/as_card.R                       12       0  100.00%
R/as_nested_list.R                41       0  100.00%
R/bind_ard.R                      47      11  76.60%   74-85
R/cards-package.R                  1       1  0.00%    14
R/check_ard_structure.R           55       5  90.91%   35-39
R/compare_ard_helpers.R          127      32  74.80%   44-50, 66-69, 113, 148, 153, 158-184, 227, 230
R/compare_ard.R                   26       0  100.00%
R/default_stat_labels.R           20       0  100.00%
R/deprecated.R                    34      30  11.76%   40-41, 56-73, 88-134
R/eval_capture_conditions.R       30       0  100.00%
R/filter_ard_hierarchical.R      207       0  100.00%
R/get_ard_statistics.R            16       0  100.00%
R/maximum_variable_value.R        15       0  100.00%
R/mock.R                         137       2  98.54%   116, 245
R/nest_for_ard.R                  80       1  98.75%   64
R/print_ard_conditions.R          83       0  100.00%
R/print.R                        101       2  98.02%   164-165
R/process_selectors.R            126       1  99.21%   337
R/rename_ard_columns.R            81       0  100.00%
R/rename_ard_groups.R             60       0  100.00%
R/replace_null_statistic.R        11       0  100.00%
R/round5.R                         1       0  100.00%
R/selectors.R                     23       0  100.00%
R/shuffle_ard.R                  167       0  100.00%
R/sort_ard_hierarchical.R        162       0  100.00%
R/summary_functions.R             25       1  96.00%   59
R/tidy_ard_order.R                34       0  100.00%
R/tidy_as_ard.R                   40       0  100.00%
R/unlist_ard_columns.R            27       0  100.00%
R/update_ard.R                    60       6  90.00%   54-59
R/utils.R                         30       0  100.00%
TOTAL                           3544     156  95.60%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: e9959fd

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

github-actions · 2026-05-22T11:13:53Z

Unit Tests Summary

1 files 273 suites 1m 9s ⏱️
272 tests 202 ✅ 70 💤 0 ❌
697 runs 575 ✅ 122 💤 0 ❌

Results for commit e9959fd.

♻️ This comment has been updated with latest results.

github-actions · 2026-05-22T11:13:58Z

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
filter_ard_hierarchical	👶		$+0.00$	$+1$	$+1$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on `main`	$±Time$	Test Case
add_calculated_row	👶		$+0.01$	add_calculated_row_expr_errors_when_a_variable_is_not_present
add_calculated_row	💀	$0.04$	$-0.04$	add_calculated_row_expr_messaging
add_calculated_row	💚	$1.29$	$-1.24$	add_calculated_row_x_
apply_fmt_fun	💔	$0.03$	$+1.27$	apply_fmt_fun_works
ard_stack_hierarchical	💔	$4.07$	$+1.19$	ard_stack_hierarchical_variables_
ard_summary	💚	$1.97$	$-1.39$	ard_summary_works
eval_capture_conditions	💀	$0.05$	$-0.05$	unnamed
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_error_messaging_works
filter_ard_hierarchical	👶		$+0.01$	filter_ard_hierarchical_keep_empty_works
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_returns_only_summary_rows_when_all_rows_filtered_out
filter_ard_hierarchical	👶		$+0.01$	filter_ard_hierarchical_var_works
filter_ard_hierarchical	👶		$+10.89$	filter_ard_hierarchical_works
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_works_when_some_variables_not_included_in_x
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_works_with_ard_stack_hierarchical_count_results
filter_ard_hierarchical	👶		$+0.02$	filter_ard_hierarchical_works_with_column_specific_filters
filter_ard_hierarchical	👶		$+0.02$	filter_ard_hierarchical_works_with_non_standard_filters
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_works_with_only_one_variable_in_x
filter_ard_hierarchical	👶		$+0.00$	filter_ard_hierarchical_works_with_overall_data
selectors	👶		$+0.04$	unnamed
shuffle_ard	💀	$0.04$	$-0.04$	unnamed
shuffle_ard	💚	$6.87$	$-3.59$	shuffle_trim_works
tidy_as_ard	💀	$0.28$	$-0.28$	tidy_as_ard_works
tidy_as_ard	👶		$+0.01$	tidy_as_ard_works_when_formals_argument_is_not_passed.
tidy_as_ard	👶		$+0.02$	tidy_as_ard_works_when_fun_args_to_record_argument_is_not_passed
tidy_as_ard	👶		$+0.01$	tidy_as_ard_works_when_primary_stats_function_errors
tidy_as_ard	👶		$+0.26$	tidy_as_ard_works_with_standard_use

Results for commit 6410a3c

♻️ This comment has been updated with latest results.

Melkiades · 2026-05-22T14:22:39Z

+  expect_message(
+    expect_message(
+      expect_message(
+        print_ard_conditions(ard),
+        "were returned during"
+      ),
+      "tis an error"
+    ),
+    "were returned during"
  )


This can be replaced by testthat::capture_messages() so to avoid nesting

But I don't want to do so! The documentation help pages of testthat::capture_messages() says: "We no longer recommend that you use these functions, instead relying on the expect_message()". We already have a snapshot below to capture all the messages and warnings.

This is to ensure we get exactly these three messages and on this order. But maybe there is a different way to express it.

shajoezhu · 2026-05-25T08:46:15Z

-      variables = list(AGEGR1 = factor(c("<65", "65-80", ">80"), levels = c("<65", "65-80", ">80"))),
-      by = list(TRTA = c("Placebo", "Xanomeline High Dose", "Xanomeline Low Dose"))
-    ) |>
-      apply_fmt_fun()


hi @llrs-roche , I was wondering why we remove the apply_fmt_fun here? thanks

The description of the (unit) test explains it is for mock_categorical(). There's no need to test apply_fmt_fun(), which is covered on its own unit test on file test-apply_fmt_fun.R.
If there is need for integration test the description of what we are testing should be clearer

shajoezhu · 2026-05-25T08:46:58Z

-    mock_continuous(
-      variables = c("AGE", "BMIBL")
-    ) |>
-      apply_fmt_fun()


same question as above

The description of the (unit) test explains it is for mock_continuous(). There's no need to test apply_fmt_fun(), which is covered on its own unit test on file test-apply_fmt_fun.R.
If there is need for integration test the description of what we are testing should be clearer. Besides it makes tests faster.

shajoezhu · 2026-05-25T08:47:36Z

-    mock_missing(
-      variables = c("AGE", "BMIBL")
-    ) |>
-      apply_fmt_fun()


i was wondering if this function was tested anywhere>

It is covered on its own unit test on file test-apply_fmt_fun.R.

shajoezhu

Thanks @llrs-roche , some minor question regarding the test, but most looks good to me.

shajoezhu

Thanks a lot @llrs-roche for checking the tests. I have no further questions.

ddsjoberg · 2026-05-26T03:40:25Z

@llrs-roche @shajoezhu there are a lot of changes here unrelated to the release. I know we all have our own "styles" for pkg development, and any of these opinionated changes you want to make should be discussed before they are put into a PR. Please revert changes unrelated to the release, so we can merge and submit to CRAN. If it's tricky to disentangle the updates, i can go ahead and submit without this PR. Please don't delete unit tests.

shajoezhu · 2026-05-26T06:57:23Z

Hi @ddsjoberg , yes, absolutely agree, we should align these changes before merging into main.

please go ahead with the CRAN relase, adn we can update these changes for later.

llrs-roche added 10 commits May 22, 2026 10:54

Add tests without snapshots. Remove apply_fmt_fun() on add_calculated…

acfe7ba

…_row tests.

Add some test that are not snapshots

cac441a

Add tests that are not snapshots

a7c326a

Do not skip this package on CRAN or CI: it works

d0c2875

Simplify testing mocked functions

8b08f4a

Use some expectations outside snapshots

f95f55c

Further testing without snapshots

3b1071e

Fix some minor issues with documentation for CRAN

7b20a35

Fix tests about names

d137c0b

Fix style

bd2f1ab

llrs-roche requested review from ddsjoberg and shajoezhu May 22, 2026 12:23

Melkiades reviewed May 22, 2026

View reviewed changes

shajoezhu reviewed May 25, 2026

View reviewed changes

More comprehensive tests for ard_formals

db11d1f

shajoezhu approved these changes May 25, 2026

View reviewed changes

Merge branch 'main' into 526_support_release

e9959fd

Uh oh!

Conversation

llrs-roche commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

github-actions Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

github-actions Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Performance Difference

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shajoezhu left a comment

Choose a reason for hiding this comment

Uh oh!

shajoezhu left a comment

Choose a reason for hiding this comment

Uh oh!

ddsjoberg commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shajoezhu commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

llrs-roche commented May 22, 2026 •

edited

Loading

github-actions Bot commented May 22, 2026 •

edited

Loading

github-actions Bot commented May 22, 2026 •

edited

Loading

github-actions Bot commented May 22, 2026 •

edited

Loading

ddsjoberg commented May 26, 2026 •

edited

Loading