Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuring child domain's appbase to be test source location #659

Merged
merged 8 commits into from
Nov 26, 2019
Merged

Configuring child domain's appbase to be test source location #659

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
740474b
Correcting error message when dynamic data doen't have any data
jayaranigarg Dec 18, 2018
e34a4c3
Fix merge conflicts
jayaranigarg Apr 30, 2019
c554dd5
Merge remote-tracking branch 'upstream/master'
jayaranigarg Nov 8, 2019
0fef58c
Configuring child domain's appbase to be test source location
jayaranigarg Nov 18, 2019
36d39bc
Nit
jayaranigarg Nov 18, 2019
752baab
Catch typeload exception if TestAssemblySettingsProvider doesn't exist
jayaranigarg Nov 25, 2019
6fc051c
PR comments
jayaranigarg Nov 26, 2019
b0f5851
Nit
jayaranigarg Nov 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/Adapter/MSTest.CoreAdapter/Discovery/AssemblyEnumeratorWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ private ICollection<UnitTestElement> GetTestsInIsolation(string fullFilePath, IR
var assemblyEnumerator = isolationHost.CreateInstanceForType(
typeof(AssemblyEnumerator), new object[] { MSTestSettings.CurrentSettings }) as AssemblyEnumerator;

// After loading adapter reset the child-domain's appbase to point to test source location
isolationHost.UpdateAppBaseToTestSourceLocation();

return assemblyEnumerator.EnumerateAssembly(fullFilePath, out warnings);
}
}
Expand Down
31 changes: 24 additions & 7 deletions src/Adapter/MSTest.CoreAdapter/Execution/TestExecutionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,6 @@ private void ExecuteTestsInSource(IEnumerable<TestCase> tests, IRunContext runCo
typeof(UnitTestRunner),
new object[] { MSTestSettings.CurrentSettings }) as UnitTestRunner;

// After loading adapter reset the chils-domain's appbase to point to test source location
isolationHost.UpdateAppBaseToTestSourceLocation();

PlatformServiceProvider.Instance.AdapterTraceLogger.LogInfo("Created unit-test runner {0}", source);

// Default test set is filtered tests based on user provided filter criteria
Expand All @@ -247,11 +244,31 @@ private void ExecuteTestsInSource(IEnumerable<TestCase> tests, IRunContext runCo
sourceLevelParameters = sourceLevelParameters.Concat(this.sessionParameters).ToDictionary(x => x.Key, x => x.Value);
}

var sourceSettingsProvider = isolationHost.CreateInstanceForType(
typeof(TestAssemblySettingsProvider),
null) as TestAssemblySettingsProvider;
TestAssemblySettingsProvider sourceSettingsProvider = null;

try
{
sourceSettingsProvider = isolationHost.CreateInstanceForType(
typeof(TestAssemblySettingsProvider),
null) as TestAssemblySettingsProvider;
}
catch (TypeLoadException ex)
{
PlatformServiceProvider.Instance.AdapterTraceLogger.LogInfo(
"Could not create TestAssemblySettingsProvider instance in child app-domain",
ex);
}

TestAssemblySettings sourceSettings;
if (sourceSettingsProvider != null)
{
sourceSettings = sourceSettingsProvider.GetSettings(source);
}
else
{
sourceSettings = new TestAssemblySettings();
}

var sourceSettings = sourceSettingsProvider.GetSettings(source);
var parallelWorkers = sourceSettings.Workers;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that default values for sourceSettings.Workers, & Scope is set appropriately in case we fail to create instance of "TestAssemblySettingsProvider"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, checked that.

var parallelScope = sourceSettings.Scope;

