Skip to content

Commit 2eb19d5

Browse files
Fix deployment regression by @Youssef1313 in #6718 (backport to rel/4.0) (#6734)
Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>
1 parent 2cd851e commit 2eb19d5

File tree

7 files changed

+151
-8
lines changed

7 files changed

+151
-8
lines changed

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ private async Task ExecuteTestsInSourceAsync(IEnumerable<TestCase> tests, IRunCo
308308
int parallelWorkers = sourceSettings.Workers;
309309
ExecutionScope parallelScope = sourceSettings.Scope;
310310
TestCase[] testsToRun = [.. tests.Where(t => MatchTestFilter(filterExpression, t, _testMethodFilter))];
311-
UnitTestElement[] unitTestElements = [.. testsToRun.Select(e => e.ToUnitTestElement(source))];
311+
UnitTestElement[] unitTestElements = [.. testsToRun.Select(e => e.ToUnitTestElementWithUpdatedSource(source))];
312312
// Create an instance of a type defined in adapter so that adapter gets loaded in the child app domain
313313
var testRunner = (UnitTestRunner)isolationHost.CreateInstanceForType(
314314
typeof(UnitTestRunner),
@@ -455,7 +455,7 @@ private async Task ExecuteTestsWithTestRunnerAsync(
455455
}
456456

457457
hasAnyRunnableTests = true;
458-
var unitTestElement = currentTest.ToUnitTestElement(source);
458+
UnitTestElement unitTestElement = currentTest.ToUnitTestElementWithUpdatedSource(source);
459459

460460
testExecutionRecorder.RecordStart(currentTest);
461461

@@ -517,7 +517,7 @@ private async Task ExecuteTestsWithTestRunnerAsync(
517517
}
518518

519519
Trait trait = currentTest.Traits.First(t => t.Name == EngineConstants.FixturesTestTrait);
520-
var unitTestElement = currentTest.ToUnitTestElement(source);
520+
UnitTestElement unitTestElement = currentTest.ToUnitTestElementWithUpdatedSource(source);
521521
FixtureTestResult fixtureTestResult = testRunner.GetFixtureTestResult(unitTestElement.TestMethod, trait.Value);
522522

523523
if (fixtureTestResult.IsExecuted)

src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,9 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn
736736
// testMethod.MethodInfo can be null if 'TestMethod' instance crossed app domain boundaries.
737737
// This happens on .NET Framework when app domain is enabled, and the MethodInfo is calculated and set during discovery.
738738
// Then, execution will cause TestMethod to cross to a different app domain, and MethodInfo will be null.
739+
// In addition, it also happens when deployment items are used and app domain is disabled.
740+
// We explicitly set it to null in this case because the original MethodInfo calculated during discovery cannot be used because
741+
// it points to the type loaded from the assembly in bin instead of from deployment directory.
739742
methodBase = testMethod.MethodInfo ?? ManagedNameHelper.GetMethod(testClassInfo.Parent.Assembly, testMethod.ManagedTypeName!, testMethod.ManagedMethodName!);
740743
}
741744
catch (InvalidManagedNameException)

src/Adapter/MSTestAdapter.PlatformServices/Extensions/TestCaseExtensions.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,19 @@ internal static string GetTestName(this TestCase testCase, string? testClassName
6868
/// <param name="testCase"> The test case. </param>
6969
/// <param name="source"> The source. If deployed this is the full path of the source in the deployment directory. </param>
7070
/// <returns> The converted <see cref="UnitTestElement"/>. </returns>
71-
internal static UnitTestElement ToUnitTestElement(this TestCase testCase, string source)
71+
internal static UnitTestElement ToUnitTestElementWithUpdatedSource(this TestCase testCase, string source)
7272
{
7373
if (testCase.LocalExtensionData is UnitTestElement unitTestElement)
7474
{
75-
return unitTestElement;
75+
// If the requested source is different, clone it with updated source.
76+
// This can happen when there are deployment items in tests.
77+
// In this case, the source would be the path of the deployment directory.
78+
// If we don't return UnitTestElement with the correct path to deployment directory, we will
79+
// end up trying to load the test assembly twice in the same appdomain, once with the default context and once in a LoadFrom context.
80+
// See https://github.com/microsoft/testfx/issues/6713
81+
return unitTestElement.TestMethod.AssemblyName != source
82+
? unitTestElement.CloneWithUpdatedSource(source)
83+
: unitTestElement;
7684
}
7785

7886
string? testClassName = testCase.GetPropertyValue(EngineConstants.TestClassNameProperty) as string;

src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/TestMethod.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,4 +165,12 @@ public string? DeclaringClassFullName
165165
internal string DisplayName { get; set; }
166166

167167
internal TestMethod Clone() => (TestMethod)MemberwiseClone();
168+
169+
internal TestMethod CloneWithUpdatedSource(string source)
170+
{
171+
var clone = (TestMethod)MemberwiseClone();
172+
AssemblyName = source;
173+
MethodInfo = null;
174+
return clone;
175+
}
168176
}

src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ internal UnitTestElement Clone()
9292
return clone;
9393
}
9494

95+
internal UnitTestElement CloneWithUpdatedSource(string source)
96+
{
97+
var clone = (UnitTestElement)MemberwiseClone();
98+
clone.TestMethod = TestMethod.CloneWithUpdatedSource(source);
99+
return clone;
100+
}
101+
95102
/// <summary>
96103
/// Convert the UnitTestElement instance to an Object Model testCase instance.
97104
/// </summary>
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
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 Microsoft.Testing.Platform.Acceptance.IntegrationTests;
5+
using Microsoft.Testing.Platform.Acceptance.IntegrationTests.Helpers;
6+
using Microsoft.Testing.Platform.Helpers;
7+
8+
namespace MSTest.Acceptance.IntegrationTests;
9+
10+
[TestClass]
11+
public class DeploymentItemTests : AcceptanceTestBase<DeploymentItemTests.TestAssetFixture>
12+
{
13+
private const string AssetName = nameof(DeploymentItemTests);
14+
15+
[TestMethod]
16+
[OSCondition(OperatingSystems.Windows)]
17+
[DataRow("AppDomainDisabled.runsettings", IgnoreMessage = "https://github.com/microsoft/testfx/issues/6738")]
18+
[DataRow("AppDomainEnabled.runsettings")]
19+
public async Task AssemblyIsLoadedOnceFromDeploymentDirectory(string runsettings)
20+
{
21+
var testHost = TestHost.LocateFrom(AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetFramework[0]);
22+
TestHostResult testHostResult = await testHost.ExecuteAsync($"--settings {runsettings}", cancellationToken: TestContext.CancellationToken);
23+
testHostResult.AssertOutputContainsSummary(failed: 0, passed: 1, skipped: 0);
24+
testHostResult.AssertExitCodeIs(ExitCodes.Success);
25+
}
26+
27+
public sealed class TestAssetFixture() : TestAssetFixtureBase(AcceptanceFixture.NuGetGlobalPackagesFolder)
28+
{
29+
private const string Sources = """
30+
#file DeploymentItemTests.csproj
31+
<Project Sdk="Microsoft.NET.Sdk">
32+
33+
<PropertyGroup>
34+
<TargetFrameworks>$TargetFrameworks$</TargetFrameworks>
35+
<EnableMSTestRunner>true</EnableMSTestRunner>
36+
<OutputType>Exe</OutputType>
37+
</PropertyGroup>
38+
39+
<ItemGroup>
40+
<PackageReference Include="MSTest.TestFramework" Version="$MSTestVersion$" />
41+
<PackageReference Include="MSTest.TestAdapter" Version="$MSTestVersion$" />
42+
</ItemGroup>
43+
44+
<ItemGroup>
45+
<None Update="TestDeploymentItem.xml">
46+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
47+
</None>
48+
</ItemGroup>
49+
50+
<ItemGroup>
51+
<None Update="AppDomainEnabled.runsettings">
52+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
53+
</None>
54+
<None Update="AppDomainDisabled.runsettings">
55+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
56+
</None>
57+
</ItemGroup>
58+
59+
</Project>
60+
61+
#file TestDeploymentItem.xml
62+
<?xml version="1.0" encoding="utf-8" ?>
63+
<Root />
64+
65+
#file AppDomainEnabled.runsettings
66+
<?xml version="1.0" encoding="utf-8" ?>
67+
<RunSettings>
68+
<RunConfiguration>
69+
<DisableAppDomain>false</DisableAppDomain>
70+
</RunConfiguration>
71+
</RunSettings>
72+
73+
#file AppDomainDisabled.runsettings
74+
<?xml version="1.0" encoding="utf-8" ?>
75+
<RunSettings>
76+
<RunConfiguration>
77+
<DisableAppDomain>true</DisableAppDomain>
78+
</RunConfiguration>
79+
</RunSettings>
80+
81+
#file TestClass1.cs
82+
using System;
83+
using System.Collections.Generic;
84+
using System.IO;
85+
using System.Linq;
86+
using Microsoft.VisualStudio.TestTools.UnitTesting;
87+
88+
[TestClass]
89+
public sealed class Test1
90+
{
91+
public TestContext TestContext { get; set; }
92+
93+
[TestMethod]
94+
[DeploymentItem("TestDeploymentItem.xml")]
95+
public void TestMethod1()
96+
{
97+
var asm = Assert.ContainsSingle(AppDomain.CurrentDomain.GetAssemblies().Where(a => a.GetName().Name == "DeploymentItemTests"));
98+
var path = asm.Location;
99+
Assert.AreEqual(TestContext.DeploymentDirectory, Path.GetDirectoryName(path));
100+
}
101+
}
102+
""";
103+
104+
public string TargetAssetPath => GetAssetPath(AssetName);
105+
106+
public override IEnumerable<(string ID, string Name, string Code)> GetAssetsToGenerate()
107+
{
108+
yield return (AssetName, AssetName,
109+
Sources
110+
.PatchTargetFrameworks(TargetFrameworks.NetFramework[0])
111+
.PatchCodeWithReplace("$MSTestVersion$", MSTestVersion));
112+
}
113+
}
114+
115+
public TestContext TestContext { get; set; }
116+
}

test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Extensions/TestCaseExtensionsTests.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
5+
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
56
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
67
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
78

@@ -23,7 +24,7 @@ public void ToUnitTestElementShouldReturnUnitTestElementWithFieldsSet()
2324
testCase.SetPropertyValue(EngineConstants.TestCategoryProperty, testCategories);
2425
testCase.SetPropertyValue(EngineConstants.TestClassNameProperty, "DummyClassName");
2526

26-
var resultUnitTestElement = testCase.ToUnitTestElement(testCase.Source);
27+
UnitTestElement resultUnitTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
2728

2829
Verify(resultUnitTestElement.Priority == 2);
2930
Verify(testCategories == resultUnitTestElement.TestCategory);
@@ -38,7 +39,7 @@ public void ToUnitTestElementForTestCaseWithNoPropertiesShouldReturnUnitTestElem
3839
TestCase testCase = new("DummyClass.DummyMethod", new("DummyUri", UriKind.Relative), Assembly.GetCallingAssembly().FullName!);
3940
testCase.SetPropertyValue(EngineConstants.TestClassNameProperty, "DummyClassName");
4041

41-
var resultUnitTestElement = testCase.ToUnitTestElement(testCase.Source);
42+
UnitTestElement resultUnitTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
4243

4344
// These are set for testCase by default by ObjectModel.
4445
Verify(resultUnitTestElement.Priority == 0);
@@ -51,7 +52,7 @@ public void ToUnitTestElementShouldAddDeclaringClassNameToTestElementWhenAvailab
5152
testCase.SetPropertyValue(EngineConstants.TestClassNameProperty, "DummyClassName");
5253
testCase.SetPropertyValue(EngineConstants.DeclaringClassNameProperty, "DummyDeclaringClassName");
5354

54-
var resultUnitTestElement = testCase.ToUnitTestElement(testCase.Source);
55+
UnitTestElement resultUnitTestElement = testCase.ToUnitTestElementWithUpdatedSource(testCase.Source);
5556

5657
Verify(resultUnitTestElement.TestMethod.FullClassName == "DummyClassName");
5758
Verify(resultUnitTestElement.TestMethod.DeclaringClassFullName == "DummyDeclaringClassName");

0 commit comments

Comments
 (0)