Skip to content

Commit 8811159

Browse files
CopilotJanProvaznik
Copilot
and
JanProvaznik
committed
Update ThreadSafeTaskRegistry documentation and enhance testing strategy with integration tests
Co-authored-by: JanProvaznik <[email protected]>
1 parent 5faddcc commit 8811159

File tree

2 files changed

+97
-35
lines changed

2 files changed

+97
-35
lines changed

‎src/Build.UnitTests/BackEnd/ThreadSafeTaskRegistry_Tests.cs

Lines changed: 91 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.IO;
67
using Microsoft.Build.BackEnd;
78
using Microsoft.Build.Execution;
89
using Microsoft.Build.Framework;
@@ -60,85 +61,141 @@ public void NullAndEmptyTaskNamesReturnFalse()
6061

6162
/// <summary>
6263
/// Tests for task routing functionality in multi-threaded execution mode.
64+
/// These tests verify both the routing logic and actual task execution behavior.
6365
/// </summary>
64-
public class TaskRouting_Tests
66+
public class TaskRouting_Tests : IDisposable
6567
{
68+
private TestEnvironment _env;
69+
70+
public TaskRouting_Tests()
71+
{
72+
_env = TestEnvironment.Create();
73+
}
74+
75+
public void Dispose()
76+
{
77+
_env?.Dispose();
78+
}
79+
6680
/// <summary>
67-
/// Verify that when multi-threaded execution is disabled, routing uses default behavior.
81+
/// Verify that thread-safe tasks (like Message) execute in-process when multi-threaded execution is enabled.
6882
/// </summary>
6983
[Fact]
70-
public void RoutingUsesDefaultBehaviorWhenMultiThreadedExecutionDisabled()
84+
public void ThreadSafeMessageTaskExecutesInProcess()
7185
{
7286
var buildParameters = new BuildParameters();
73-
buildParameters.MultiThreaded = false;
87+
buildParameters.MultiThreaded = true;
7488
buildParameters.IsOutOfProc = false;
7589

7690
var componentHost = new MockHost(buildParameters);
7791
var requestEntry = CreateMockBuildRequestEntry();
7892
var taskHost = new TaskHost(componentHost, requestEntry, new MockElementLocation("test.proj"), null);
7993

80-
// Should use default behavior (false in this case)
94+
// Message task should execute in-process (return false for out-of-process)
8195
taskHost.ShouldTaskExecuteOutOfProc("Message").ShouldBeFalse();
82-
taskHost.ShouldTaskExecuteOutOfProc("UnknownTask").ShouldBeFalse();
8396
}
8497

8598
/// <summary>
86-
/// Verify that when already out-of-process, tasks always execute out-of-process.
99+
/// Verify that non-thread-safe tasks (like Copy) execute out-of-process when multi-threaded execution is enabled.
87100
/// </summary>
88101
[Fact]
89-
public void TasksExecuteOutOfProcessWhenAlreadyOutOfProcess()
102+
public void NonThreadSafeCopyTaskExecutesOutOfProcess()
90103
{
91104
var buildParameters = new BuildParameters();
92105
buildParameters.MultiThreaded = true;
93-
buildParameters.IsOutOfProc = true;
106+
buildParameters.IsOutOfProc = false;
94107

95108
var componentHost = new MockHost(buildParameters);
96109
var requestEntry = CreateMockBuildRequestEntry();
97110
var taskHost = new TaskHost(componentHost, requestEntry, new MockElementLocation("test.proj"), null);
98111

99-
// Should always be out-of-process when already out-of-process
100-
taskHost.ShouldTaskExecuteOutOfProc("Message").ShouldBeTrue();
101-
taskHost.ShouldTaskExecuteOutOfProc("UnknownTask").ShouldBeTrue();
112+
// Copy task should execute out-of-process (return true for out-of-process)
113+
taskHost.ShouldTaskExecuteOutOfProc("Copy").ShouldBeTrue();
102114
}
103115

104116
/// <summary>
105-
/// Verify that thread-safe tasks execute in-process when multi-threaded execution is enabled.
117+
/// Integration test that verifies Message task routing behavior using actual MSBuild execution.
106118
/// </summary>
107119
[Fact]
108-
public void ThreadSafeTasksExecuteInProcessWhenMultiThreadedExecutionEnabled()
120+
public void MessageTaskRoutingIntegrationTest()
109121
{
110-
var buildParameters = new BuildParameters();
111-
buildParameters.MultiThreaded = true;
112-
buildParameters.IsOutOfProc = false;
113-
114-
var componentHost = new MockHost(buildParameters);
115-
var requestEntry = CreateMockBuildRequestEntry();
116-
var taskHost = new TaskHost(componentHost, requestEntry, new MockElementLocation("test.proj"), null);
117-
118-
// Thread-safe tasks should execute in-process (return false for out-of-process)
119-
taskHost.ShouldTaskExecuteOutOfProc("Message").ShouldBeFalse();
120-
taskHost.ShouldTaskExecuteOutOfProc("Warning").ShouldBeFalse();
121-
taskHost.ShouldTaskExecuteOutOfProc("Error").ShouldBeFalse();
122+
using var buildManager = new BuildManager();
123+
var projectContent = @"
124+
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
125+
<Target Name='TestTarget'>
126+
<Message Text='This is a test message' Importance='high' />
127+
</Target>
128+
</Project>";
129+
130+
var projectFile = _env.CreateFile("test.proj", projectContent).Path;
131+
132+
var parameters = new BuildParameters()
133+
{
134+
MultiThreaded = true
135+
};
136+
137+
var request = new BuildRequestData(projectFile, new Dictionary<string, string>(), null, new[] { "TestTarget" }, null);
138+
var result = buildManager.Build(parameters, request);
139+
140+
// Verify the build succeeded - this confirms the Message task was executed
141+
result.OverallResult.ShouldBe(BuildResultCode.Success);
122142
}
123143

124144
/// <summary>
125-
/// Verify that non-thread-safe tasks execute out-of-process when multi-threaded execution is enabled.
145+
/// Integration test that verifies Copy task routing behavior using actual MSBuild execution.
126146
/// </summary>
127147
[Fact]
128-
public void NonThreadSafeTasksExecuteOutOfProcessWhenMultiThreadedExecutionEnabled()
148+
public void CopyTaskRoutingIntegrationTest()
149+
{
150+
using var buildManager = new BuildManager();
151+
152+
var sourceFile = _env.CreateFile("source.txt", "test content").Path;
153+
var targetFile = Path.Combine(_env.DefaultTestDirectory.Path, "target.txt");
154+
155+
var projectContent = $@"
156+
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
157+
<Target Name='TestTarget'>
158+
<Copy SourceFiles='{sourceFile}' DestinationFiles='{targetFile}' />
159+
</Target>
160+
</Project>";
161+
162+
var projectFile = _env.CreateFile("test.proj", projectContent).Path;
163+
164+
var parameters = new BuildParameters()
165+
{
166+
MultiThreaded = true
167+
};
168+
169+
var request = new BuildRequestData(projectFile, new Dictionary<string, string>(), null, new[] { "TestTarget" }, null);
170+
var result = buildManager.Build(parameters, request);
171+
172+
// Verify the build succeeded - this confirms the Copy task was executed
173+
result.OverallResult.ShouldBe(BuildResultCode.Success);
174+
// Verify the file was actually copied
175+
File.Exists(targetFile).ShouldBeTrue();
176+
}
177+
178+
/// <summary>
179+
/// Verify routing logic works correctly for various scenarios.
180+
/// </summary>
181+
[Theory]
182+
[InlineData(false, false, "Message", false)] // MultiThreaded=false -> default behavior
183+
[InlineData(false, false, "Copy", false)] // MultiThreaded=false -> default behavior
184+
[InlineData(true, true, "Message", true)] // Already out-of-proc -> stays out-of-proc
185+
[InlineData(true, true, "Copy", true)] // Already out-of-proc -> stays out-of-proc
186+
[InlineData(true, false, "Message", false)] // Thread-safe task -> in-process
187+
[InlineData(true, false, "Copy", true)] // Non-thread-safe task -> out-of-process
188+
public void TaskRoutingDecisionLogic(bool multiThreaded, bool isOutOfProc, string taskName, bool expectedOutOfProc)
129189
{
130190
var buildParameters = new BuildParameters();
131-
buildParameters.MultiThreaded = true;
132-
buildParameters.IsOutOfProc = false;
191+
buildParameters.MultiThreaded = multiThreaded;
192+
buildParameters.IsOutOfProc = isOutOfProc;
133193

134194
var componentHost = new MockHost(buildParameters);
135195
var requestEntry = CreateMockBuildRequestEntry();
136196
var taskHost = new TaskHost(componentHost, requestEntry, new MockElementLocation("test.proj"), null);
137197

138-
// Non-thread-safe tasks should execute out-of-process (return true for out-of-process)
139-
taskHost.ShouldTaskExecuteOutOfProc("UnknownTask").ShouldBeTrue();
140-
taskHost.ShouldTaskExecuteOutOfProc("CustomTask").ShouldBeTrue();
141-
taskHost.ShouldTaskExecuteOutOfProc("ThirdPartyTask").ShouldBeTrue();
198+
taskHost.ShouldTaskExecuteOutOfProc(taskName).ShouldBe(expectedOutOfProc);
142199
}
143200

144201
/// <summary>

‎src/Build/BackEnd/Components/RequestBuilder/ThreadSafeTaskRegistry.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@
1010
namespace Microsoft.Build.BackEnd
1111
{
1212
/// <summary>
13-
/// Registry that maintains an allowlist of tasks that are known to be thread-safe
13+
/// Temporary registry that maintains an allowlist of tasks that are known to be thread-safe
1414
/// and can be executed within thread nodes in multi-threaded execution mode.
15+
///
16+
/// This is an interim solution until we adopt a proper thread-safe task interface.
17+
/// Tasks listed here have been manually verified to be thread-safe in their current
18+
/// implementation. Once a formal thread-safe task interface is established, this
19+
/// allowlist approach will be replaced with interface-based detection.
1520
/// </summary>
1621
internal static class ThreadSafeTaskRegistry
1722
{

0 commit comments

Comments
 (0)