[wasm] Dispose Xunit ToolCommand#108319
Conversation
|
Tagging subscribers to this area: @directhex, @matouskozak |
|
Instead of rerunning the pipeline on this PR, I would prefer to merge it asap and see how the report of known build issue changes. The change is harmless, even if it won't be the correct fix. |
pavelsavara
changed the title
WriteLine methods in try catch.
|
do we have the same problems in |
No, wasi was never hit in total of ~60 hits till now. |
|
/ba-g the failing test is timeout with no log, typical for CI builds on windows. We cannot catch it with active issue |
| .WithWorkingDirectory(_projectDir!) | ||
| .ExecuteWithCapturedOutput($"build -restore -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")} {extraBuildArgs} -f {targetFramework}") | ||
| .EnsureSuccessful(); | ||
| cmd.WithWorkingDirectory(_projectDir!) |
There was a problem hiding this comment.
Overriding working dir is void if we reuse the instance (which is by the way interesting idea!).
Does it have any side effects? The Dispose of ToolCommand disposes the CurrentProcess. Which happens only when the process didn't exit, which can't happen here I guess...
There was a problem hiding this comment.
True, thanks for pointing out. Follow up PR it is.
One side effect: runs will share the ITestOutputHelper instance, so when we do result.Output we will see output out of both runs. In this case it does not matter, first we only build, then only run. But in tests where we build twice but with different params and the check the output for Assert.Contains, it could be a problem.
* Make sure ToolCommand instances are disposed + do not log after the disposal. with: @pavelsavara
3 participants
Trying to fix:
#105315
Following the stack from the linked issue, we have:
runtime/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs
Line 548 in 84b7c2f
https://github.com/xunit/xunit/blob/6bfa188331f6d4ed6e877ab6da5e7b2725ccd721/src/xunit.v3.core/Framework/TestOutputHelper.cs#L45
We might be trying to
WriteLine, after xunit threw an exception and the test state is alreadynull. Do not try to log the messages (both error and info) if the command is already disposed.