From 2e8383b098de0c157858022aa4ce58f36989276d Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Thu, 30 Jul 2026 11:31:28 -0400 Subject: [PATCH] fix: let the doc-content tests survive R CMD check CI failed on the previous commit. testthat::test_local() from a checkout was green, but rcmdcheck was not: under R CMD check the suite runs against the INSTALLED package, where README.md, vignettes/ and man/ do not exist. Both newly-activated tests read them through test_path("..", "..", ...) and died on `cannot open the connection`. The defect was latent in the committed tests, not introduced here -- they shipped skip()ped, so CI had never executed either one. Removing the skips is what exposed it, which is the mechanism working as intended. Guarded with skip_if_no_source_tree(), so they skip in the installed-package context that structurally cannot satisfy them. They are NOT thereby unchecked in CI: the workflow runs testthat::test_local() from the checkout as its own step before rcmdcheck, and there the paths resolve and the assertions run. Deliberately not split: test-peer-summary-scope.R's numeric pin needs only the corpus and would survive check on its own, but it exists to protect the sentence above it. Separating them would let the prose drift while the pin kept passing. Verified locally: test_local 629 pass / 0 fail / 3 skip; rcmdcheck 0 errors / 0 warnings / 0 notes. --- tests/testthat/helper-fixture.R | 27 +++++++++++++++++++ tests/testthat/test-amount-units-documented.R | 15 ++++++++--- tests/testthat/test-peer-summary-scope.R | 11 ++++++-- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/tests/testthat/helper-fixture.R b/tests/testthat/helper-fixture.R index 30c7225..47f0726 100644 --- a/tests/testthat/helper-fixture.R +++ b/tests/testthat/helper-fixture.R @@ -6,6 +6,33 @@ fixture_corpus_path <- function() { if (nzchar(p)) paste0(p, "/") else "" } +# Path to a file in the SOURCE tree (README.md, man/*.Rd, vignettes/*.Rmd), +# or "" when it isn't there. +# +# Tests that assert on documentation content have to read the sources, and the +# sources only exist when the suite runs from a checkout. Under R CMD check the +# suite runs from the INSTALLED package, where man/ and vignettes/ are not +# shipped and `../../README.md` does not resolve -- so those tests must skip +# rather than error. CI runs testthat::test_local() from the checkout BEFORE +# rcmdcheck, so the assertions are still enforced on every push; this only +# stops them from failing a context that structurally cannot satisfy them. +source_tree_path <- function(...) { + p <- testthat::test_path("..", "..", ...) + if (file.exists(p)) p else "" +} + +# Skip unless every named source file is present (see source_tree_path()). +skip_if_no_source_tree <- function(...) { + paths <- vapply(list(...), function(rel) do.call(source_tree_path, as.list(rel)), + character(1)) + missing <- vapply(paths, function(p) !nzchar(p), logical(1)) + testthat::skip_if( + any(missing), + "package source tree not available (running against the installed package)" + ) + invisible(paths) +} + # Skip a test if no corpus is reachable (bundled fixture or explicit remote URL). skip_if_no_corpus <- function() { p <- fixture_corpus_path() diff --git a/tests/testthat/test-amount-units-documented.R b/tests/testthat/test-amount-units-documented.R index 7d008cb..07d3d7d 100644 --- a/tests/testthat/test-amount-units-documented.R +++ b/tests/testthat/test-amount-units-documented.R @@ -18,16 +18,23 @@ test_that("returned amounts are documented as full US dollars where readers meet the package", { + # README and vignettes ship only in the source tree, not in the installed + # package, so these assertions cannot run under R CMD check -- CI's earlier + # testthat::test_local() step is what enforces them. See + # skip_if_no_source_tree() in helper-fixture.R. + docs <- skip_if_no_source_tree( + "README.md", + c("vignettes", "total-spending.Rmd"), + c("vignettes", "population-denominators.Rmd") + ) + says_units <- function(path) { txt <- paste(readLines(path, warn = FALSE), collapse = " ") grepl("full US dollars|full U\\.S\\. dollars", txt, ignore.case = TRUE) && grepl("\\$1,000s|thousands of dollars", txt, ignore.case = TRUE) } - expect_true(says_units(testthat::test_path("..", "..", "README.md"))) - expect_true(says_units(testthat::test_path("..", "..", "vignettes", "total-spending.Rmd"))) - expect_true(says_units(testthat::test_path("..", "..", "vignettes", - "population-denominators.Rmd"))) + for (path in docs) expect_true(says_units(path)) # Pin the documented claim to the actual behaviour, so the two cannot drift. # The expected raw amount is read straight from the corpus's parquet diff --git a/tests/testthat/test-peer-summary-scope.R b/tests/testthat/test-peer-summary-scope.R index 3746d1e..a4575a9 100644 --- a/tests/testthat/test-peer-summary-scope.R +++ b/tests/testthat/test-peer-summary-scope.R @@ -14,8 +14,15 @@ test_that("cog_peer_compare() documents that summary_* rows are per-category quantiles", { - rd <- paste(readLines(testthat::test_path("..", "..", "man", "cog_peer_compare.Rd"), - warn = FALSE), collapse = " ") + # man/ ships only in the source tree (the installed package carries a + # compiled help database instead), so the prose assertions below cannot run + # under R CMD check -- CI's earlier testthat::test_local() step enforces + # them. The numeric pin further down needs only the corpus, but it lives in + # the same test_that() as the sentence it protects, deliberately: they are + # one claim, and splitting them would let the prose drift while a separate + # test kept passing. + rd_path <- skip_if_no_source_tree(c("man", "cog_peer_compare.Rd")) + rd <- paste(readLines(rd_path, warn = FALSE), collapse = " ") # The @return section must say the quantile is computed within each cell... expect_match(rd, "within each|per-category|per category", ignore.case = TRUE)