Skip to content

snapshot_accept() / snapshot_review() hint includes wrong path when a test changes the working directory #2335

@cderv

Description

@cderv

When a test changes the working directory (e.g. via withr::local_dir()) and expect_snapshot() then fails, the rendered hint passes the in-test working directory as the path argument to snapshot_accept() / snapshot_review(). That path is unrelated to where the snapshot was actually written, so the suggested command is not runnable.

The snapshot file itself is written to the correct location under tests/testthat/_snaps/ — only the hint text is affected.

Reproduction

I can reproduce this on testthat 3.3.2 and on main (4b13712). The script below builds a throwaway package in tempdir(), runs the snapshot test twice, and prints the hint:

library(testthat)

pkg <- file.path(tempdir(), "snaprepro")
unlink(pkg, recursive = TRUE)
usethis::create_package(pkg, open = FALSE, rstudio = FALSE)
usethis::use_testthat(3)
dir.create(file.path(pkg, "tests", "testthat"), recursive = TRUE)

test_file <- file.path(pkg, "tests", "testthat", "test-leak.R")
write_test <- function(value) {
  writeLines(
    c(
      'test_that("hint includes wd set inside test", {',
      "  local_edition(3)",
      "  tmp <- withr::local_tempdir()",
      "  withr::local_dir(tmp)",
      sprintf('  expect_snapshot(cat("%s"))', value),
      "})"
    ),
    test_file
  )
}

# 1. Create baseline snapshot
write_test("hello")
testthat::test_dir(file.path(pkg, "tests", "testthat"))

# 2. Change the snapshotted value, run again -> hint shows temp dir as `path`
write_test("world")
testthat::test_dir(file.path(pkg, "tests", "testthat"))

Output of the second test_dir():

FAILURE: 'test-leak.R:5:3' ------------------------
Snapshot of code has changed:
    old            | new
[1] Code           | Code           [1]
[2]   cat("hello") -   cat("world") [2]
[3] Output         | Output         [3]
[4]   hello        -   world        [4]
* Run `testthat::snapshot_accept("leak", "C:\\Users\\chris\\AppData\\Local\\Temp\\RtmpSK3o36\\filea24028be7513")` to accept the change.
* Run `testthat::snapshot_review("leak", "C:\\Users\\chris\\AppData\\Local\\Temp\\RtmpSK3o36\\filea24028be7513")` to review the change.

The second argument is the temp directory created by withr::local_tempdir() inside the test, not <pkg>/tests/testthat. Removing the withr::local_dir(tmp) line produces a clean hint (testthat::snapshot_accept("leak")) on the same code path.

The new .new.md is written to <pkg>/tests/testthat/_snaps/leak.new.md regardless — the path leak is only in the printed hint.

Technical context

I had a quick look at the code, and my guess at where this comes from — please correct me if I'm reading it wrong.

snapshot_hint_path() looks like it builds the second argument of the hint by comparing Sys.getenv("TESTTHAT_WD") against getwd() at the moment the hint is rendered:

testthat/R/snapshot.R

Lines 431 to 450 in ace29c5

snapshot_hint_path <- function() {
wd <- Sys.getenv("TESTTHAT_WD", unset = "")
if (wd == "") {
return()
}
test_path <- file.path(wd, "tests/testthat")
if (test_path == getwd()) {
return()
}
old <- normalizePath(wd)
new <- normalizePath(getwd())
if (startsWith(new, old)) {
substr(new, nchar(old) + 2, nchar(new))
} else {
new
}
}

TESTTHAT_WD appears to be set once in local_test_directory(), capturing the working directory at the start of the test run:

testthat/R/local.R

Lines 176 to 191 in ace29c5

local_test_directory <- function(path, package = NULL, .env = parent.frame()) {
# Set edition before changing working directory in case path is relative
local_edition(find_edition(path, package), .env = .env)
# Capture current working directory so we can use for relative paths
wd <- getwd()
withr::local_dir(path, .local_envir = .env)
withr::local_envvar(
R_TESTS = "",
TESTTHAT = "true",
TESTTHAT_PKG = package,
TESTTHAT_WD = wd,
.local_envir = .env
)
}

If that's right, the hint is built during expectation failure, which happens inside test_that(), so any withr::local_dir() or setwd() from the test would still be in scope when getwd() is called — and the returned value would be whatever the test set rather than the testthat directory. That would match the symptom above, but I may be missing something.

For context, the hint was added in #2191 (fixes #1577) to help when test_dir() is invoked from outside the package directory.

Session info

testthat 3.3.2 (CRAN) and main @ 4b13712bb08bbcf19e3b5265e95218043c3d528e
R 4.5.x, Windows 11

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions