Skip to content

test(renv): assert lockfile package names instead of grepping raw JSON#137

Merged
VincentGuyader merged 1 commit into
mainfrom
fix-renv-tests-grep-false-positive
May 1, 2026
Merged

test(renv): assert lockfile package names instead of grepping raw JSON#137
VincentGuyader merged 1 commit into
mainfrom
fix-renv-tests-grep-false-positive

Conversation

@VincentGuyader
Copy link
Copy Markdown
Member

Problem

Three tests in tests/testthat/test-renv_create.R (suggested package are not in renv prod, lines 316/416/467 on main) grep the lockfile JSON as a flat string:

base <- paste(readLines(out_renv_file), collapse = " ")
expect_false(grepl(pattern = "withr", x = base))

This matches any textual mention of withr in the file — including a Suggests: field inside another package's metadata. After remotes 2.5.0 shipped with withr listed in its Suggests, every prod-lockfile run pulls in remotes (it's used by attachment::set_remotes), and the grep finds:

"remotes": {
  ...
  "Suggests": [ "...", "withr" ]
}

…which is a metadata mention, not a top-level package of the lockfile.

skip_on_cran() hides the failures from R CMD check, so CI stayed green; the fails only surface when running devtools::test() locally with NOT_CRAN=true.

Fix

Switch to parsing the lockfile via renv:::lockfile() and asserting membership in names(...$data()$Packages). This is the same idiom already used at test-renv_create.R:128-143 for the dev/prod lockfile checks higher up in the file — just applied consistently to the three lower test_that blocks too.

Three expect_false(grepl(...)) blocks become three expect_false("X" %in% pkg_in_lock) blocks. No semantic change in what the tests intend to assert; only the implementation is hardened against textual false positives.

Coverage

  • test-renv_create.R:316suggested package are not in renv prod
  • test-renv_create.R:416suggested package are not in renv prod even from vignettes (with document = FALSE)
  • test-renv_create.R:467suggested package are not in renv prod even from vignettes (with document = TRUE)

A fourth grepl in the same file (line 371, on pkg_local_renv which is already a names(...) vector) is correct and untouched.

Test plan

  • Pre-fix on baseline: [ FAIL 3 | WARN 0 | SKIP 1 | PASS 63 ] for test_file("test-renv_create.R")
  • Post-fix: [ FAIL 0 | WARN 0 | SKIP 1 | PASS 61 ]
  • devtools::test() full suite: [ FAIL 0 | WARN 0 | SKIP 1 | PASS 392 ]
  • R CMD check unaffected (file is skip_on_cran())

Independent of #136 — both branches diverge from main and don't touch each other's files.

The three `suggested package are not in renv prod ...` tests grepped the
lockfile JSON as a flat string, so they matched any textual mention of
a package name — including Suggests fields inside another package's
metadata. After remotes 2.5.0 added `withr` to its Suggests, the tests
became false positives outside R CMD check (`skip_on_cran()` hides them
from CI).

Switch to parsing the lockfile via `renv:::lockfile()` and asserting
membership in `names(...$data()$Packages)`, mirroring the pattern
already used at line 274 of the same file.
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 PR hardens renv lockfile assertions in the test suite by switching from raw JSON string greps to structured lockfile parsing, eliminating false positives caused by package metadata (e.g., Suggests fields).

Changes:

  • Parse renv.lock via getFromNamespace("lockfile", "renv")(... )$data()$Packages in three tests.
  • Replace grepl() checks on collapsed JSON text with %in% checks against top-level lockfile package names.
  • Add clarifying test comments explaining the false-positive scenario being avoided.

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

@VincentGuyader VincentGuyader merged commit 8f084cd into main May 1, 2026
13 checks passed
@VincentGuyader VincentGuyader deleted the fix-renv-tests-grep-false-positive branch May 1, 2026 14:57
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.

2 participants