From 2945f31e9cd34095359a769bab44285f87d8f175 Mon Sep 17 00:00:00 2001 From: Charlie Poole Date: Fri, 1 Nov 2024 12:07:11 -0700 Subject: [PATCH] Revise api in response to comments --- choco/nunit-console-runner.nuspec | 6 -- choco/nunit.console.choco.addins | 7 --- choco/nunit.console.choco.agent.addins | 7 --- .../Services/ExtensionManagerTests.cs | 2 +- .../Services/ExtensionServiceTests.cs | 5 +- .../Services/ExtensionManager.cs | 63 +++++++++++-------- .../Services/ExtensionService.cs | 20 ++---- .../Services/IExtensionManager.cs | 10 ++- 8 files changed, 50 insertions(+), 70 deletions(-) delete mode 100644 choco/nunit.console.choco.addins delete mode 100644 choco/nunit.console.choco.agent.addins diff --git a/choco/nunit-console-runner.nuspec b/choco/nunit-console-runner.nuspec index a1c687367..d5eededcc 100644 --- a/choco/nunit-console-runner.nuspec +++ b/choco/nunit-console-runner.nuspec @@ -29,7 +29,6 @@ - @@ -49,7 +48,6 @@ - @@ -60,7 +58,6 @@ - @@ -71,7 +68,6 @@ - @@ -82,7 +78,6 @@ - @@ -93,6 +88,5 @@ - diff --git a/choco/nunit.console.choco.addins b/choco/nunit.console.choco.addins deleted file mode 100644 index d07fb7a21..000000000 --- a/choco/nunit.console.choco.addins +++ /dev/null @@ -1,7 +0,0 @@ -# Extensions built for a single runtime target -../../nunit-extension-*/**/tools/ # nuget v2 layout -../../../nunit-extension-*/**/tools/ # nuget v3 layout - -# Extensions built for multiple targets -../../nunit-extension-*/**/tools/*/ # nuget v2 layout -../../../nunit-extension-*/**/tools/*/ # nuget v3 layout diff --git a/choco/nunit.console.choco.agent.addins b/choco/nunit.console.choco.agent.addins deleted file mode 100644 index d326d241b..000000000 --- a/choco/nunit.console.choco.agent.addins +++ /dev/null @@ -1,7 +0,0 @@ -# Extensions built for a single runtime target -../../../../nunit-extension-*/tools/ # find extensions installed under chocolatey -../../../../../nunit-extension-*/tools/ # find extensions installed under chocolatey - -# Extensions built for multiple targets -../../../../nunit-extension-*/tools/*/ # find extensions installed under chocolatey -../../../../../nunit-extension-*/tools/*/ # find extensions installed under chocolatey diff --git a/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionManagerTests.cs b/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionManagerTests.cs index dc3e8e12d..ccf1382dd 100644 --- a/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionManagerTests.cs +++ b/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionManagerTests.cs @@ -12,7 +12,7 @@ using NUnit.Engine.Internal.FileSystemAccess; using System.Diagnostics; -namespace NUnit.Engine.Services +namespace NUnit.Engine.Services.Tests { public class ExtensionManagerTests { diff --git a/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionServiceTests.cs b/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionServiceTests.cs index 3f6a5664f..9b8f3bc21 100644 --- a/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionServiceTests.cs +++ b/src/NUnitEngine/nunit.engine.core.tests/Services/ExtensionServiceTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Text; using System.Threading.Tasks; @@ -51,12 +52,12 @@ public void CreateService() [Test] public void StartServiceInitializesExtensionManager() { - var workingDir = AssemblyHelper.GetDirectoryName(typeof(ExtensionService).Assembly); + Assembly hostAssembly = typeof(ExtensionService).Assembly; _extensionService.StartService(); _extensionManager.ReceivedWithAnyArgs().FindExtensionPoints(typeof(ExtensionService).Assembly, typeof(ITestEngine).Assembly); - _extensionManager.Received().FindExtensions(workingDir); + _extensionManager.Received().FindStandardExtensions(hostAssembly); Assert.That(_extensionService.Status, Is.EqualTo(ServiceStatus.Started)); } diff --git a/src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs b/src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs index 731ec1ab7..5becf2a7d 100644 --- a/src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs +++ b/src/NUnitEngine/nunit.engine.core/Services/ExtensionManager.cs @@ -16,8 +16,6 @@ namespace NUnit.Engine.Services public class ExtensionManager : IExtensionManager { static readonly Logger log = InternalTrace.GetLogger(typeof(ExtensionManager)); - static readonly Assembly THIS_ASSEMBLY = Assembly.GetExecutingAssembly(); - static readonly Version ENGINE_VERSION = THIS_ASSEMBLY.GetName().Version; private readonly IFileSystem _fileSystem; //private readonly IAddinsFileReader _addinsReader; @@ -138,15 +136,18 @@ public void FindExtensions(string startDir) } /// - /// Find and install extensions starting from a given base directory, - /// and using the provided list of patterns to direct the search using - /// a built-in algorithm. + /// Find and install standard extensions for a host assembly using + /// a built-in algorithm that searches in certain known locations. /// - /// Path to the initial directory. - /// A list of patterns used to identify potential candidates. - public void FindExtensions(string startDir, string[] patterns) + /// An assembly that supports extensions. + public void FindStandardExtensions(Assembly hostAssembly) { - FindExtensionAssemblies(_fileSystem.GetDirectory(startDir), patterns); + bool isChocolateyPackage = System.IO.File.Exists(Path.Combine(hostAssembly.Location, "VERIFICATION.txt")); + string[] extensionPatterns = isChocolateyPackage + ? new[] { "nunit-extension-*/**/tools/", "nunit-extension-*/**/tools/*/" } + : new[] { "NUnit.Extension.*/**/tools/", "NUnit.Extension.*/**/tools/*/" }; + + FindExtensionAssemblies(hostAssembly, extensionPatterns); foreach (var candidate in _assemblies) FindExtensionsInAssembly(candidate); @@ -265,16 +266,15 @@ private void FindExtensionAssemblies(IDirectory initialDirectory) } /// - /// Find candidate extension assemblies starting from a given base directory - /// and using the provided list of patterns to direct the search using + /// Find candidate extension assemblies for a given host assembly, + /// using the provided list of patterns to direct the search using /// a built-in algorithm. /// - /// Path to the initial directory. + /// An assembly that supports extensions /// A list of patterns used to identify potential candidates. - private void FindExtensionAssemblies(IDirectory initialDirectory, string[] patterns) + private void FindExtensionAssemblies(Assembly hostAssembly, string[] patterns) { - // Start looking two levels above initial directory - var startDir = initialDirectory.Parent.Parent; + IDirectory startDir = _fileSystem.GetDirectory(AssemblyHelper.GetDirectoryName(hostAssembly)); while (startDir != null) { @@ -459,18 +459,29 @@ internal void FindExtensionsInAssembly(ExtensionAssembly assembly) } #endif - foreach (var type in assembly.MainModule.GetTypes()) + foreach (var extensionType in assembly.MainModule.GetTypes()) { - CustomAttribute extensionAttr = type.GetAttribute("NUnit.Engine.Extensibility.ExtensionAttribute"); + CustomAttribute extensionAttr = extensionType.GetAttribute("NUnit.Engine.Extensibility.ExtensionAttribute"); if (extensionAttr == null) continue; - object versionArg = extensionAttr.GetNamedArgument("EngineVersion"); - if (versionArg != null && new Version((string)versionArg) > ENGINE_VERSION) - continue; + // TODO: This is a remnant of older code. In principle, this should be generalized + // to something like "HostVersion". However, this can safely remain until + // we separate ExtensionManager into its own assembly. + string versionArg = extensionAttr.GetNamedArgument("EngineVersion") as string; + if (versionArg != null) + { + Assembly THIS_ASSEMBLY = Assembly.GetExecutingAssembly(); + Version ENGINE_VERSION = THIS_ASSEMBLY.GetName().Version; + if (new Version(versionArg) > ENGINE_VERSION) + { + log.Warning($" Ignoring {extensionType.Name}. It requires version {versionArg}."); + continue; + } + } - var node = new ExtensionNode(assembly.FilePath, assembly.AssemblyVersion, type.FullName, assemblyTargetFramework) + var node = new ExtensionNode(assembly.FilePath, assembly.AssemblyVersion, extensionType.FullName, assemblyTargetFramework) { Path = extensionAttr.GetNamedArgument("Path") as string, Description = extensionAttr.GetNamedArgument("Description") as string @@ -479,9 +490,9 @@ internal void FindExtensionsInAssembly(ExtensionAssembly assembly) object enabledArg = extensionAttr.GetNamedArgument("Enabled"); node.Enabled = enabledArg == null || (bool)enabledArg; - log.Info(" Found ExtensionAttribute on Type " + type.Name); + log.Info(" Found ExtensionAttribute on Type " + extensionType.Name); - foreach (var attr in type.GetAttributes("NUnit.Engine.Extensibility.ExtensionPropertyAttribute")) + foreach (var attr in extensionType.GetAttributes("NUnit.Engine.Extensibility.ExtensionPropertyAttribute")) { string name = attr.ConstructorArguments[0].Value as string; string value = attr.ConstructorArguments[1].Value as string; @@ -498,12 +509,12 @@ internal void FindExtensionsInAssembly(ExtensionAssembly assembly) ExtensionPoint ep; if (node.Path == null) { - ep = DeduceExtensionPointFromType(type); + ep = DeduceExtensionPointFromType(extensionType); if (ep == null) { string msg = string.Format( "Unable to deduce ExtensionPoint for Type {0}. Specify Path on ExtensionAttribute to resolve.", - type.FullName); + extensionType.FullName); throw new NUnitEngineException(msg); } @@ -517,7 +528,7 @@ internal void FindExtensionsInAssembly(ExtensionAssembly assembly) { string msg = string.Format( "Unable to locate ExtensionPoint for Type {0}. The Path {1} cannot be found.", - type.FullName, + extensionType.FullName, node.Path); throw new NUnitEngineException(msg); } diff --git a/src/NUnitEngine/nunit.engine.core/Services/ExtensionService.cs b/src/NUnitEngine/nunit.engine.core/Services/ExtensionService.cs index d3dba41b4..7459ed8ca 100644 --- a/src/NUnitEngine/nunit.engine.core/Services/ExtensionService.cs +++ b/src/NUnitEngine/nunit.engine.core/Services/ExtensionService.cs @@ -19,13 +19,6 @@ namespace NUnit.Engine.Services /// public class ExtensionService : Service, IExtensionService { - private static readonly Assembly THIS_ASSEMBLY = typeof(ExtensionService).Assembly; - - private static readonly string[] CHOCO_PATTERNS = new[] { - "nunit-extension-*/**/tools/", "nunit-extension-*/**/tools/*/" }; - private static readonly string[] NUGET_PATTERNS = new[] { - "NUnit.Extension.*/**/tools/", "NUnit.Extension.*/**/tools/*/" }; - private readonly IExtensionManager _extensionManager; public ExtensionService() @@ -83,16 +76,13 @@ public void EnableExtension(string typeName, bool enabled) public override void StartService() { + Assembly thisAssembly = Assembly.GetExecutingAssembly(); + Assembly apiAssembly = typeof(ITestEngine).Assembly; + try { - _extensionManager.FindExtensionPoints( - Assembly.GetExecutingAssembly(), - typeof(ITestEngine).Assembly); - - var initialDirectory = AssemblyHelper.GetDirectoryName(THIS_ASSEMBLY); - bool isChocolateyPackage = System.IO.File.Exists(Path.Combine(THIS_ASSEMBLY.Location, "VERIFICATION.txt")); - - _extensionManager.FindExtensions(initialDirectory, isChocolateyPackage ? CHOCO_PATTERNS : NUGET_PATTERNS); + _extensionManager.FindExtensionPoints(thisAssembly, apiAssembly); + _extensionManager.FindStandardExtensions(thisAssembly); Status = ServiceStatus.Started; } diff --git a/src/NUnitEngine/nunit.engine.core/Services/IExtensionManager.cs b/src/NUnitEngine/nunit.engine.core/Services/IExtensionManager.cs index d688eb328..c6e9ebd63 100644 --- a/src/NUnitEngine/nunit.engine.core/Services/IExtensionManager.cs +++ b/src/NUnitEngine/nunit.engine.core/Services/IExtensionManager.cs @@ -23,13 +23,11 @@ public interface IExtensionManager : IDisposable void FindExtensions(string initialDirectory); /// - /// Find and install extensions starting from a given base directory, - /// and using the provided list of patterns to direct the search using - /// a built-in algorithm. + /// Find and install standard extensions for a host assembly using + /// a built-in algorithm that searches in certain known locations. /// - /// Path to the initial directory. - /// A list of patterns used to identify potential candidates. - void FindExtensions(string initialDirectory, string[] patterns); + /// An assembly that supports NUnit extensions. + void FindStandardExtensions(Assembly hostAssembly); IExtensionPoint GetExtensionPoint(string path);