Skip to content

Commit 02cebe5

Browse files
engyebrahimEvangelink
engyebrahim
and
Evangelink
authored
Disable appdomain usage by default (#1555)
Co-authored-by: Amaury Levé <[email protected]> fix #1297
1 parent 1db6567 commit 02cebe5

File tree

9 files changed

+234
-23
lines changed

9 files changed

+234
-23
lines changed

‎src/Adapter/MSTest.TestAdapter/ObjectModel/TestMethod.cs‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.ObjectModel;
77
using System.Diagnostics.CodeAnalysis;
88
using System.Reflection;
9+
using System.Text;
910

1011
using Microsoft.TestPlatform.AdapterUtilities;
1112
using Microsoft.TestPlatform.AdapterUtilities.ManagedNameUtilities;
@@ -173,10 +174,42 @@ public string? DeclaringClassFullName
173174
/// </summary>
174175
internal string? DisplayName { get; set; }
175176

176-
internal string UniqueName
177-
=> HasManagedMethodAndTypeProperties
178-
? $"{ManagedTypeName}.{ManagedMethodName}->{string.Join(", ", SerializedData ?? Array.Empty<string>())}"
179-
: $"{FullClassName}.{Name}->{string.Join(", ", SerializedData ?? Array.Empty<string>())}";
177+
public string UniqueName
178+
{
179+
get
180+
{
181+
var uniqueNameBuilder = new StringBuilder();
182+
183+
if (HasManagedMethodAndTypeProperties)
184+
{
185+
uniqueNameBuilder.Append(ManagedTypeName);
186+
uniqueNameBuilder.Append('.');
187+
uniqueNameBuilder.Append(ManagedMethodName);
188+
}
189+
else
190+
{
191+
uniqueNameBuilder.Append(FullClassName);
192+
uniqueNameBuilder.Append('.');
193+
uniqueNameBuilder.Append(Name);
194+
}
195+
196+
if (SerializedData is not null)
197+
{
198+
uniqueNameBuilder.Append("->");
199+
for (int i = 0; i < SerializedData.Length; i++)
200+
{
201+
if (i > 0)
202+
{
203+
uniqueNameBuilder.Append(", ");
204+
}
205+
206+
uniqueNameBuilder.Append(SerializedData[i]);
207+
}
208+
}
209+
210+
return uniqueNameBuilder.ToString();
211+
}
212+
}
180213

181214
internal TestMethod Clone() => (TestMethod)MemberwiseClone();
182215
}

‎src/Adapter/MSTest.TestAdapter/PublicAPI/PublicAPI.Unshipped.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
#nullable enable
2-
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.TreatDiscoveryWarningsAsErrors.get -> bool
2+
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.MSTestSettings.TreatDiscoveryWarningsAsErrors.get -> bool
3+
Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.TestMethod.UniqueName.get -> string!

‎src/Adapter/MSTestAdapter.PlatformServices/Services/MSTestAdapterSettings.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,21 @@ public static MSTestAdapterSettings ToSettings(XmlReader reader)
129129

130130
public static bool IsAppDomainCreationDisabled(string? settingsXml)
131131
{
132+
// By default appDomain is disabled.
132133
if (StringEx.IsNullOrEmpty(settingsXml))
133134
{
134-
return false;
135+
return true;
135136
}
136137

137138
StringReader stringReader = new(settingsXml);
138139
XmlReader reader = XmlReader.Create(stringReader, XmlRunSettingsUtilities.ReaderSettings);
139140

140-
return reader.ReadToFollowing("DisableAppDomain")
141-
&& bool.TryParse(reader.ReadInnerXml(), out var disableAppDomain)
142-
&& disableAppDomain;
141+
if (reader.ReadToFollowing("DisableAppDomain") && bool.TryParse(reader.ReadInnerXml(), out var disableAppDomain))
142+
{
143+
return disableAppDomain;
144+
}
145+
146+
return true;
143147
}
144148

