Restore compatibility with roxygen2 8.x (fixes CRAN archival notice) - #136
Open
VincentGuyader wants to merge 6 commits into
Open
Restore compatibility with roxygen2 8.x (fixes CRAN archival notice)#136VincentGuyader wants to merge 6 commits into
VincentGuyader wants to merge 6 commits into
Conversation
* Convert to testthat 3e * Switch to parallel testing (~170s -> ~60s) * Don't change global state in `helpers.R` * Add new `local_` helpers to quiet all output * Add `add = TRUE` to bare `on.exit()` calls
roxygen2 8.0.0 migrates the DESCRIPTION RoxygenNote field to Config/roxygen2/version. usethis (<= 3.2.1) still detects roxygen through the removed RoxygenNote field, so usethis::use_pipe() aborts with 'Package checkpackage does not use roxygen2' on any package documented by roxygen2 8.x. This is what failed on the CRAN Fedora r-devel flavors and triggered the archival notice. The use_pipe() call was pure test scaffolding: the magrittr pipe reexport was never part of the asserted 12-function contract, and the audit parses source without executing the fixture functions. Drop it, add a dedicated regression guard for the roxygen2 8.x field migration and the end-to-end find_missing_tags() flow, and raise the roxygen2 test floor to 7.3.0, the boundary the suite actually branches on.
- Depends: R (>= 4.1), matching the requirement of roxygen2 8.0.0 (previously inconsistent at 4.0) - migrate RoxygenNote to Config/roxygen2/version via roxygen2 8.1.0 re-documentation: consolidated importFrom directives in NAMESPACE, refreshed Rd files (maintainer now listed in Authors, roclets docs inherited from roxygen2 8.x) - NEWS.md entry for 1.0.1
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores checkhelper’s test-suite compatibility with roxygen2 8.x (addressing the CRAN archival trigger) by removing usethis::use_pipe() scaffolding that no longer works when roxygen metadata migrates from RoxygenNote to Config/roxygen2/version, and by adding regression coverage for the roxygen2 8.x migration behavior.
Changes:
- Remove/avoid
usethis::use_pipe()usage in tests and add a dedicated roxygen2 8.x compatibility regression test. - Harden test requirements (roxygen2 version floor) and reduce noisy test output via new local helper utilities.
- Bump package metadata to 1.0.1, update roxygen config fields, and refresh generated docs/NAMESPACE.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-use_data_doc.R | Switches to quiet_create_example_pkg() and silences output in the test. |
| tests/testthat/test-use_data_doc_overwrite.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-s3_missing_value.R | Uses quiet fixture creation and local output silencing for the S3 fixture pkg. |
| tests/testthat/test-roxygen2-8x-compat.R | New regression tests guarding roxygen2 8.x DESCRIPTION field migration + end-to-end audit flow. |
| tests/testthat/test-returns_alias.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-requirements.R | New explicit roxygen2 version floor assertion for the test suite. |
| tests/testthat/test-rdname_topic_block.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-inherit_return.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-get_data_info.R | Adds deprecation quieting and updates an assertion helper. |
| tests/testthat/test-fix-globals.R | Uses quiet fixture creation and local output silencing. |
| tests/testthat/test-fix-globals-multi-run.R | Adds local quieting to reduce noise across multi-run tests; uses quiet fixture creation in E. |
| tests/testthat/test-fix-globals-merge.R | Adds local quieting to reduce noise across merge/extractor tests. |
| tests/testthat/test-fix-dataset-doc.R | Makes on.exit() additive to avoid clobbering other teardown handlers. |
| tests/testthat/test-fix-ascii.R | Uses quiet fixture creation + additive teardown for temp packages. |
| tests/testthat/test-find_missing_values.R | Drops use_pipe() scaffolding; tightens message/warning expectations; uses quiet fixture creation. |
| tests/testthat/test-env_pollution.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-empty_package.R | Adds local quieting helpers to reduce test noise. |
| tests/testthat/test-deprecated.R | Uses quiet fixture creation and adjusts warning expectations to isolate deprecation warnings. |
| tests/testthat/test-create_example_pkg.R | Adds local quieting and uses quiet fixture creation for coverage cases. |
| tests/testthat/test-checkhelper.R | Uses quiet fixture creation and opts out of deprecation warnings per-test. |
| tests/testthat/test-check_clean_userspace.R | Uses quiet fixture creation + deprecation quieting to reduce noise. |
| tests/testthat/test-check_clean_userspace_robust.R | Uses quiet fixture creation and clarifies warning assertion structure. |
| tests/testthat/test-check_as_cran.R | Uses quiet fixture creation and local output silencing. |
| tests/testthat/test-audit-userspace.R | Uses quiet fixture creation; refactors expect_message() to capture returned values. |
| tests/testthat/test-audit-tags.R | Uses quiet fixture creation and local quieting helpers for message-heavy paths. |
| tests/testthat/test-audit-globals.R | Uses quiet fixture creation and local output silencing. |
| tests/testthat/test-audit-globals-coverage.R | Adds local quieting across many unit tests to reduce noise. |
| tests/testthat/test-audit-dontrun.R | Adds local quieting and refines warning suppression around readLines() failure cases. |
| tests/testthat/test-audit-dataset-doc.R | Makes on.exit() additive to avoid clobbering other teardown handlers. |
| tests/testthat/test-audit-citation.R | Adds local quieting across tests to reduce noise. |
| tests/testthat/test-audit-check.R | Refactors expect_message() to capture returned values. |
| tests/testthat/test-audit-ascii.R | Uses quiet fixture creation + additive teardown for temp packages. |
| tests/testthat/test-asciify.R | Refactors warning assignment; adds local quieting for deprecation/output. |
| tests/testthat/test-asciify-edge-cases.R | Adds local deprecation quieting across edge-case tests. |
| tests/testthat/helpers.R | Replaces global test-side effects with local helpers (local_quiet*, quiet_create_example_pkg, tempdir cleanup). |
| R/globals.R | Namespaces globalVariables() as utils::globalVariables(). |
| NEWS.md | Adds 1.0.1 release notes highlighting roxygen2 8.x compatibility fix. |
| NAMESPACE | Consolidates importFrom() directives (generated format). |
| man/find_missing_tags.Rd | Updates docs for roclets argument (roxygen2 8.x output). |
| man/checkhelper-package.Rd | Updates package author listing in generated docs. |
| DESCRIPTION | Bumps version, updates R dependency, migrates roxygen2 metadata to Config/roxygen2/version, and adds testthat config fields. |
Files not reviewed (2)
- man/checkhelper-package.Rd: Generated file
- man/find_missing_tags.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Confirmed findings of the multi-agent pre-CRAN audit, each verified adversarially before being applied: - cran-comments.md rewritten for the 1.0.1 patch release (was still describing the 1.0.0 feature release and the maintainer change) - DESCRIPTION: declare roxygen2 (>= 7.3.0) in Imports (the test suite hard-requires it) and drop the dead LazyData field (no data/) - NEWS.md: disclose the R >= 4.1 floor bump under Breaking changes and refer to usethis:::uses_roxygen() as the internal it is - unwrap three fast, side-effect-free examples from dontrun with requireNamespace guards (audit_tags, audit_ascii, fix_ascii) per CRAN policy on executable examples; regenerate their Rd - test-deprecated.R: point check_as_cran(check_output=) at a tempfile so the deprecation test stops leaking a check/ dir in the source tree - test-use_data_doc.R: clean the file-scope example package fixture at teardown (Copilot finding, validated) - asciify six comment-only non-ASCII occurrences in test files; intentional non-ASCII fixtures untouched - trim bug-history comments in tests to forward-looking rationale
…chain The regression guard asserted that a package scaffolded under roxygen2 >= 8.0.0 carries Config/roxygen2/version. On win-builder r-devel the usethis/attachment/roxygen2 combination leaves the legacy RoxygenNote field instead, failing the check while the audited behaviour itself is fine (the end-to-end audit test passes there). Assert the invariant checkhelper actually relies on, the same one usethis uses: the package counts as roxygen-managed when either RoxygenNote or Config/roxygen2/version is present. The assertion still fails when neither field exists.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 49 changed files in this pull request and generated no new comments.
Files not reviewed (5)
- man/audit_ascii.Rd: Generated file
- man/audit_tags.Rd: Generated file
- man/checkhelper-package.Rd: Generated file
- man/find_missing_tags.Rd: Generated file
- man/fix_ascii.Rd: Generated file
Suppressed comments (1)
tests/testthat/test-audit-tags.R:2
quiet_create_example_pkg()returns<tempdir>/pkg-xxxx/checkpackage(seecreate_example_pkg()), sounlink(path, recursive = TRUE)removes only the package directory and leaves the parentpkg-xxxxdirectory behind. Over many tests (especially with parallel execution), this can unnecessarily pollutetempdir()and potentially interfere with tests that snapshot/cleantempdir().
withr::defer(unlink(path, recursive = TRUE), teardown_env())
The archival trigger is not Fedora specific. CRAN reports ERROR on the four
r-devel Linux flavors (debian and fedora, clang and gcc), on
r-patched-linux-x86_64, on r-release-windows-x86_64 and on
r-oldrel-windows-x86_64, that is on every flavor that has already picked up
roxygen2 8.x. cran-comments.md and NEWS.md both claimed the failure was
limited to the r-devel Fedora flavors.
After the win-builder fix, the first assertion of test-roxygen2-8x-compat.R
could no longer fail for the reason it exists: both roxygen2 lines satisfy
`any(has_fields(c("RoxygenNote", "Config/roxygen2/version")))`, so the guard
verified nothing on any flavor. A new test writes the post-8.x DESCRIPTION
shape by hand (RoxygenNote deleted, Config/roxygen2/version set) and asserts
find_missing_tags() still classifies the fixture, which exercises the exact
broken state everywhere instead of only where the scaffolding toolchain
happens to migrate. The existing test now also asserts that a migrated
DESCRIPTION carries the new field alone.
Config/testthat/parallel arrived with #135 and was not disclosed in this
branch's housekeeping list. It buys nothing on CRAN, where testthat caps at
two workers, and adds a flakiness surface while the package sits under an
archival deadline. Measured cost of running serially: 324s against 233s.
Config/testthat/edition 3, also new with #135, is kept: the rewrites from
`out <- expect_message(f())` to `expect_message(out <- f())` that it requires
are correct and necessary, since edition 3 returns the condition rather than
the value.
Also guard create_example_pkg()'s own example with the same requireNamespace()
pair as the three examples this branch unwrapped from \dontrun{}, move the
CONTRIBUTING.md roxygen2 floor from the stale `> 7.1.2` to `>= 7.3.0` to match
Imports and test-requirements.R, and teach .Rbuildignore to drop *.stackdump:
a Git Bash crash file landing at the repository root is shipped by R CMD build
and raises a "Non-standard file/directory found at top level" NOTE.
Verified on the result: R CMD check --as-cran is Status OK with 0 failures,
601 passing assertions and 8 skips, run once under roxygen2 7.3.3 and once
under roxygen2 8.1.0 in an isolated library.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restore compatibility with roxygen2 8.x (CRAN archival fix)
Why
CRAN flagged
checkhelper1.0.0 for archival: the r-devel Fedora flavors(which ship roxygen2 8.x, released to CRAN 2026-08-04) fail the test suite,
while every other flavor is OK.
Original CRAN error (
test-find_missing_values.R):Root cause
roxygen2 8.0.0 migrates the DESCRIPTION
RoxygenNotefield toConfig/roxygen2/versionand dropsRoxygenNote. usethis (<= 3.2.1) stilldetects roxygen through the old field:
So on any package documented by roxygen2 8.x,
usethis::use_pipe()(and anyother usethis helper gated by
check_uses_roxygen()) aborts with "does not useroxygen2". The test used
use_pipe()only as scaffolding, to re-export themagrittr
%>%and givefind_missing_tags()an extra documented object.This is not an
audit_tags.Rregression. The audit pipeline reimplementsroxygenise()through roxygen2 internals; I verified independently that all 13of those internals (8 via
getFromNamespace, 5 exported helpers) still existand remain call-compatible in roxygen2 8.1.0, and that the
rdroclet'sroclet_preprocessis a no-op default (nordtoolscache dependency). Thefind_missing_tags()assertions all pass unchanged under roxygen2 8.1.0.Fix
tests/testthat/test-find_missing_values.R: drop theusethis::use_pipe()scaffolding. The
%>%reexport was never part of the asserted 12-functioncontract, and the fixture functions are only defined, never executed by the audit (which parses source rather than resolving
%>%at runtime), so noassertion changes. Modern packages use the native
|>, which needs no setup.This test fails without the change and passes with it under roxygen2 8.x.
tests/testthat/test-roxygen2-8x-compat.R(new): regression guard pinning(1) that
create_example_pkg()stays recognisable as roxygen-managed throughConfig/roxygen2/version, and (2) that the fullfind_missing_tags()auditflow and block classification keep working end-to-end under the installed
roxygen2.
tests/testthat/test-requirements.R: harden the toothless> "7.1.2"floor to>= "7.3.0", the boundary the suite actually branches on.Housekeeping bundled in
DESCRIPTION:Version: 1.0.1;Depends: R (>= 4.1)(roxygen2 8.0.0 requiresR >= 4.1, previously inconsistent at 4.0); migrate to
Config/roxygen2/version: 8.1.0.@importFromdirectives inNAMESPACE, refreshedman/*.Rd(maintainer now listed in Authors;find_missing_tagsinherits roxygen2 8.x's richerrocletsdocs).NEWS.md: new1.0.1section.Verification
R CMD check --as-cranwith roxygen2 8.1.0: 0 errors / 0 warnings / 0 notes.devtools::test()green under roxygen2 8.1.0.Relationship to #135
This branch is built on top of #135 ("Test upkeep" by @hadley, authorship
preserved as the first commit): without its per-test condition handling the
real roxygen2 8.x failure stays buried under global suppressions. Merging this
PR therefore includes and supersedes #135.
Follow-ups after the initial review round
72ab746): applied the confirmed findingsof a multi-agent, adversarially verified pre-CRAN audit. Highlights:
cran-comments.md rewritten for the 1.0.1 patch release;
roxygen2 (>= 7.3.0)floor declared in Imports; dead
LazyDataremoved; R >= 4.1 bump disclosedin NEWS under Breaking changes; three fast side-effect-free examples
unwrapped from
\dontrun{}withrequireNamespace()guards; acheck/directory leak from test-deprecated.R fixed; tempdir cleanup added to
test-use_data_doc.R (validated Copilot finding). Backward compatibility with
roxygen2 7.3.3 verified in an isolated library (35 assertions, 0 failures).
8227ba1): the first win-builder r-devel runfailed on the new regression test itself: it asserted that a package
scaffolded under roxygen2 >= 8.0.0 carries
Config/roxygen2/version, butthe usethis/attachment/roxygen2 combination on win-builder leaves the legacy
RoxygenNotefield instead (595 other assertions passed, the auditedbehaviour is fine there). The test now asserts the invariant checkhelper
actually relies on, the same one usethis uses: roxygen-managed means either
field is present. It still fails when neither field exists.
Verification (final head)
R CMD check --as-cranwith roxygen2 8.1.0: 0 errors / 0 warnings /0 notes (four runs across the branch history, including a fresh clone of the
pushed state).
Pre-submission control round
An independent control pass on
8227ba1(localR CMD check --as-cranruntwice, once under roxygen2 7.3.3 and once under roxygen2 8.1.0 in an isolated
library, plus a direct reproduction of the CRAN failure) confirmed the
diagnosis and the fix, and surfaced the corrections applied in
ad1d6a3.cran-comments.mdandNEWS.md. The failureis not Fedora specific. The CRAN check page reports ERROR on the four
r-devel Linux flavors (debian and fedora, clang and gcc), on
r-patched-linux-x86_64, onr-release-windows-x86_64and onr-oldrel-windows-x86_64, that is on every flavor that has already pickedup roxygen2 8.x. The
Test environmentssection now lists what was actuallyrun.
win-builder fix, the first assertion of
test-roxygen2-8x-compat.Rcould nolonger fail for the reason it exists: measured on both lines, the fixture
carries
Config/roxygen2/versionalone under 8.1.0 andRoxygenNotealoneunder 7.3.3, so
any(has_fields(...))holds either way and the guardverified nothing anywhere. A new test writes the post-8.x DESCRIPTION shape
by hand and asserts
find_missing_tags()still classifies the fixture.Config/testthat/parallel: truedropped. It came in with Test upkeep #135 and wasnot disclosed in the housekeeping list above. It buys nothing on CRAN, where
testthat caps at two workers, and adds a flakiness surface under an archival
deadline. Measured cost of running serially: 324s against 233s.
Config/testthat/edition: 3, also new with Test upkeep #135, is kept: theout <- expect_message(f())toexpect_message(out <- f())rewrites itrequires are correct and necessary, since edition 3 returns the condition
rather than the value.
create_example_pkg()'s own example is now guarded by the samerequireNamespace()pair as the three examples unwrapped from\dontrun{}.CONTRIBUTING.mdroxygen2 floor moved from the stale> 7.1.2to>= 7.3.0, matchingImportsandtest-requirements.R, with a note thatusethis helpers gated by
check_uses_roxygen()now abort on this repositorybecause its DESCRIPTION carries
Config/roxygen2/version..Rbuildignorenow drops*.stackdump. A Git Bash crash file landing atthe repository root is shipped by
R CMD buildand raises a "Non-standardfile/directory found at top level" NOTE. Caught empirically during the pass.
Independently verified and unchanged: the fourteen roxygen2 internals and
exports
audit_tags()reaches through all exist and stay call compatible in8.1.0, with
roclet_preprocess.defaulta genuine no-op;roxygenise()under8.1.0 produces a zero diff against the committed
man/,NAMESPACEandDESCRIPTION;use_pipe()is the onlycheck_uses_roxygen()gated usethishelper the suite ever reached, so removing it closes the root cause completely;
and the CI logs confirm roxygen2 8.1.0 was genuinely installed on the windows,
macos and ubuntu-devel jobs.
Verification (after the control round)
R CMD check --as-cranon a freshly built tarball, R 4.5.0 on Windows:Status: OKunder roxygen2 8.1.0 andStatus: OKunder roxygen2 7.3.3.both roxygen2 lines.