Skip to content

Commit 22cf0cd

Browse files
ilonatommymaraf
ilonatommy
and
maraf
authored
[release/9.0-staging] Backport "Dispose Xunit ToolCommand" (#116685)
* Backport "Dispose Xunit ToolCommand" * Feedback: add lock. * Update src/mono/wasm/Wasm.Build.Tests/Common/ToolCommand.cs Co-authored-by: Marek Fišera <[email protected]> --------- Co-authored-by: Marek Fišera <[email protected]>
1 parent b1ec39e commit 22cf0cd

File tree

12 files changed

+128
-113
lines changed

12 files changed

+128
-113
lines changed

‎src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmTestBase.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ public void InitBlazorWasmProjectDir(string id, string targetFramework = Default
4646
public string CreateBlazorWasmTemplateProject(string id)
4747
{
4848
InitBlazorWasmProjectDir(id);
49-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
50-
.WithWorkingDirectory(_projectDir!)
51-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
52-
.ExecuteWithCapturedOutput("new blazorwasm")
53-
.EnsureSuccessful();
49+
using DotNetCommand dotnetCommand = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false);
50+
CommandResult result = dotnetCommand.WithWorkingDirectory(_projectDir!)
51+
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
52+
.ExecuteWithCapturedOutput("new blazorwasm")
53+
.EnsureSuccessful();
5454

5555
return Path.Combine(_projectDir!, $"{id}.csproj");
5656
}
@@ -195,12 +195,12 @@ public async Task BlazorRunTest(string runArgs,
195195
runOptions.ServerEnvironment?.ToList().ForEach(
196196
kv => s_buildEnv.EnvVars[kv.Key] = kv.Value);
197197

198-
using var runCommand = new RunCommand(s_buildEnv, _testOutput)
199-
.WithWorkingDirectory(workingDirectory);
198+
using RunCommand runCommand = new RunCommand(s_buildEnv, _testOutput);
199+
ToolCommand cmd = runCommand.WithWorkingDirectory(workingDirectory);
200200

201201
await using var runner = new BrowserRunner(_testOutput);
202202
var page = await runner.RunAsync(
203-
runCommand,
203+
cmd,
204204
runArgs,
205205
onConsoleMessage: OnConsoleMessage,
206206
onServerMessage: runOptions.OnServerMessage,

‎src/mono/wasm/Wasm.Build.Tests/Blazor/CleanTests.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ public void Blazor_BuildThenClean_NativeRelinking(string config)
4040
Assert.True(Directory.Exists(relinkDir), $"Could not find expected relink dir: {relinkDir}");
4141

4242
string logPath = Path.Combine(s_buildEnv.LogRootPath, id, $"{id}-clean.binlog");
43-
new DotNetCommand(s_buildEnv, _testOutput)
44-
.WithWorkingDirectory(_projectDir!)
45-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
46-
.ExecuteWithCapturedOutput("build", "-t:Clean", $"-p:Configuration={config}", $"-bl:{logPath}")
47-
.EnsureSuccessful();
43+
using ToolCommand cmd = new DotNetCommand(s_buildEnv, _testOutput)
44+
.WithWorkingDirectory(_projectDir!);
45+
cmd.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
46+
.ExecuteWithCapturedOutput("build", "-t:Clean", $"-p:Configuration={config}", $"-bl:{logPath}")
47+
.EnsureSuccessful();
4848

4949
AssertEmptyOrNonExistentDirectory(relinkDir);
5050
}
@@ -88,9 +88,9 @@ private void Blazor_BuildNativeNonNative_ThenCleanTest(string config, bool first
8888
Assert.True(Directory.Exists(relinkDir), $"Could not find expected relink dir: {relinkDir}");
8989

9090
string logPath = Path.Combine(s_buildEnv.LogRootPath, id, $"{id}-clean.binlog");
91-
new DotNetCommand(s_buildEnv, _testOutput)
92-
.WithWorkingDirectory(_projectDir!)
93-
.WithEnvironmentVariable("NUGET_PACKAGES", _projectDir!)
91+
using ToolCommand cmd = new DotNetCommand(s_buildEnv, _testOutput)
92+
.WithWorkingDirectory(_projectDir!);
93+
cmd.WithEnvironmentVariable("NUGET_PACKAGES", _projectDir!)
9494
.ExecuteWithCapturedOutput("build", "-t:Clean", $"-p:Configuration={config}", $"-bl:{logPath}")
9595
.EnsureSuccessful();
9696

‎src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests2.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ private CommandResult PublishForRequiresWorkloadTest(string config, string extra
5959
extraItems: extraItems);
6060

6161
string publishLogPath = Path.Combine(s_buildEnv.LogRootPath, id, $"{id}.binlog");
62-
return new DotNetCommand(s_buildEnv, _testOutput)
63-
.WithWorkingDirectory(_projectDir!)
64-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
65-
.ExecuteWithCapturedOutput("publish",
66-
$"-bl:{publishLogPath}",
67-
$"-p:Configuration={config}");
62+
using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput);
63+
return cmd.WithWorkingDirectory(_projectDir!)
64+
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
65+
.ExecuteWithCapturedOutput("publish",
66+
$"-bl:{publishLogPath}",
67+
$"-p:Configuration={config}");
6868
}
6969
}

