Skip to content

Commit c049b85

Browse files
ilonatommyilonatommy
authored
[wasm] Dispose Xunit ToolCommand (#108319)
* Make sure ToolCommand instances are disposed + do not log after the disposal. with: @pavelsavara
1 parent 98db53f commit c049b85

14 files changed

+107
-104
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 & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,15 @@ 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();
107-
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();
108107

109108
string razorProjectDir = Path.Combine(_projectDir!, "RazorClassLibrary");
110109
Directory.CreateDirectory(razorProjectDir);
111-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
112-
.WithWorkingDirectory(razorProjectDir)
110+
cmd.WithWorkingDirectory(razorProjectDir)
113111
.WithEnvironmentVariable("NUGET_PACKAGES", _nugetPackagesDir)
114112
.ExecuteWithCapturedOutput("new razorclasslib")
115113
.EnsureSuccessful();

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

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public void WriteLine(string message)
1818
baseOutput.WriteLine(message);
1919
_outputBuffer.AppendLine(message);
2020
if (EnvironmentVariables.ShowBuildOutput)
21-
Console.WriteLine(message);
21+
Console.WriteLine(message);
2222
}
2323

2424
public void WriteLine(string format, params object[] args)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace Wasm.Build.Tests
1313
{
1414
public class ToolCommand : IDisposable
1515
{
16+
private bool isDisposed = false;
1617
private string _label;
1718
protected ITestOutputHelper _testOutput;
1819

@@ -93,12 +94,15 @@ public virtual CommandResult ExecuteWithCapturedOutput(params string[] args)
9394

9495
public virtual void Dispose()
9596
{
97+
if (isDisposed)
98+
return;
9699
if (CurrentProcess is not null && !CurrentProcess.HasExited)
97100
{
98101
CurrentProcess.Kill(entireProcessTree: true);
99102
CurrentProcess.Dispose();
100103
CurrentProcess = null;
101104
}
105+
isDisposed = true;
102106
}
103107

104108
protected virtual string GetFullArgs(params string[] args) => string.Join(" ", args);
@@ -109,7 +113,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
109113
CurrentProcess = CreateProcess(executable, args);
110114
DataReceivedEventHandler errorHandler = (s, e) =>
111115
{
112-
if (e.Data == null)
116+
if (e.Data == null || isDisposed)
113117
return;
114118

115119
string msg = $"[{_label}] {e.Data}";
@@ -120,7 +124,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
120124

121125
DataReceivedEventHandler outputHandler = (s, e) =>
122126
{
123-
if (e.Data == null)
127+
if (e.Data == null || isDisposed)
124128
return;
125129

126130
string msg = $"[{_label}] {e.Data}";

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,20 @@ private void NonWasmConsoleBuild(string config,
108108
File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.props"), "<Project />");
109109
File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.targets"), directoryBuildTargets);
110110

111-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
112-
.WithWorkingDirectory(_projectDir!)
113-
.ExecuteWithCapturedOutput("new console --no-restore")
114-
.EnsureSuccessful();
111+
using DotNetCommand cmd = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false);
112+
cmd.WithWorkingDirectory(_projectDir!)
113+
.ExecuteWithCapturedOutput("new console --no-restore")
114+
.EnsureSuccessful();
115115

116-
new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
117-
.WithWorkingDirectory(_projectDir!)
118-
.ExecuteWithCapturedOutput($"build -restore -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")} {extraBuildArgs} -f {targetFramework}")
119-
.EnsureSuccessful();
116+
cmd.WithWorkingDirectory(_projectDir!)
117+
.ExecuteWithCapturedOutput($"build -restore -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")} {extraBuildArgs} -f {targetFramework}")
118+
.EnsureSuccessful();
120119

121120
if (shouldRun)
122121
{
123-
var result = new DotNetCommand(s_buildEnv, _testOutput, useDefaultArgs: false)
124-
.WithWorkingDirectory(_projectDir!)
125-
.ExecuteWithCapturedOutput($"run -c {config} -f {targetFramework} --no-build")
126-
.EnsureSuccessful();
122+
CommandResult result = cmd.WithWorkingDirectory(_projectDir!)
123+
.ExecuteWithCapturedOutput($"run -c {config} -f {targetFramework} --no-build")
124+
.EnsureSuccessful();
127125

128126
Assert.Contains("Hello, World!", result.Output);
129127
}

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