Expand Down
24 changes: 6 additions & 18 deletions src/Adapter/PlatformServices.Desktop/Services/DesktopTestSourceHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ internal TestSourceHost(string sourceFileName, IRunSettings runSettings, IFramew
this.sourceFileName = sourceFileName;
this.runSettings = runSettings;
this.frameworkHandle = frameworkHandle;

this.appDomain = appDomain;

// Set the environment context.
Expand Down Expand Up @@ -109,10 +108,7 @@ public void SetupHost()
this.targetFrameworkVersion = this.GetTargetFrameworkVersionString(this.sourceFileName);
AppDomainUtilities.SetAppDomainFrameworkVersionBasedOnTestSource(appDomainSetup, this.targetFrameworkVersion);

// Temporarily set appbase to the location from where adapter should be picked up from. We will later reset this to test source location
// once adapter gets loaded in the child app domain.
appDomainSetup.ApplicationBase = Path.GetDirectoryName(typeof(TestSourceHost).Assembly.Location);

appDomainSetup.ApplicationBase = this.GetAppBaseAsPerPlatform();
var configFile = this.GetConfigFileForTestSource(this.sourceFileName);
AppDomainUtilities.SetConfigurationFile(appDomainSetup, configFile);

Expand Down Expand Up @@ -214,17 +210,11 @@ public void Dispose()
}

/// <summary>
/// Updates child-domain's appbase to point to test source location.
/// Gets child-domain's appbase to point to appropriate location.
/// </summary>
public void UpdateAppBaseToTestSourceLocation()
/// <returns>Appbase path that should be set for child appdomain</returns>
internal string GetAppBaseAsPerPlatform()
{
// Simply return if no child-appdomain was created
if (this.isAppDomainCreationDisabled)
{
return;
}

// After adapter has been loaded, reset child-appdomains appbase.
// The below logic of preferential setting the appdomains appbase is needed because:
// 1. We set this to the location of the test source if it is built for Full CLR -> Ideally this needs to be done in all situations.
// 2. We set this to the location where the current adapter is being picked up from for UWP and .Net Core scenarios -> This needs to be
Expand All @@ -234,14 +224,12 @@ public void UpdateAppBaseToTestSourceLocation()
// there would be a mismatch of platform service assemblies during discovery.
if (this.targetFrameworkVersion.Contains(PlatformServices.Constants.DotNetFrameWorkStringPrefix))
{
this.domain.SetData("APPBASE", Path.GetDirectoryName(this.sourceFileName) ?? Path.GetDirectoryName(typeof(TestSourceHost).Assembly.Location));
return Path.GetDirectoryName(this.sourceFileName) ?? Path.GetDirectoryName(typeof(TestSourceHost).Assembly.Location);
}
else
{
this.domain.SetData("APPBASE", Path.GetDirectoryName(typeof(TestSourceHost).Assembly.Location));
return Path.GetDirectoryName(typeof(TestSourceHost).Assembly.Location);
}

EqtTrace.Info("DesktopTestSourceHost.UpdateAppBaseToTestSourceLocation(): Updating domain's appbase path for source {0} to {1}.", this.sourceFileName, this.domain.SetupInformation.ApplicationBase);
}

/// <summary>
Expand Down
5 changes: 0 additions & 5 deletions src/Adapter/PlatformServices.Interface/ITestSourceHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,5 @@ public interface ITestSourceHost : IDisposable
/// <returns> An instance of the type created in the host. </returns>
/// <remarks> If a type is to be created in isolation then it needs to be a MarshalByRefObject. </remarks>
object CreateInstanceForType(Type type, object[] args);

/// <summary>
/// Updates child-domain's appbase to point to test source location
/// </summary>
void UpdateAppBaseToTestSourceLocation();
}
}
8 changes: 0 additions & 8 deletions src/Adapter/PlatformServices.NetCore/Services/NetCoreTestSourceHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,6 @@ public object CreateInstanceForType(Type type, object[] args)
return Activator.CreateInstance(type, args);
}

/// <summary>
/// Updates child-domain's appbase to point to test source location
/// </summary>
public void UpdateAppBaseToTestSourceLocation()
{
// Do nothing.
}

