VSCode-R-Debugger icon indicating copy to clipboard operation
VSCode-R-Debugger copied to clipboard

Breakpoints not working with `devtools::test` + `testthat`

Open katrinabrock opened this issue 1 year ago • 7 comments

NOTE: Before submitting an issue, please make sure to install the latest version of both the vscode extension and the R package. This can usually be achieved by running the command r.debugger.updateRPackage in vscode's command palette (F1).

Describe the bug devtools::test() doesn't trigger breakpoints in testing file or package.

MRE

  1. Open this repo as an r project with vsc r debugger enabled: https://github.com/katrinabrock/minimal-r-package
  2. Open tests/testthat/test-output.R).
  3. Add some breakpoints.
  4. Run the Debug testthat test config. (This runs devtools::test(filter='output')) image

Your Launch config If applicable, the launch config that causes the bug. E.g.:

https://github.com/katrinabrock/minimal-r-package/blob/main/.vscode/launch.json#L15-L23

Expected behavior Dropped into debug console at breakpoints.

Actual behavior Tests run without stopping at breakpoints.

Desktop (please complete the following information):

  • OS: Desktop is Mac OS, using VS Code Remote..remote (where R is running) is Ubuntu
  • R Version: 4.3.2
  • VSCode version: 1.92.2
  • vscDebugger Version: 0.5.3
  • vscode-r-debugger Version: v0.5.4

Additional context

  • Breakpoints in the code that calls devtools::test (in this case example2.R): working ✅
  • Breakpoints in the test files themselves: not working ❌
  • Breakpoints in package functions called within the tests: not working: ❌
  • Direct browser calls: working everywhere ✅ I believe this is failing because testthat runs pkgload::load_all(): https://github.com/r-lib/testthat/blob/fe38519d72247a8907228a4ecaf926483aa5d4ff/R/test-files.R#L235 and that call is not successfully getting overwritten with vscDebugger::.vsc.load_all

katrinabrock avatar Aug 27 '24 13:08 katrinabrock

thanks. I didn't click the link. :-)

katrinabrock avatar Aug 27 '24 18:08 katrinabrock

Thanks for raising this. Your assumption about pkdload::load_all() sounds reasonable, but I'm not sure if there is an easy way to fix this. I'll have a look, though.

ManuelHentschel avatar Sep 02 '24 15:09 ManuelHentschel

Update: The breakpoints that you indicated in the picture above (i.e. in tests/testthat/...) would be quite hard to implement, because they are neither in the package, nor in the normal R code. Instead they are loaded and executed by testthat in a rather complicated fashion, which would be difficult to modify reliably.

One way around this is to define the test-code in a separate function, which is part of the main script, and then call that function from your test file:

# [In main file (example2.R)]
f <- function() {
    cat('Alice!\n')    # <- Breakpoint here
}

# [In tests/testthat/test-output.R]
test_that('Output Test #3', {
  f()
  expect_true(TRUE)
})

In order to set breakpoints in package code, adding the following sinippet at the beginning of your example2.R file and removing "debuggedPackages": [ "temp" ] from the launch config seems to yield the desired behavior. In more complex scenarios, e.g. involving multiple packages, this might lead to undesired behavior, though.

env <- loadNamespace('pkgload')
unlockBinding('load_all', env)
env$load_all <- function(...) cat('Ignoring pkgload::load_all!\n')
lockBinding('load_all', env)

ManuelHentschel avatar Sep 05 '24 20:09 ManuelHentschel

@ManuelHentschel

Thanks for the response!

I am hesitant to move any of the testing logic out of the test file and into example2.R because ultimately example2.R is a short-lived launcher script. However, I could move the logic to a helper file, and source the helper file in main/launcher script (example2.R).

The second option of putting the breakpoints in the package did work. I've updated my example package to demonstrate this (https://github.com/katrinabrock/minimal-r-package/commit/1dd6f51bd749127fd33b9650e8f8cbb1224c3823). With a breakpoint like this: image

I still think there may be a cleaner way to achieve this...like some magical combination of Config/testthat/load-all: list(...) in the DESCRIPTION file, but I haven't found it yet. :-/

katrinabrock avatar Sep 06 '24 08:09 katrinabrock

I am hesitant to move any of the testing logic out of the test file and into example2.R because ultimately example2.R is a short-lived launcher script. However, I could move the logic to a helper file, and source the helper file in main/launcher script (example2.R).

Breakpoints in helper files seem to work if you prevent source_test_helpers from reloading the file and thus overwriting the breakpoints:

env <- loadNamespace('testthat')
unlockBinding('source_test_helpers', env)
env$source_test_helpers <- function(...) cat('Ignoring testthat::source_test_helpers!\n')
lockBinding('source_test_helpers', env)

However, if I understood correctly, the code from test-XXX.R scripts is loaded here. Modifying that would get quite hacky, so I don't see an easy way to have breakpoints in these files for now.

ManuelHentschel avatar Sep 08 '24 09:09 ManuelHentschel

Getting breakpoints working the package and helpers is already a big help. Thanks :-)

katrinabrock avatar Sep 09 '24 13:09 katrinabrock

btw - I believe there is a similar issue (and work around) with tinytest.

However, instead of using a load_all like testthat, tinytest seems to use parse + eval: https://github.com/markvanderloo/tinytest/blob/master/pkg/R/tinytest.R#L671-L689 It seems like something like .vsc.parse would be needed to fix this.

I did some experimenting, and similar to testthat, breakpoints in testfiles themselves don't work, but breakpoints in helper files do work. In tinytest, these are explicitly sourced in the testfile.

Here is an MRE of this issue: https://github.com/katrinabrock/minimal-r-package/blob/main/example6.R . You can add breakpoints to example6.R#L7 and test_example6.R#L1 as shown below to reproduce:

image

katrinabrock avatar Jan 07 '25 17:01 katrinabrock