145149
/// <summary>

‎src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ internal TestSourceHost(string sourceFileName, IRunSettings? runSettings, IFrame
8383
SetContext(sourceFileName);
8484

8585
// Set isAppDomainCreationDisabled flag
86-
_isAppDomainCreationDisabled = _runSettings != null && MSTestAdapterSettings.IsAppDomainCreationDisabled(_runSettings.SettingsXml);
86+
_isAppDomainCreationDisabled = MSTestAdapterSettings.IsAppDomainCreationDisabled(_runSettings?.SettingsXml);
8787
}
8888

8989
/// <summary>

‎test/IntegrationTests/MSTest.IntegrationTests/OutputTests.cs‎

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

44
using System;
55
using System.Collections.Generic;
6+
using System.Data;
67
using System.Globalization;
78
using System.Linq;
9+
using System.Reflection;
810

911
using FluentAssertions;
1012

@@ -16,17 +18,58 @@ public class OutputTests : CLITestBase
1618
{
1719
private const string TestAssetName = "OutputTestProject";
1820

21+
private const string RunSettingXml =
22+
@"<RunSettings>
23+
<RunConfiguration>
24+
<DisableAppDomain>False</DisableAppDomain>
25+
</RunConfiguration>
26+
</RunSettings>";
27+
1928
public void OutputIsNotMixedWhenTestsRunInParallel()
2029
{
21-
ValidateOutputForClass("UnitTest1");
30+
ValidateOutputForClass("UnitTest1", RunSettingXml);
2231
}
2332

2433
public void OutputIsNotMixedWhenAsyncTestsRunInParallel()
2534
{
26-
ValidateOutputForClass("UnitTest2");
35+
ValidateOutputForClass("UnitTest2", RunSettingXml);
36+
}
37+
38+
public void TestContextWriteLine_WhenTestsRunInParallel_OutputIsNotMixed()
39+
{
40+
// By default no appDomain.
41+
ValidateTestContextOutputForClass("UnitTest3");
42+
}
43+
44+
private void ValidateTestContextOutputForClass(string className)
45+
{
46+
// Arrange
47+
var assemblyPath = GetAssetFullPath(TestAssetName);
48+
49+
// Act
50+
var testCases = DiscoverTests(assemblyPath, null, RunSettingXml).Where(tc => tc.FullyQualifiedName.Contains(className)).ToList();
51+
testCases.Should().HaveCount(4);
52+
testCases.Should().NotContainNulls();
53+
54+
var testResults = RunTests(testCases);
55+
testResults.Should().HaveCount(4);
56+
testResults.Should().NotContainNulls();
57+
58+
// Assert
59+
// Ensure that some tests are running in parallel, because otherwise the output just works correctly.
60+
var firstEnd = testResults.Min(t => t.EndTime);
61+
var someStartedBeforeFirstEnded = testResults.Where(t => t.EndTime != firstEnd).Any(t => firstEnd > t.StartTime);
62+
someStartedBeforeFirstEnded.Should().BeTrue("Tests must run in parallel, but there were no other tests that started, before the first one ended.");
63+
64+
ValidateOutputIsNotMixed(testResults, "TestMethod2(DataRowValue1)", new[] { "TestMethod1", "TestMethod3" }, IsStandardOutputMessage, "TestMethod2 (DataRowValue1)");
65+
ValidateOutputIsNotMixed(testResults, "TestMethod2(DataRowValue2)", new[] { "TestMethod1", "TestMethod3" }, IsStandardOutputMessage, "TestMethod2 (DataRowValue2)");
66+
ValidateOutputIsNotMixed(testResults, "TestMethod1", new[] { "TestMethod2(DataRowValue1)", "TestMethod2(DataRowValue2)", "TestMethod3" }, IsStandardOutputMessage);
67+
ValidateOutputIsNotMixed(testResults, "TestMethod3", new[] { "TestMethod1", "TestMethod2(DataRowValue1)", "TestMethod2(DataRowValue2)", }, IsStandardOutputMessage);
68+
69+
ValidateInitializeAndCleanup(testResults, IsStandardOutputMessage);
2770
}
2871

29-
private void ValidateOutputForClass(string className)
72+
private void ValidateOutputForClass(string className, string runSettingXml = "")
3073
{
3174
// LogMessageListener uses an implementation of a string writer that captures output per async context.
3275
// This allows us to capture output from tasks even when they are running in parallel.
@@ -35,7 +78,7 @@ private void ValidateOutputForClass(string className)
3578
var assemblyPath = GetAssetFullPath(TestAssetName);
3679

3780
// Act
38-
var testCases = DiscoverTests(assemblyPath).Where(tc => tc.FullyQualifiedName.Contains(className)).ToList();
81+
var testCases = DiscoverTests(assemblyPath, null, runSettingXml).Where(tc => tc.FullyQualifiedName.Contains(className)).ToList();
3982
testCases.Should().HaveCount(3);
4083
testCases.Should().NotContainNulls();
4184

@@ -75,10 +118,10 @@ private static void ValidateInitializationsAndCleanups(IEnumerable<TestResult> t
75118
ValidateInitializeAndCleanup(testResults, IsDebugMessage);
76119
}
77120

78-
private static void ValidateOutputIsNotMixed(IEnumerable<TestResult> testResults, string methodName, string[] shouldNotContain, Func<TestResultMessage, bool> messageFilter)
121+
private static void ValidateOutputIsNotMixed(IEnumerable<TestResult> testResults, string methodName, string[] shouldNotContain, Func<TestResultMessage, bool> messageFilter, string dataRowDisplayName = null)
79122
{
80123
// Make sure that the output between methods is not mixed. And that every method has test initialize and cleanup.
81-
var testMethod = testResults.Single(t => t.DisplayName == methodName);
124+
var testMethod = testResults.Single(t => t.DisplayName == (dataRowDisplayName ?? methodName));
82125

83126
// Test method {methodName} was not found.
84127
testMethod.Should().NotBeNull();

‎test/IntegrationTests/MSTest.IntegrationTests/Utilities/CLITestBase.discovery.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@
2424
namespace Microsoft.MSTestV2.CLIAutomation;
2525
public partial class CLITestBase : TestContainer
2626
{
27-
internal ImmutableArray<TestCase> DiscoverTests(string assemblyPath, string testCaseFilter = null)
27+
internal ImmutableArray<TestCase> DiscoverTests(string assemblyPath, string testCaseFilter = null, string settingsXml = "")
2828
{
2929
var unitTestDiscoverer = new UnitTestDiscoverer();
3030
var logger = new InternalLogger();
3131
var sink = new InternalSink();
3232

33-
string runSettingXml = GetRunSettingXml(string.Empty);
33+
string runSettingXml = GetRunSettingXml(settingsXml);
3434
var context = new InternalDiscoveryContext(runSettingXml, testCaseFilter);
3535

3636
unitTestDiscoverer.DiscoverTestsInSource(assemblyPath, logger, sink, context);

‎test/IntegrationTests/PlatformServices.Desktop.IntegrationTests/DesktopTestSourceHostTests.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,13 @@ public void ChildDomainResolutionPathsShouldHaveSearchDirectoriesSpecifiedInRuns
8484
public void DisposeShouldUnloadChildAppDomain()
8585
{
8686
var testSource = GetTestAssemblyPath("DesktopTestProjectx86Debug");
87-
_testSourceHost = new TestSourceHost(testSource, null, null);
87+
string runSettingXml =
88+
$@"<RunSettings>
89+
<RunConfiguration>
90+
<DisableAppDomain>False</DisableAppDomain>
91+
</RunConfiguration>
92+
</RunSettings>";
93+
_testSourceHost = new TestSourceHost(testSource, GetMockedIRunSettings(runSettingXml).Object, null);
8894
_testSourceHost.SetupHost();
8995

9096
// Check that child appdomain was indeed created
@@ -95,6 +101,19 @@ public void DisposeShouldUnloadChildAppDomain()
95101
_testSourceHost.AppDomain.Should().BeNull();
96102
}
97103

104+
public void RunSettings_WithoutSetDisableAppDomain_WillNotCreatAppDomain()
105+
{
106+
// Arrange
107+
var testSource = GetTestAssemblyPath("DesktopTestProjectx86Debug");
108+
_testSourceHost = new TestSourceHost(testSource, null, null);
109+
110+
// AcT
111+
_testSourceHost.SetupHost();
112+
113+
// Assert
114+
_testSourceHost.AppDomain.Should().BeNull();
115+
}
116+
98117
private static string GetArtifactsBinDir()
99118
{
100119
var artifactsBinDirPath = Path.GetFullPath(Path.Combine(

‎test/IntegrationTests/TestAssets/OutputTestProject/UnitTest3.cs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Collections.Specialized;
6+
using System.Data;
7+
using System.Threading;
8+
9+
using Microsoft.VisualStudio.TestTools.UnitTesting;
10+
11+
namespace OutputTestProject;
12+
13+
[TestClass]
14+
public class UnitTest3
15+
{
16+
private static readonly Random Rng = new();
17+
18+
private static TestContext s_testContext;
19+
20+
public TestContext TestContext { get; set; }
21+
22+
[ClassInitialize]
23+
public static void ClassInitialize(TestContext testContext)
24+
{
25+
s_testContext = testContext;
26+
s_testContext.WriteLine("UnitTest3 - ClassInitialize");
27+
}
28+
29+
[TestInitialize]
30+
public void TestInitialize()
31+
{
32+
TestContext.WriteLine("UnitTest3 - TestInitialize");
33+
}
34+
35+
[TestCleanup]
36+
public void TestCleanup()
37+
{
38+
TestContext.WriteLine("UnitTest3 - TestCleanup");
39+
}
40+
41+
[ClassCleanup]
42+
public static void ClassCleanup()
43+
{
44+
s_testContext.WriteLine("UnitTest3 - ClassCleanup");
45+
}
46+
47+
[TestMethod]
48+
public void TestMethod1()
49+
{
50+
TestContext.WriteLine("UnitTest3 - TestMethod1 - Call 1");
51+
52+
// This makes the outputs more likely to run into each other
53+
// when running in parallel.
54+
// It also makes the test longer, because we check in the test
55+
// that all tests started before any test finished (to make sure
56+
// they actually run in parallel), and this gives us more leeway
57+
// on slower machines.
58+
Thread.Sleep(Rng.Next(20, 50));
59+
TestContext.WriteLine("UnitTest3 - TestMethod1 - Call 2");
60+
Thread.Sleep(Rng.Next(20, 50));
61+
TestContext.WriteLine("UnitTest3 - TestMethod1 - Call 3");
62+
}
63+
64+
[TestMethod]
65+
[DataRow("DataRowValue1")]
66+
[DataRow("DataRowValue2")]
67+
public void TestMethod2(string dataRowValu)
68+
{
69+
TestContext.WriteLine($"UnitTest3 - TestMethod2({dataRowValu}) - Call 1");
70+
Thread.Sleep(Rng.Next(20, 50));
71+
TestContext.WriteLine($"UnitTest3 - TestMethod2({dataRowValu}) - Call 2");
72+
Thread.Sleep(Rng.Next(20, 50));
73+
TestContext.WriteLine($"UnitTest3 - TestMethod2({dataRowValu}) - Call 3");
74+
}
75+
76+
[TestMethod]
77+
public void TestMethod3()
78+
{
79+
TestContext.WriteLine("UnitTest3 - TestMethod3 - Call 1");
80+
Thread.Sleep(Rng.Next(20, 50));
81+
TestContext.WriteLine("UnitTest3 - TestMethod3 - Call 2");
82+
Thread.Sleep(Rng.Next(20, 50));
83+
TestContext.WriteLine("UnitTest3 - TestMethod3 - Call 3");
84+
}
85+
}

‎test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/DesktopTestSourceHostTests.cs‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,16 @@ public void CreateInstanceForTypeShouldCreateTheTypeInANewAppDomain()
8484
// Setup
8585
DummyClass dummyClass = new();
8686
int currentAppDomainId = dummyClass.AppDomainId;
87+
string runSettingxml =
88+
@"<RunSettings>
89+
<RunConfiguration>
90+
<DisableAppDomain>False</DisableAppDomain>
91+
</RunConfiguration>
92+
</RunSettings>";
93+
var mockRunSettings = new Mock<IRunSettings>();
94+
mockRunSettings.Setup(rs => rs.SettingsXml).Returns(runSettingxml);
8795

88-
TestSourceHost sut = new(Assembly.GetExecutingAssembly().Location, null, null);
96+
TestSourceHost sut = new(Assembly.GetExecutingAssembly().Location, mockRunSettings.Object, null);
8997
sut.SetupHost();
9098

9199
// Execute
@@ -103,9 +111,18 @@ public void SetupHostShouldSetChildDomainsAppBaseToTestSourceLocation()
103111
{
104112
// Arrange
105113
DummyClass dummyClass = new();
114+
string runSettingxml =
115+
@"<RunSettings>
116+
<RunConfiguration>
117+
<DisableAppDomain>False</DisableAppDomain>
118+
</RunConfiguration>
119+
</RunSettings>";
120+
var mockRunSettings = new Mock<IRunSettings>();
121+
mockRunSettings.Setup(rs => rs.SettingsXml).Returns(runSettingxml);
106122

107123
var location = typeof(TestSourceHost).Assembly.Location;
108-
Mock<TestSourceHost> sourceHost = new(location, null, null) { CallBase = true };
124+
125+
Mock<TestSourceHost> sourceHost = new(location, mockRunSettings.Object, null) { CallBase = true };
109126

110127
try
111128
{
@@ -180,11 +197,20 @@ public void DisposeShouldSetTestHostShutdownOnIssueWithAppDomainUnload()
180197
{
181198
// Arrange
182199
var frameworkHandle = new Mock<IFrameworkHandle>();
183-
var testableAppDomain = new Mock<IAppDomain>();
200+
string runSettingxml =
201+
@"<RunSettings>
202+
<RunConfiguration>
203+
<DisableAppDomain>False</DisableAppDomain>
204+
</RunConfiguration>
205+
</RunSettings>";
206+
var mockRunSettings = new Mock<IRunSettings>();
207+
mockRunSettings.Setup(rs => rs.SettingsXml).Returns(runSettingxml);
184208

209+
var testableAppDomain = new Mock<IAppDomain>();
185210
testableAppDomain.Setup(ad => ad.CreateDomain(It.IsAny<string>(), It.IsAny<Evidence>(), It.IsAny<AppDomainSetup>())).Returns(AppDomain.CurrentDomain);
186211
testableAppDomain.Setup(ad => ad.Unload(It.IsAny<AppDomain>())).Throws(new CannotUnloadAppDomainException());
187-
var sourceHost = new TestSourceHost(typeof(DesktopTestSourceHostTests).Assembly.Location, null, frameworkHandle.Object, testableAppDomain.Object);
212+
213+
var sourceHost = new TestSourceHost(typeof(DesktopTestSourceHostTests).Assembly.Location, mockRunSettings.Object, frameworkHandle.Object, testableAppDomain.Object);
188214
sourceHost.SetupHost();
189215

190216
// Act

0 commit comments

Comments
 (0)