‎src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests3.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,16 @@ public void BugRegression_60479_WithRazorClassLib()
9999
string wasmProjectDir = Path.Combine(_projectDir!, "wasm");
100100
string wasmProjectFile = Path.Combine(wasmProjectDir, "wasm.csproj");
101101
Directory.CreateDirectory(wasmProjectDir);
102-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
103-
.WithWorkingDirectory(wasmProjectDir)
104-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
105-
.ExecuteWithCapturedOutput("new blazorwasm")
106-
.EnsureSuccessful();
102+
using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false);
103+
cmd.WithWorkingDirectory(wasmProjectDir)
104+
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
105+
.ExecuteWithCapturedOutput("new blazorwasm")
106+
.EnsureSuccessful();
107107

108108

109109
string razorProjectDir = Path.Combine(_projectDir!, "RazorClassLibrary");
110110
Directory.CreateDirectory(razorProjectDir);
111-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
112-
.WithWorkingDirectory(razorProjectDir)
111+
cmd.WithWorkingDirectory(razorProjectDir)
113112
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
114113
.ExecuteWithCapturedOutput("new razorclasslib")
115114
.EnsureSuccessful();

‎src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ public BuildTestBase(ProjectProviderBase providerBase, ITestOutputHelper output,
164164
if (buildProjectOptions.Publish && buildProjectOptions.BuildOnlyAfterPublish)
165165
commandLineArgs.Append("-p:WasmBuildOnlyAfterPublish=true");
166166

167-
var cmd = new DotNetCommand(s_buildEnv, _testOutput)
168-
.WithWorkingDirectory(_projectDir!)
169-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
170-
.WithEnvironmentVariables(buildProjectOptions.ExtraBuildEnvironmentVariables);
167+
using ToolCommand cmd = new DotNetCommand(s_buildEnv, _testOutput)
168+
.WithWorkingDirectory(_projectDir!);
169+
cmd.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
170+
.WithEnvironmentVariables(buildProjectOptions.ExtraBuildEnvironmentVariables);
171171
if (UseWBTOverridePackTargets && s_buildEnv.IsWorkload)
172172
cmd.WithEnvironmentVariable("WBTOverrideRuntimePack", "true");
173173

‎src/mono/wasm/Wasm.Build.Tests/Common/ToolCommand.cs‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ namespace Wasm.Build.Tests
1313
{
1414
public class ToolCommand : IDisposable
1515
{
16+
private bool isDisposed = false;
17+
private readonly object _lock = new object();
1618
private string _label;
1719
protected ITestOutputHelper _testOutput;
1820

@@ -93,11 +95,17 @@ public virtual CommandResult ExecuteWithCapturedOutput(params string[] args)
9395

9496
public virtual void Dispose()
9597
{
96-
if (CurrentProcess is not null && !CurrentProcess.HasExited)
98+
lock (_lock)
9799
{
98-
CurrentProcess.Kill(entireProcessTree: true);
99-
CurrentProcess.Dispose();
100-
CurrentProcess = null;
100+
if (isDisposed)
101+
return;
102+
if (CurrentProcess is not null && !CurrentProcess.HasExited)
103+
{
104+
CurrentProcess.Kill(entireProcessTree: true);
105+
CurrentProcess.Dispose();
106+
CurrentProcess = null;
107+
}
108+
isDisposed = true;
101109
}
102110
}
103111

@@ -109,24 +117,33 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
109117
CurrentProcess = CreateProcess(executable, args);
110118
DataReceivedEventHandler errorHandler = (s, e) =>
111119
{
112-
if (e.Data == null)
120+
if (e.Data == null || isDisposed)
113121
return;
114122

115-
string msg = $"[{_label}] {e.Data}";
116-
output.Add(msg);
117-
_testOutput.WriteLine(msg);
118-
ErrorDataReceived?.Invoke(s, e);
123+
lock (_lock)
124+
{
125+
if (e.Data == null || isDisposed)
126+
return;
127+
128+
string msg = $"[{_label}] {e.Data}";
129+
output.Add(msg);
130+
_testOutput.WriteLine(msg);
131+
ErrorDataReceived?.Invoke(s, e);
132+
}
119133
};
120134

121135
DataReceivedEventHandler outputHandler = (s, e) =>
122136
{
123-
if (e.Data == null)
124-
return;
125-
126-
string msg = $"[{_label}] {e.Data}";
127-
output.Add(msg);
128-
_testOutput.WriteLine(msg);
129-
OutputDataReceived?.Invoke(s, e);
137+
lock (_lock)
138+
{
139+
if (e.Data == null || isDisposed)
140+
return;
141+
142+
string msg = $"[{_label}] {e.Data}";
143+
output.Add(msg);
144+
_testOutput.WriteLine(msg);
145+
OutputDataReceived?.Invoke(s, e);
146+
}
130147
};
131148

132149
CurrentProcess.ErrorDataReceived += errorHandler;

‎src/mono/wasm/Wasm.Build.Tests/NonWasmTemplateBuildTests.cs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,22 +112,18 @@ private void NonWasmConsoleBuild(string config,
112112
File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.props"), "<Project />");
113113
File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.targets"), directoryBuildTargets);
114114

115-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
116-
.WithWorkingDirectory(_projectDir!)
117-
.ExecuteWithCapturedOutput("new console --no-restore")
118-
.EnsureSuccessful();
115+
using ToolCommand cmd = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
116+
.WithWorkingDirectory(_projectDir!);
117+
cmd.ExecuteWithCapturedOutput("new console --no-restore")
118+
.EnsureSuccessful();
119119

120-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
121-
.WithWorkingDirectory(_projectDir!)
122-
.ExecuteWithCapturedOutput($"build -restore -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")} {extraBuildArgs} -f {targetFramework}")
123-
.EnsureSuccessful();
120+
cmd.ExecuteWithCapturedOutput($"build -restore -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")} {extraBuildArgs} -f {targetFramework}")
121+
.EnsureSuccessful();
124122

125123
if (shouldRun)
126124
{
127-
var result = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
128-
.WithWorkingDirectory(_projectDir!)
129-
.ExecuteWithCapturedOutput($"run -c {config} -f {targetFramework} --no-build")
130-
.EnsureSuccessful();
125+
CommandResult result = cmd.ExecuteWithCapturedOutput($"run -c {config} -f {targetFramework} --no-build")
126+
.EnsureSuccessful();
131127

132128
Assert.Contains("Hello, World!", result.Output);
133129
}

‎src/mono/wasm/Wasm.Build.Tests/Templates/NativeBuildTests.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ public void BuildWithUndefinedNativeSymbol(bool allowUndefined)
4545
File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), code);
4646
File.Copy(Path.Combine(BuildEnvironment.TestAssetsPath, "native-libs", "undefined-symbol.c"), Path.Combine(_projectDir!, "undefined_xyz.c"));
4747

