Visitar URL original
Enable class-level parallelism in Process tests (#135160) · dotnet/runtime@780c55a · GitHub
Skip to content

Commit 780c55a

Browse files
steveisokCopilot
andauthored
Enable class-level parallelism in Process tests (#135160)
## Motivation Mitigate #135001's Process-suite execution-budget exhaustion by isolating ambient-state-sensitive tests and restoring default xUnit class-level parallelism on Windows/Linux. This follows the [review suggestion](#135160 (comment)) and replaces the previous sharding approach without rewriting history. **Desktop macOS retains the original assembly-wide serialization.** Recent Mono x64 and Checked CoreCLR arm64 runs timed out under class parallelism. This is a compatibility safeguard against the unresolved concurrency problem, not a root-cause fix or a claim that macOS CI is now green. ## Changes - Run the three parent-environment tests wholly inside `RemoteExecutor`, preserving nested child inheritance, overrides, null removal, and fixture disposal. Process exit handles environment cleanup. - Run the Windows encoding test inside a remote process that detaches from the inherited console and allocates a private console. Preserve native/code-page checks and the Unix/container UTF-8 path; process exit frees the private console. - Isolate the process-wide first-chance-exception counter with `RemoteExecutor`. - Use a unique directory and remote working directory for the Unix executable-resolution test. Its standard `RemoteExecutor.IsSupported` condition intentionally excludes that one outer-loop test when RemoteExecutor is unsupported; the bespoke single-file fallback stays removed. - Define `TARGET_OSX` only for `TargetOS=osx` with the Unix target framework, then apply the original `CollectionPerAssembly` attribute. This uses the actual test build target, not the build host, and applies to both Mono and CoreCLR. Windows/Linux keep class-level collections; Android, MacCatalyst, and Browser behavior is unchanged. The Browser assembly skip is preserved. The existing single project and packaging remain. No sharding, thread caps, timeout increases, product-code changes, or PR-specific CI gates. The macOS safeguard changes scheduling only, not test eligibility. ## Validation Supplemental compilation and actual xUnit discovery passed for eight target configurations using cached CI dependencies. The macOS Mono x64, CoreCLR arm64, and single-file source configurations each place all 765 locally discovered rows in **one shared collection**. Windows/Linux retain class collections; Android/MacCatalyst/Browser remain class-based. Cross-target evaluation on Windows verified that the gate follows `TargetOS`, and an `osx` build with a MacCatalyst target framework does not enable it. Local execution of two inexpensive tests from different classes confirmed one executed collection for the macOS-targeted assembly versus two for Windows/Linux. These are cached-runtime metadata/smoke checks, **not actual macOS or NativeAOT execution**; collection membership restores serialization even when the runner reports multiple available threads. The prior cleanup revision passed all four affected Windows tests and ten concurrent parent-state probe calls with zero violations across 3,747 samples. Supported discovery/traits remained 724 Windows rows and 765 Unix rows, with 689 ordinary Windows cases; the only unsupported-runner eligibility change was the accepted Unix outer-loop condition. Earlier local serial/parallel full-suite times were 763.201s and 311.975s on the same saved x86 Checked runtime. Both had the same job-breakaway access-denied failure. These measurements are indicative, not controlled performance proof or a promised CI speedup. The earlier [Windows Server 2016 experiment](https://helix.dot.net/Job/e2b3d201-2701-4bfa-98b4-c796b562836b), on test source `a083867ca24`, ran 691 cases: 687 passed, one job-breakaway failure, three skips; 518.706 test seconds and 586.995 full work-item seconds. All five isolation cases passed with four-thread class parallelism. It used the intact historical x86 Checked/Debug-libraries runtime from [build 1621730](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1621730), source `dda144147efa0a3669f11b283253da3ddf0bb9c1`, not a current-head runtime build. Its reporting script also encountered an XML naming collision. It was **not a green run**. ## Remaining limits The local repository build is blocked by the missing shared-framework targeting pack; actual NativeAOT evaluation is blocked by missing ILCompiler targets. Supplemental checks do not establish a successful repository, bundled single-file, or native-platform build. The macOS concurrency problem and Windows job-breakaway failure remain unresolved. This revision restores the old macOS scheduling only; passing macOS Mono/CoreCLR execution still needs confirmation from the new automatic CI run. No manual jobs, new baselines, reruns, or monitoring were started. > [!NOTE] > This pull request description and changes were generated with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 44b18162-ee5a-4b48-8ce9-96ac2bb8dc5d
1 parent 2846af4 commit 780c55a

7 files changed

Lines changed: 93 additions & 83 deletions

File tree

‎src/libraries/System.Diagnostics.Process/tests/AssemblyInfo.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
using Xunit;
55

6-
// Process tests can conflict with each other, as they modify ambient state
7-
// like the console code page and environment variables
6+
#if TARGET_OSX
7+
// Retain serialization until process-management hangs under concurrent test classes on macOS are resolved.
8+
// https://github.com/dotnet/runtime/issues/135294
89
[assembly: CollectionBehavior(CollectionBehavior.CollectionPerAssembly)]
10+
#endif
911

1012
[assembly: SkipOnPlatform(TestPlatforms.Browser, "System.Diagnostics.Process is not supported on Browser.")]

‎src/libraries/System.Diagnostics.Process/tests/Interop.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,18 @@ public struct SID_AND_ATTRIBUTES
9898
[DllImport("kernel32.dll")]
9999
internal static extern int GetConsoleOutputCP();
100100

101-
[DllImport("kernel32.dll")]
101+
[DllImport("kernel32.dll", SetLastError = true)]
102102
internal static extern int SetConsoleCP(int codePage);
103103

104-
[DllImport("kernel32.dll")]
104+
[DllImport("kernel32.dll", SetLastError = true)]
105105
internal static extern int SetConsoleOutputCP(int codePage);
106106

107+
[DllImport("kernel32.dll", SetLastError = true)]
108+
internal static extern int FreeConsole();
109+
110+
[DllImport("kernel32.dll", SetLastError = true)]
111+
internal static extern int AllocConsole();
112+
107113
[DllImport("advapi32.dll")]
108114
internal static extern bool OpenProcessToken(SafeProcessHandle ProcessHandle, uint DesiredAccess, out SafeProcessHandle TokenHandle);
109115

‎src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.ComponentModel;
45
using System.IO;
56
using System.IO.Pipes;
67
using System.Text;
@@ -18,22 +19,6 @@ public void TestChangesInConsoleEncoding()
1819
{
1920
const int ConsoleEncoding = 437;
2021

21-
void RunWithExpectedCodePage(int expectedCodePage)
22-
{
23-
Process p = CreateProcessLong();
24-
p.StartInfo.RedirectStandardInput = true;
25-
p.StartInfo.RedirectStandardOutput = true;
26-
p.StartInfo.RedirectStandardError = true;
27-
p.Start();
28-
29-
Assert.Equal(expectedCodePage, p.StandardInput.Encoding.CodePage);
30-
Assert.Equal(expectedCodePage, p.StandardOutput.CurrentEncoding.CodePage);
31-
Assert.Equal(expectedCodePage, p.StandardError.CurrentEncoding.CodePage);
32-
33-
p.Kill();
34-
Assert.True(p.WaitForExit(WaitInMS));
35-
};
36-
3722
// Don't test this on Windows containers, as there is a known issue.
3823
// See https://github.com/dotnet/runtime/issues/42000 for more details.
3924
if (!OperatingSystem.IsWindows() || PlatformDetection.IsInContainer)
@@ -42,21 +27,41 @@ void RunWithExpectedCodePage(int expectedCodePage)
4227
return;
4328
}
4429

45-
int inputEncoding = Interop.GetConsoleCP();
46-
int outputEncoding = Interop.GetConsoleOutputCP();
47-
48-
try
30+
RemoteExecutor.Invoke(static () =>
4931
{
50-
Interop.SetConsoleCP(ConsoleEncoding);
51-
Interop.SetConsoleOutputCP(ConsoleEncoding);
32+
// Remote processes inherit the runner's console; allocate a private one before changing its code pages.
33+
if (Interop.FreeConsole() == 0)
34+
{
35+
throw new Win32Exception();
36+
}
37+
if (Interop.AllocConsole() == 0)
38+
{
39+
throw new Win32Exception();
40+
}
41+
if (Interop.SetConsoleCP(ConsoleEncoding) == 0 || Interop.SetConsoleOutputCP(ConsoleEncoding) == 0)
42+
{
43+
throw new Win32Exception();
44+
}
5245

53-
RunWithExpectedCodePage(ConsoleEncoding);
54-
}
55-
finally
56-
{
57-
Interop.SetConsoleCP(inputEncoding);
58-
Interop.SetConsoleOutputCP(outputEncoding);
59-
}
46+
using var tests = new ProcessStandardConsoleTests();
47+
tests.RunWithExpectedCodePage(ConsoleEncoding);
48+
}).Dispose();
49+
}
50+
51+
private void RunWithExpectedCodePage(int expectedCodePage)
52+
{
53+
Process p = CreateProcessLong();
54+
p.StartInfo.RedirectStandardInput = true;
55+
p.StartInfo.RedirectStandardOutput = true;
56+
p.StartInfo.RedirectStandardError = true;
57+
p.Start();
58+
59+
Assert.Equal(expectedCodePage, p.StandardInput.Encoding.CodePage);
60+
Assert.Equal(expectedCodePage, p.StandardOutput.CurrentEncoding.CodePage);
61+
Assert.Equal(expectedCodePage, p.StandardError.CurrentEncoding.CodePage);
62+
63+
p.Kill();
64+
Assert.True(p.WaitForExit(WaitInMS));
6065
}
6166

6267
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]

‎src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs‎

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -235,35 +235,41 @@ public void EnvironmentVariableContainingNull_ThrowsArgumentException()
235235
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
236236
public void TestSetEnvironmentOnChildProcess()
237237
{
238-
const string name = "b5a715d3-d74f-465d-abb7-2abe844750c9";
239-
Environment.SetEnvironmentVariable(name, "parent-process-value");
240-
241-
Process p = CreateProcess(() =>
238+
RemoteExecutor.Invoke(static () =>
242239
{
243-
if (Environment.GetEnvironmentVariable(name) != "child-process-value")
244-
return 1;
240+
using var tests = new ProcessStartInfoTests();
241+
const string name = "b5a715d3-d74f-465d-abb7-2abe844750c9";
242+
Environment.SetEnvironmentVariable(name, "parent-process-value");
245243

246-
return RemoteExecutor.SuccessExitCode;
247-
});
248-
p.StartInfo.Environment.Add(name, "child-process-value");
249-
p.Start();
244+
Process p = tests.CreateProcess(() =>
245+
{
246+
if (Environment.GetEnvironmentVariable(name) != "child-process-value")
247+
return 1;
250248

251-
Assert.True(p.WaitForExit(WaitInMS));
252-
Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode);
249+
return RemoteExecutor.SuccessExitCode;
250+
});
251+
p.StartInfo.Environment.Add(name, "child-process-value");
252+
p.Start();
253+
254+
Assert.True(p.WaitForExit(WaitInMS));
255+
Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode);
256+
}).Dispose();
253257
}
254258

