ManuelHentschel / vscDebugger

(Partial) Implementation of the Debug Adapter Protocol for R
https://manuelhentschel.github.io/vscDebugger/
MIT License
88 stars 14 forks source link

`.vsc.cat` is incompatible with `testthat::expect_output` and `tinytest::expect_stdout` #196

Open katrinabrock opened 2 months ago

katrinabrock commented 2 months ago

Describe the bug .vsc.cat does not emit output in such a way that testthat::expect_output observes it. Impact: when running tests in debug mode, some tests erroneously fail.

To Reproduce Run these two lines:

testthat::expect_output(cat('hello world'), 'hello world')

testthat::expect_output(vscDebugger::.vsc.cat('hello world'), 'hello world')

Your Launch config N/A This can be reproduced with R alone.

Expected behavior Above two lines to produce the same result (no failure, no output).

Actual behavior First line with base::cat produces no output (expectation is met). Second line with vscDebugger::.vsc.cat raises an error (expectation not met).

> testthat::expect_output(cat('hello world'), 'hello world')
> testthat::expect_output(vscDebugger::.vsc.cat('hello world'), 'hello world')
Error: `vscDebugger::.vsc.cat("hello world")` produced no output
7: stop(exp)
6: doWithOneRestart(return(expr), restart)
5: withOneRestart(expr, restarts[[1L]])
4: withRestarts(if (expectation_broken(exp)) {
       stop(exp)
   } else {
       signalCondition(exp)
   }, continue_test = function(e) NULL)
3: exp_signal(exp)
2: expect(!identical(act$cap, ""), sprintf("%s produced no output", 
       act$lab), info = info)
1: testthat::expect_output(vscDebugger::.vsc.cat("hello world"), 
       "hello world")

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

katrinabrock commented 2 months ago

Same problem with tinytest:

> tinytest::expect_stdout(cat('hello world'), 'hello world')
----- PASSED      : <-->
 call| tinytest::expect_stdout(cat("hello world"), "hello world") 
> tinytest::expect_stdout(vscDebugger::.vsc.cat('hello world'), 'hello world')
----- FAILED[xcpt]: <-->
 call| tinytest::expect_stdout(vscDebugger::.vsc.cat("hello world"), 
 call| "hello world")
 diff| output ''
 diff| does not match pattern 'hello world' 
ManuelHentschel commented 2 months ago

Thanks for opening this issue. This is the expected behavior of .vsc.cat. The function does not write to the normal stdout channel, but instead sends a message to the debug-frontend, including information about the location of the print statement in the code. Doing both of this at the same time, would make every output appear twice, which is not desired, I guess.

You can stop the debugger from overwriting cat (and print, str) using the debug config entries described here. This will fix your issue, but remove the source info in normal usage, which you can resolve by using different debug configurations for each.

Hope this helps!

ManuelHentschel commented 1 month ago

Update: I just realized the debug config entry "splitOverwrittenOutput": true does pretty much exactly what you need. Currently it will give a warning Property splitOverwrittenOutput is not allowed., but this can be ignored and the behavior will change anyways. I'll update the documentation accordingly.

katrinabrock commented 1 month ago

@ManuelHentschel Thanks! Indeed, in my example repo this setting worked perfectly to eliminate erroneous test failures. I will try it out in my main project(s) and report back if I hit any other issues. Feel free to close this issue.

Other doc changes that may be helpful: