From efc614f02eab4484f882341374ce475fa6411aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Mon, 27 May 2024 15:06:17 +0200 Subject: [PATCH] Fix assembly resolution error (#2948) --- .../AssemblyResolver.cs | 123 ++++++++++-------- .../Services/TestSourceHost.cs | 24 ++-- .../AssemblyResolverTests.cs | 4 +- 3 files changed, 88 insertions(+), 63 deletions(-) diff --git a/src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs b/src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs index 9f8eb71a73..24a53288f9 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs @@ -86,6 +86,7 @@ class AssemblyResolver : /// private readonly object _syncLock = new(); + private static List? s_currentlyLoading; private bool _disposed; /// @@ -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); } @@ -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) { @@ -359,7 +360,7 @@ 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); } }); @@ -367,7 +368,31 @@ protected virtual { 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(); + 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; @@ -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; } @@ -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); } }); @@ -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); } }); @@ -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 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 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 { @@ -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); } }); } @@ -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; @@ -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); } }); @@ -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); } }); @@ -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); } }); } diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs b/src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs index d7f1254272..792aab749e 100644 --- a/src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs +++ b/src/Adapter/MSTestAdapter.PlatformServices/Services/TestSourceHost.cs @@ -102,14 +102,8 @@ internal TestSourceHost(string sourceFileName, IRunSettings? runSettings, IFrame /// public void SetupHost() { -#if NETFRAMEWORK || NET - List resolutionPaths = GetResolutionPaths( - _sourceFileName, -#if NETFRAMEWORK - VSInstallationUtilities.IsCurrentProcessRunningInPortableMode()); -#else - false); -#endif +#if NET + List resolutionPaths = GetResolutionPaths(_sourceFileName, false); if (EqtTrace.IsInfoEnabled) { @@ -125,10 +119,20 @@ public void SetupHost() { assemblyResolver.Dispose(); } +#elif NETFRAMEWORK + List 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) { diff --git a/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs b/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs index 01fab28cc0..619d20fe10 100644 --- a/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs +++ b/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/AssemblyResolverTests.cs @@ -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, @@ -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); }