255259
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
256260
public void TestEnvironmentOfChildProcess()
257261
{
258-
const string ExtraEnvVar = "TestEnvironmentOfChildProcess_SpecialStuff";
259-
Environment.SetEnvironmentVariable(ExtraEnvVar, "\x1234" + Environment.NewLine + "\x5678"); // ensure some Unicode characters and newlines are in the output
260-
const string EmptyEnvVar = "TestEnvironmentOfChildProcess_Empty";
261-
Environment.SetEnvironmentVariable(EmptyEnvVar, "");
262-
try
262+
RemoteExecutor.Invoke(static () =>
263263
{
264+
using var tests = new ProcessStartInfoTests();
265+
const string ExtraEnvVar = "TestEnvironmentOfChildProcess_SpecialStuff";
266+
Environment.SetEnvironmentVariable(ExtraEnvVar, "\x1234" + Environment.NewLine + "\x5678"); // ensure some Unicode characters and newlines are in the output
267+
const string EmptyEnvVar = "TestEnvironmentOfChildProcess_Empty";
268+
Environment.SetEnvironmentVariable(EmptyEnvVar, "");
269+
264270
// Schedule a process to see what env vars it gets. Have it write out those variables
265271
// to its output stream so we can read them.
266-
Process p = CreateProcess(() =>
272+
Process p = tests.CreateProcess(() =>
267273
{
268274
Console.Write(string.Join(ItemSeparator, Environment.GetEnvironmentVariables().Cast<DictionaryEntry>().Select(e => Convert.ToBase64String(Encoding.UTF8.GetBytes(e.Key + "=" + e.Value)))));
269275
return RemoteExecutor.SuccessExitCode;
@@ -294,22 +300,19 @@ public void TestEnvironmentOfChildProcess()
294300
string.Join(", ", envEnv.Except(actualEnv)),
295301
Environment.NewLine,
296302
string.Join(", ", actualEnv.Except(envEnv))));
297-
}
298-
finally
299-
{
300-
Environment.SetEnvironmentVariable(ExtraEnvVar, null);
301-
Environment.SetEnvironmentVariable(EmptyEnvVar, null);
302-
}
303+
}).Dispose();
303304
}
304305

