Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
katrinabrock opened this issue Aug 28, 2024 · 4 comments

Comments

@katrinabrock
Copy link

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):

  • OS: ...
  • R Version: ...
  • vscDebugger Version: ...
  • vscode-r-debugger Version: ...

Additional context
Add any other context about the problem here.

@katrinabrock
Copy link
Author

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' 

@katrinabrock katrinabrock changed the title .vsc.cat is incompatible with testthat::expect_output .vsc.cat is incompatible with testthat::expect_output and tinytest::expect_stdout Aug 28, 2024
@ManuelHentschel
Copy link
Owner

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
Copy link
Owner

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 added a commit to katrinabrock/minimal-r-package that referenced this issue Sep 6, 2024
@katrinabrock
Copy link
Author

@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:

  • It may be nice to call out that this config is useful when working with testing frameworks or other tools that deal with stderr/stdout.
  • In the doc about the config, specify that the default is false.

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

No branches or pull requests

2 participants