testthat::set_max_fails() not working anymore

[EmilHvitfeldt]

Positron Version:

Positron Version: 2024.04.0 (Universal) build 29
Code - OSS Version: 1.87.0
Commit: 6c41ddd
Date: 2024-04-04T03:32:13.676Z
Electron: 27.3.2
Chromium: 118.0.5993.159
Node.js: 18.17.1
V8: 11.8.172.18-electron.0
OS: Darwin arm64 23.4.0

Steps to reproduce the issue:

1. run testthat::set_max_fails(1000)
2. run test R package command
3. have many failures

Kapture.2024-04-10.at.09.58.06.mp4

What did you expect to happen?

I feel like this used to work (although, didn't check)

Were there any error messages in the output or Developer Tools console?

Nope

[DavisVaughan]

That just sets the TESTTHAT_MAX_FAILS env var so we are likely not inheriting session env vars in the subprocess

[jennybc]

@EmilHvitfeldt since you seem to have a good test case of massive test failure at hand 😄, what happens in RStudio?

[DavisVaughan]

RStudio's build pane inherits the env vars of the R process, so it "just works" and will chug through all the tests because it inherits the new max failure limit of 1k

[lionel-]

Two ways we could go about this:

• Expose a .ps.set_env() function that testthat and others would use to manipulate envvars in positron
• Inspect the environment of the R session and reproduce it when launching subprocesses

The second approach has the downside that it might make it a bit harder to create reproducible behaviour in subprocesses. Did that come up as an issue in RStudio?

[DavisVaughan]

We probably want variants for "clean" R subprocesses and "make it look like the current session" R subprocesses

[lionel-]

That would be nice but it's not clear which one we should choose in this case. We want a clean process to run tests but also propagate user configuration.

Maybe the solution in this case is to call set_max_fail() in Rprofile though.

[jennybc]

We have encountered this type of problem before. It's why I ask the running session for its locale-related info and then pass LANG along to the test explorer's child process here in #2488. I guess it's the downside (outweighed by lots of upsides) of having these "side" processes encapsulated from the main R session.

Another alternative is for the env vars to transmit to be task-specific, by using the options argument in these ShellExecution calls:

positron/extensions/positron-r/src/tasks.ts

Lines 58 to 61 in e790eef

	new vscode.ShellExecution(
	binpath,
	['-e', { value: data.rcode, quoting: vscode.ShellQuoting.Strong }]
	),

Maybe our taskData entries should optionally include env vars to transmit. And for r.task.packageTest this would include testthat usual suspects, such as TESTTHAT_MAX_FAILS.

[juliasilge]

I've got QA notes available here: #4606 (comment)

[testlabauto]

Verified Fixed

Positron Version(s) : 2024.09.0-33
OS Version          : OSX

Test scenario(s)

Hit:
Maximum number of failures exceeded; quitting at end of file.
After setting:
testthat::set_max_fails(1)

Link(s) to TestRail test cases run or created:
N/A