305306
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
306307
public void EnvironmentNullValue()
307308
{
308-
const string NullEnvVar = "TestEnvironmentOfChildProcess_Null";
309-
Environment.SetEnvironmentVariable(NullEnvVar, "");
310-
try
309+
RemoteExecutor.Invoke(static () =>
311310
{
312-
Process p = CreateProcess(() =>
311+
using var tests = new ProcessStartInfoTests();
312+
const string NullEnvVar = "TestEnvironmentOfChildProcess_Null";
313+
Environment.SetEnvironmentVariable(NullEnvVar, "");
314+
315+
Process p = tests.CreateProcess(() =>
313316
{
314317
// Verify that setting the value to null in StartInfo is going to remove the process environment.
315318
Assert.Null(Environment.GetEnvironmentVariable(NullEnvVar));
@@ -319,11 +322,7 @@ public void EnvironmentNullValue()
319322
Assert.Null(p.StartInfo.Environment[NullEnvVar]);
320323
p.Start();
321324
Assert.True(p.WaitForExit(WaitInMS));
322-
}
323-
finally
324-
{
325-
Environment.SetEnvironmentVariable(NullEnvVar, null);
326-
}
325+
}).Dispose();
327326
}
328327

329328
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]

‎src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,35 +93,27 @@ public void ProcessStart_UseShellExecute_OnUnix_ThrowsWhenNoOpenerOnPath()
9393
}, options).Dispose();
9494
}
9595

96-
[Fact]
96+
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
9797
[OuterLoop("Opens program")]
9898
[SkipOnPlatform(TestPlatforms.MacCatalyst, "In App Sandbox mode, the process doesn't have read access to the binary.")]
9999
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.Android | TestPlatforms.Browser, "Not supported on iOS/tvOS/Android/Browser.")]
100100
public void ProcessStart_DirectoryNameInCurDirectorySameAsFileNameInExecDirectory_Success()
101101
{
102-
string fileToOpen = "dotnet";
103-
string curDir = Environment.CurrentDirectory;
104-
string dotnetFolder = Path.Combine(Path.GetTempPath(),"dotnet");
105-
bool shouldDelete = !Directory.Exists(dotnetFolder);
106-
try
102+
Directory.CreateDirectory(Path.Combine(TestDirectory, "dotnet"));
103+
104+
RemoteExecutor.Invoke(StartDotnet, new RemoteInvokeOptions
107105
{
108-
Directory.SetCurrentDirectory(Path.GetTempPath());
109-
Directory.CreateDirectory(dotnetFolder);
106+
StartInfo = new ProcessStartInfo { WorkingDirectory = TestDirectory }
107+
}).Dispose();
110108

109+
static void StartDotnet()
110+
{
111+
string fileToOpen = "dotnet";
111112
using (var px = Process.Start(fileToOpen))
112113
{
113114
Assert.NotNull(px);
114115
}
115116
}
116-
finally
117-
{
118-
if (shouldDelete)
119-
{
120-
Directory.Delete(dotnetFolder);
121-
}
122-
123-
Directory.SetCurrentDirectory(curDir);
124-
}
125117
}
126118

127119
[Fact]

‎src/libraries/System.Diagnostics.Process/tests/ProcessTests.Windows.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ private static unsafe void ReEnableCtrlCHandlerIfNeeded(PosixSignal signal)
123123

124124
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
125125
public void Kill_EntireProcessTree_MinimalExceptions()
126+
{
127+
RemoteExecutor.Invoke(KillEntireProcessTreeMinimalExceptions).Dispose();
128+
}
129+
130+
private static void KillEntireProcessTreeMinimalExceptions()
126131
{
127132
// This test validates that Kill(true) doesn't throw excessive exceptions internally
128133
// during process enumeration, which causes severe performance degradation with debugger attached.

‎src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
<PropertyGroup>
1111
<TargetPlatformIdentifier>$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)'))</TargetPlatformIdentifier>
1212
<DefineConstants Condition="'$(TargetPlatformIdentifier)' == 'windows'">$(DefineConstants);TargetsWindows</DefineConstants>
13+
<DefineConstants Condition="'$(TargetOS)' == 'osx' and '$(TargetPlatformIdentifier)' == 'unix'">$(DefineConstants);TARGET_OSX</DefineConstants>
1314
</PropertyGroup>
1415
<ItemGroup>
1516
<Compile Include="$(CommonPath)System\IO\StringParser.cs"

0 commit comments

Comments
 (0)