48-
CommandResult result = new DotNetCommand(s_buildEnv, _testOutput)
49-
.WithWorkingDirectory(_projectDir!)
50-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
51-
.ExecuteWithCapturedOutput("build", "-c Release");
48+
using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput);
49+
CommandResult result = cmd.WithWorkingDirectory(_projectDir!)
50+
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
51+
.ExecuteWithCapturedOutput("build", "-c Release");
5252

5353
if (allowUndefined)
5454
{
@@ -83,17 +83,17 @@ public void ProjectWithDllImportsRequiringMarshalIlGen_ArrayTypeParameter(string
8383
Path.Combine(_projectDir!, "Program.cs"),
8484
overwrite: true);
8585

86-
CommandResult result = new DotNetCommand(s_buildEnv, _testOutput)
87-
.WithWorkingDirectory(_projectDir!)
88-
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
89-
.ExecuteWithCapturedOutput("build", $"-c {config} -bl");
86+
using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput);
87+
CommandResult result = cmd.WithWorkingDirectory(_projectDir!)
88+
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
89+
.ExecuteWithCapturedOutput("build", $"-c {config} -bl");
9090

9191
Assert.True(result.ExitCode == 0, "Expected build to succeed");
9292

93-
CommandResult res = new RunCommand(s_buildEnv, _testOutput)
94-
.WithWorkingDirectory(_projectDir!)
95-
.ExecuteWithCapturedOutput($"run --no-silent --no-build -c {config}")
96-
.EnsureSuccessful();
93+
using RunCommand runCmd = new RunCommand(s_buildEnv, _testOutput);
94+
CommandResult res = runCmd.WithWorkingDirectory(_projectDir!)
95+
.ExecuteWithCapturedOutput($"run --no-silent --no-build -c {config}")
96+
.EnsureSuccessful();
9797
Assert.Contains("Hello, Console!", res.Output);
9898
Assert.Contains("Hello, World! Greetings from node version", res.Output);
9999
}

0 commit comments

Comments
 (0)