/// <summary>
/// Sets context required for running tests.
/// </summary>
Expand Down
8 changes: 0 additions & 8 deletions src/Adapter/PlatformServices.Shared/netstandard1.0/Services/ns10TestSourceHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ public void Dispose()
{
// Do nothing.
}

/// <summary>
/// Updates child-domain's appbase to point to test source location
/// </summary>
public void UpdateAppBaseToTestSourceLocation()
{
// Do nothing.
}
}

#pragma warning restore SA1649 // SA1649FileNameMustMatchTypeName
Expand Down
64 changes: 2 additions & 62 deletions test/UnitTests/PlatformServices.Desktop.Unit.Tests/Services/DesktopTestSourceHostTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void CreateInstanceForTypeShouldCreateTheTypeInANewAppDomain()
}

[TestMethod]
public void SetupHostShouldSetChildDomainsAppBaseToAdapterLocation()
public void SetupHostShouldSetChildDomainsAppBaseToTestSourceLocation()
{
// Arrange
DummyClass dummyclass = new DummyClass();
Expand All @@ -121,7 +121,7 @@ public void SetupHostShouldSetChildDomainsAppBaseToAdapterLocation()
var expectedObject = sourceHost.Object.CreateInstanceForType(typeof(DummyClass), null) as DummyClass;

// Assert
Assert.AreEqual(Path.GetDirectoryName(location), expectedObject.AppDomainAppBase);
Assert.AreEqual(Path.GetDirectoryName(typeof(DesktopTestSourceHostTests).Assembly.Location), expectedObject.AppDomainAppBase);
}
finally
{
Expand Down Expand Up @@ -185,66 +185,6 @@ public void SetupHostShouldSetResolutionsPaths()
}
}

/// <summary>
/// This test should ideally be choosing a different path for the test source. Currently both the test source and the adapter
/// are in the same location. However when we move to run these tests with the V2 itself, then this would be valid.
/// Leaving the test running till then.
/// </summary>
[TestMethod]
public void UpdateAppBaseToTestSourceLocationShouldSetDomainsAppBaseToTestSourceLocationForFullCLRTestss()
{
// Arrange
DummyClass dummyclass = new DummyClass();

var location = typeof(DesktopTestSourceHostTests).Assembly.Location;
var sourceHost = new Mock<TestSourceHost>(location, null, null) { CallBase = true };
try
{
sourceHost.Setup(tsh => tsh.GetTargetFrameworkVersionString(location))
.Returns(".NETFramework,Version=v4.5.1");

// Act
sourceHost.Object.SetupHost();
var expectedObject =
sourceHost.Object.CreateInstanceForType(typeof(DummyClass), null) as DummyClass;
sourceHost.Object.UpdateAppBaseToTestSourceLocation();

// Assert
Assert.AreEqual(Path.GetDirectoryName(location), expectedObject.AppDomainAppBase);
}
finally
{
sourceHost.Object.Dispose();
}
}

[TestMethod]
public void UpdateAppBaseToTestSourceLocationShouldSetDomainsAppBaseToAdaptersLocationForNonFullCLRTests()
{
// Arrange
DummyClass dummyclass = new DummyClass();

var location = typeof(TestSourceHost).Assembly.Location;
Mock<TestSourceHost> sourceHost = new Mock<TestSourceHost>(location, null, null) { CallBase = true };

try
{
sourceHost.Setup(tsh => tsh.GetTargetFrameworkVersionString(location)).Returns(".NETCore,Version=v5.0");

// Act
sourceHost.Object.SetupHost();
var expectedObject = sourceHost.Object.CreateInstanceForType(typeof(DummyClass), null) as DummyClass;
sourceHost.Object.UpdateAppBaseToTestSourceLocation();

// Assert
Assert.AreEqual(Path.GetDirectoryName(location), expectedObject.AppDomainAppBase);
}
finally
{
sourceHost.Object.Dispose();
}
}

[TestMethod]
public void DisposeShouldSetTestHostShutdownOnIssueWithAppDomainUnload()
{
Expand Down