Skip to content

Commit

Permalink
Fix assembly resolution error (#2948)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Evangelink
Evangelink authored May 27, 2024
1 parent b4ab8ee commit efc614f
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 63 deletions.
123 changes: 72 additions & 51 deletions src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class AssemblyResolver :
/// </summary>
private readonly object _syncLock = new();

private static List<string>? s_currentlyLoading;
private bool _disposed;

/// <summary>
Expand Down Expand Up @@ -335,7 +336,7 @@ protected virtual
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info(
"AssemblyResolver: {0}: Failed to create assemblyName. Reason:{1} ",
"MSTest.AssemblyResolver.OnResolve: Failed to create assemblyName '{0}'. Reason: {1} ",
name,
ex);
}
Expand All @@ -344,7 +345,7 @@ protected virtual
return null;
}

DebugEx.Assert(requestedName != null && !StringEx.IsNullOrEmpty(requestedName.Name), "AssemblyResolver.OnResolve: requested is null or name is empty!");
DebugEx.Assert(requestedName != null && !StringEx.IsNullOrEmpty(requestedName.Name), "MSTest.AssemblyResolver.OnResolve: requested is null or name is empty!");

foreach (string dir in searchDirectorypaths)
{
Expand All @@ -359,15 +360,39 @@ protected virtual
{
if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose("AssemblyResolver: Searching assembly: {0} in the directory: {1}", requestedName.Name, dir);
EqtTrace.Verbose("MSTest.AssemblyResolver.OnResolve: Searching assembly '{0}' in the directory '{1}'", requestedName.Name, dir);
}
});

foreach (string extension in new string[] { ".dll", ".exe" })
{
string assemblyPath = Path.Combine(dir, requestedName.Name + extension);

bool isPushed = false;
bool isResource = requestedName.Name.EndsWith(".resources", StringComparison.InvariantCulture);
if (isResource)
{
// Are we recursively looking up the same resource? Note - our backout code will set
// the ResourceHelper's currentlyLoading stack to null if an exception occurs.
if (s_currentlyLoading != null && s_currentlyLoading.Count > 0 && s_currentlyLoading.LastIndexOf(assemblyPath) != -1)
{
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Assembly '{0}' is searching for itself recursively '{1}', returning as not found.", name, assemblyPath);
_resolvedAssemblies[name] = null;
return null;
}

s_currentlyLoading ??= new List<string>();
s_currentlyLoading.Add(assemblyPath); // Push
isPushed = true;
}

Assembly? assembly = SearchAndLoadAssembly(assemblyPath, name, requestedName, isReflectionOnly);
if (isResource && isPushed)
{
DebugEx.Assert(s_currentlyLoading is not null, "_currentlyLoading should not be null");
s_currentlyLoading.RemoveAt(s_currentlyLoading.Count - 1); // Pop
}

if (assembly != null)
{
return assembly;
Expand Down Expand Up @@ -447,7 +472,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
{
if (StringEx.IsNullOrEmpty(args.Name))
{
Debug.Fail("AssemblyResolver.OnResolve: args.Name is null or empty.");
Debug.Fail("MSTest.AssemblyResolver.OnResolve: args.Name is null or empty.");
return null;
}

Expand All @@ -457,7 +482,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Resolving assembly: {0}.", args.Name);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolving assembly '{0}'", args.Name);
}
});

Expand All @@ -469,7 +494,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Resolving assembly after applying policy: {0}.", assemblyNameToLoad);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolving assembly after applying policy '{0}'", assemblyNameToLoad);
}
});

Expand All @@ -482,62 +507,58 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
}

assembly = SearchAssembly(_searchDirectories, assemblyNameToLoad, isReflectionOnly);

if (assembly != null)
{
return assembly;
}

if (_directoryList != null && _directoryList.Count != 0)
// required assembly is not present in searchDirectories??
// see, if we can find it in user specified search directories.
while (assembly == null && _directoryList?.Count > 0)
{
// required assembly is not present in searchDirectories??
// see, if we can find it in user specified search directories.
while (assembly == null && _directoryList.Count > 0)
{
// instead of loading whole search directory in one time, we are adding directory on the basis of need
RecursiveDirectoryPath currentNode = _directoryList.Dequeue();

List<string> incrementalSearchDirectory = [];
// instead of loading whole search directory in one time, we are adding directory on the basis of need
RecursiveDirectoryPath currentNode = _directoryList.Dequeue();

if (DoesDirectoryExist(currentNode.DirectoryPath))
{
incrementalSearchDirectory.Add(currentNode.DirectoryPath);

if (currentNode.IncludeSubDirectories)
{
// Add all its sub-directory in depth first search order.
AddSubdirectories(currentNode.DirectoryPath, incrementalSearchDirectory);
}
List<string> incrementalSearchDirectory = [];

// Add this directory list in this.searchDirectories so that when we will try to resolve some other
// assembly, then it will look in this whole directory first.
_searchDirectories.AddRange(incrementalSearchDirectory);
if (DoesDirectoryExist(currentNode.DirectoryPath))
{
incrementalSearchDirectory.Add(currentNode.DirectoryPath);

assembly = SearchAssembly(incrementalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
}
else
if (currentNode.IncludeSubDirectories)
{
// generate warning that path does not exist.
SafeLog(
assemblyNameToLoad,
() =>
{
if (EqtTrace.IsWarningEnabled)
{
EqtTrace.Warning(
"The Directory: {0}, does not exist",
currentNode.DirectoryPath);
}
});
// Add all its sub-directory in depth first search order.
AddSubdirectories(currentNode.DirectoryPath, incrementalSearchDirectory);
}
}

if (assembly != null)
// Add this directory list in this.searchDirectories so that when we will try to resolve some other
// assembly, then it will look in this whole directory first.
_searchDirectories.AddRange(incrementalSearchDirectory);

assembly = SearchAssembly(incrementalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
}
else
{
return assembly;
// generate warning that path does not exist.
SafeLog(
assemblyNameToLoad,
() =>
{
if (EqtTrace.IsWarningEnabled)
{
EqtTrace.Warning(
"MSTest.AssemblyResolver.OnResolve: the directory '{0}', does not exist",
currentNode.DirectoryPath);
}
});
}
}

if (assembly != null)
{
return assembly;
}

// Try for default load for System dlls that can't be found in search paths. Needs to loaded just by name.
try
{
Expand Down Expand Up @@ -580,7 +601,7 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: {0}: Failed to load assembly. Reason: {1}", assemblyNameToLoad, ex);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason: {1}", assemblyNameToLoad, ex);
}
});
}
Expand Down Expand Up @@ -609,7 +630,7 @@ private bool TryLoadFromCache(string assemblyName, bool isReflectionOnly, out As
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Resolved: {0}.", assemblyName);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolved '{0}'", assemblyName);
}
});
return true;
Expand Down Expand Up @@ -685,7 +706,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Resolved assembly: {0}. ", assemblyName);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Resolved assembly '{0}'", assemblyName);
}
});

Expand All @@ -699,7 +720,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Failed to load assembly: {0}. Reason:{1} ", assemblyName, ex);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason:{1} ", assemblyName, ex);
}
});

Expand All @@ -717,7 +738,7 @@ private static void SafeLog(string? assemblyName, Action loggerAction)
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: Failed to load assembly: {0}. Reason:{1} ", assemblyName, ex);
EqtTrace.Info("MSTest.AssemblyResolver.OnResolve: Failed to load assembly '{0}'. Reason:{1} ", assemblyName, ex);
}
});
}
Expand Down
24 changes: 14 additions & 10 deletions src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,8 @@ internal TestSourceHost(string sourceFileName, IRunSettings? runSettings, IFrame
/// </summary>
public void SetupHost()
{
#if NETFRAMEWORK || NET
List<string> resolutionPaths = GetResolutionPaths(
_sourceFileName,
#if NETFRAMEWORK
VSInstallationUtilities.IsCurrentProcessRunningInPortableMode());
#else
false);
#endif
#if NET
List<string> resolutionPaths = GetResolutionPaths(_sourceFileName, false);

if (EqtTrace.IsInfoEnabled)
{
Expand All @@ -125,10 +119,20 @@ public void SetupHost()
{
assemblyResolver.Dispose();
}
#elif NETFRAMEWORK
List<string> resolutionPaths = GetResolutionPaths(_sourceFileName, VSInstallationUtilities.IsCurrentProcessRunningInPortableMode());

#endif
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("DesktopTestSourceHost.SetupHost(): Creating assembly resolver with resolution paths {0}.", string.Join(",", resolutionPaths));
}

// NOTE: These 2 lines are super important, see https://github.com/microsoft/testfx/issues/2922
// It's not entirely clear why but not assigning directly the resolver to the field (or/and) disposing the resolver in
// case of an error in TryAddSearchDirectoriesSpecifiedInRunSettingsToAssemblyResolver causes the issue.
_parentDomainAssemblyResolver = new AssemblyResolver(resolutionPaths);
_ = TryAddSearchDirectoriesSpecifiedInRunSettingsToAssemblyResolver(_parentDomainAssemblyResolver, Path.GetDirectoryName(_sourceFileName)!);

#if NETFRAMEWORK
// Case when DisableAppDomain setting is present in runsettings and no child-appdomain needs to be created
if (!_isAppDomainCreationDisabled)
{
Expand Down
4 changes: 2 additions & 2 deletions test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ namespace MSTest.Acceptance.IntegrationTests;
[TestGroup]
public class AssemblyResolverTests : AcceptanceTestBase
{
private readonly TestAssetFixture _testAssetFixture;
private const string AssetName = "AssemblyResolverCrash";
private readonly TestAssetFixture _testAssetFixture;

// There's a bug in TAFX where we need to use it at least one time somewhere to use it inside the fixture self (AcceptanceFixture).
public AssemblyResolverTests(ITestExecutionContext testExecutionContext, TestAssetFixture testAssetFixture,
Expand All @@ -31,7 +31,7 @@ public async Task RunningTests_DoesNotHitResourceRecursionIssueAndDoesNotCrashTh

var testHost = TestHost.LocateFrom(_testAssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetFramework[0].Arguments);

var testHostResult = await testHost.ExecuteAsync();
TestHostResult testHostResult = await testHost.ExecuteAsync();

testHostResult.AssertExitCodeIs(ExitCodes.Success);
}
Expand Down

0 comments on commit efc614f

Please sign in to comment.