Merge pull request #1755 from nunit/issue-1754

Don't throw for unknown extensions; other API changes

Author: CharliePoole

src/NUnitCommon/nunit.extensibility.api/IExtensionNode.cs

@@ -9,6 +9,18 @@ namespace NUnit.Extensibility
     /// The IExtensionNode interface is implemented by a class that represents a
     /// single extension being installed on a particular extension point.
     /// </summary>
+    public enum ExtensionStatus
+    {
+        /// <summary>Extension is not yet loaded</summary>
+        Unloaded,
+        /// <summary>Extension has been loaded</summary>
+        Loaded,
+        /// <summary>An error occurred trying to load the extension</summary>
+        Error,
+        /// <summary>Extension without a corresponding path./summary>
+        Unknown,
+    }
+
     public interface IExtensionNode
     {
         /// <summary>
@@ -22,6 +34,16 @@ public interface IExtensionNode
         /// <value><c>true</c> if enabled; otherwise, <c>false</c>.</value>
         bool Enabled { get; }
 
+        /// <summary>
+        /// Status of this extension.
+        /// </summary>
+        ExtensionStatus Status { get; }
+
+        /// <summary>
+        /// Exception thrown in creating the ExtensionObject, if Status is error, otherwise null.
+        /// </summary>
+        Exception? Exception { get; }
+
         /// <summary>
         /// Gets the unique string identifying the ExtensionPoint for which
         /// this Extension is intended. This identifier may be supplied by the attribute
@@ -39,14 +61,6 @@ public interface IExtensionNode
         /// </summary>
         IEnumerable<string> PropertyNames { get; }
 
-        /// <summary>
-        /// Gets a collection of the values of a particular named property
-        /// If none are present, returns an empty enumerator.
-        /// </summary>
-        /// <param name="name">The property name</param>
-        /// <returns>A collection of values</returns>
-        IEnumerable<string> GetValues(string name);
-
         /// <summary>
         /// The path to the assembly implementing this extension.
         /// </summary>
@@ -56,5 +70,13 @@ public interface IExtensionNode
         /// The version of the assembly implementing this extension.
         /// </summary>
         Version AssemblyVersion { get; }
+
+        /// <summary>
+        /// Gets a collection of the values of a particular named property.
+        /// If none are present, returns an empty enumerator.
+        /// </summary>
+        /// <param name="name">The property name</param>
+        /// <returns>A collection of values</returns>
+        IEnumerable<string> GetValues(string name);
     }
 }

src/NUnitCommon/nunit.extensibility.tests/ExtensionManagerTests.cs

@@ -8,6 +8,7 @@
 using System.IO;
 using System.Reflection;
 using System.Collections.Generic;
+using System.Net;
 
 namespace NUnit.Extensibility
 {
@@ -18,9 +19,11 @@ public class ExtensionManagerTests
         private static readonly string THIS_ASSEMBLY_DIRECTORY = Path.GetDirectoryName(THIS_ASSEMBLY.Location)!;
         private const string FAKE_EXTENSIONS_FILENAME = "FakeExtensions.dll";
         private static readonly string FAKE_EXTENSIONS_PARENT_DIRECTORY =
-            Path.Combine(new DirectoryInfo(THIS_ASSEMBLY_DIRECTORY).Parent!.FullName, "fakesv2");
-        private static readonly string FAKE_EXTENSIONS_SOURCE_DIRECTORY =
-            Path.Combine(new DirectoryInfo(THIS_ASSEMBLY_DIRECTORY).Parent!.Parent!.Parent!.FullName, "src/TestData/FakeExtensions");
+#if NETFRAMEWORK
+            Path.Combine(new DirectoryInfo(THIS_ASSEMBLY_DIRECTORY).Parent!.FullName, "fakesv2/net462");
+#else
+            Path.Combine(new DirectoryInfo(THIS_ASSEMBLY_DIRECTORY).Parent!.FullName, "fakesv2/netstandard2.0");
+#endif
 
         private const string FAKE_AGENT_LAUNCHER_EXTENSION = "NUnit.Engine.Fakes.FakeAgentLauncherExtension";
         private const string FAKE_FRAMEWORK_DRIVER_EXTENSION = "NUnit.Engine.Fakes.FakeFrameworkDriverExtension";
@@ -30,6 +33,7 @@ public class ExtensionManagerTests
         private const string FAKE_SERVICE_EXTENSION = "NUnit.Engine.Fakes.FakeServiceExtension";
         private const string FAKE_DISABLED_EXTENSION = "NUnit.Engine.Fakes.FakeDisabledExtension";
         private const string FAKE_NUNIT_V2_DRIVER_EXTENSION = "NUnit.Engine.Fakes.V2DriverExtension";
+        private const string FAKE_EXTENSION_WITH_NO_EXTENSION_POINT = "NUnit.Engine.Fakes.FakeExtension_NoExtensionPointFound";
 
         private readonly string[] KnownExtensions =
         {
@@ -39,8 +43,9 @@ public class ExtensionManagerTests
             FAKE_RESULT_WRITER_EXTENSION,
             FAKE_EVENT_LISTENER_EXTENSION,
             FAKE_SERVICE_EXTENSION,
-            FAKE_DISABLED_EXTENSION
-            //FAKE_NUNIT_V2_DRIVER_EXTENSION
+            FAKE_DISABLED_EXTENSION,
+            //FAKE_NUNIT_V2_DRIVER_EXTENSION,
+            FAKE_EXTENSION_WITH_NO_EXTENSION_POINT
         };
 
         private ExtensionManager _extensionManager;
@@ -94,7 +99,7 @@ public void CreateExtensionManager()
             _extensionManager.FindExtensionPoints(typeof(ITestEngine).Assembly);
 
             // Find Fake Extensions using alternate start directory
-            _extensionManager.FindExtensionAssemblies(FAKE_EXTENSIONS_SOURCE_DIRECTORY);
+            _extensionManager.FindExtensionAssemblies(FAKE_EXTENSIONS_PARENT_DIRECTORY);
             _extensionManager.LoadExtensions();
         }
 
@@ -121,6 +126,18 @@ public void AllExtensionsUseTheLatestVersion()
                 Assert.That(node.AssemblyVersion.ToString(), Is.EqualTo("2.0.0.0"));
         }
 
+        [Test]
+        public void AllExtensionsHaveCorrectStatus()
+        {
+            foreach (var node in _extensionManager.Extensions)
+            {
+                var expectedStatus = node.TypeName == FAKE_EXTENSION_WITH_NO_EXTENSION_POINT
+                    ? ExtensionStatus.Unknown
+                    : ExtensionStatus.Unloaded;
+                Assert.That(node.Status, Is.EqualTo(expectedStatus));
+            }
+        }
+
         [Test]
         public void AllKnownExtensionsAreEnabledAsRequired()
         {
@@ -216,20 +233,20 @@ public void SkipsGracefullyLoadingOtherFrameworkExtensionAssembly()
 
 #if NETCOREAPP
         [TestCase("netstandard2.0", ExpectedResult = true)]
-        [TestCase("net462", ExpectedResult = false)]
+        //[TestCase("net462", ExpectedResult = false)]
         //[TestCase("net20", ExpectedResult = false)]
 #elif NET40_OR_GREATER
-        [TestCase("netstandard2.0", ExpectedResult = false)]
+        //[TestCase("netstandard2.0", ExpectedResult = false)]
         [TestCase("net462", ExpectedResult = true)]
         //[TestCase("net20", ExpectedResult = true)]
 #else
-        [TestCase("netstandard2.0", ExpectedResult = false)]
-        [TestCase("net462", ExpectedResult = false)]
+        //[TestCase("netstandard2.0", ExpectedResult = false)]
+        //[TestCase("net462", ExpectedResult = false)]
         //[TestCase("net20", ExpectedResult = true)]
 #endif
         public bool LoadTargetFramework(string tfm)
         {
-            return _extensionManager.CanLoadTargetFramework(THIS_ASSEMBLY, FakeExtensions(tfm));
+            return _extensionManager.CanLoadTargetFramework(THIS_ASSEMBLY, FakeExtensions());
         }
 
         //[TestCaseSource(nameof(ValidCombos))]
@@ -351,10 +368,10 @@ private static string GetSiblingDirectory(string dir)
         /// assembly based on the argument provided.
         /// </summary>
         /// <param name="tfm">A test framework moniker. Must be one for which the fake extensions are built.</param>
-        private static ExtensionAssembly FakeExtensions(string tfm)
+        private static ExtensionAssembly FakeExtensions()
         {
             return new ExtensionAssembly(
-                Path.Combine(FAKE_EXTENSIONS_PARENT_DIRECTORY, Path.Combine(tfm, FAKE_EXTENSIONS_FILENAME)), false);
+                Path.Combine(FAKE_EXTENSIONS_PARENT_DIRECTORY, FAKE_EXTENSIONS_FILENAME), false);
         }
     }
 }

src/NUnitCommon/nunit.extensibility/ExtensionManager.cs

@@ -547,25 +547,19 @@ public void FindExtensionsInAssembly(ExtensionAssembly extensionAssembly)
 
                 _extensions.Add(node);
 
-                ExtensionPoint? ep;
-                if (extensionAttrPath is null)
-                {
-                    ep = DeduceExtensionPointFromType(extensionType);
-                    if (ep is null)
-                        throw new ExtensibilityException($"Unable to deduce ExtensionPoint for Type {extensionType.FullName}. Specify Path on ExtensionAttribute to resolve.");
+                ExtensionPoint? ep = extensionAttrPath is not null
+                    ? GetExtensionPoint(extensionAttrPath)
+                    : DeduceExtensionPointFromType(extensionType);
 
-                    node.Path = ep.Path;
-                }
-                else
+                if (ep is null)
                 {
-                    node.Path = extensionAttrPath;
-
-                    // TODO: Remove need for the cast
-                    ep = GetExtensionPoint(node.Path) as ExtensionPoint;
-                    if (ep is null)
-                        throw new ExtensibilityException($"Unable to locate ExtensionPoint for Type {extensionType.FullName}. The Path {node.Path} cannot be found.");
+                    log.Warning($"Extension ignored - Unable to deduce ExtensionPoint.");
+                    node.Status = ExtensionStatus.Unknown;
+                    node.Exception = new Exception("Unable to deduce ExtensionPoint");
+                    continue;
                 }
 
+                node.Path = ep.Path;
                 ep.Install(node);
             }
         }
@@ -619,28 +613,6 @@ public bool CanLoadTargetFramework(Assembly? runnerAsm, ExtensionAssembly extens
                             return false;
                     }
             }
-
-            //string extensionFrameworkName = AssemblyDefinition.ReadAssembly(extensionAsm.FilePath).GetFrameworkName();
-            //string runnerFrameworkName = AssemblyDefinition.ReadAssembly(runnerAsm.Location).GetFrameworkName();
-            //if (runnerFrameworkName?.StartsWith(".NETStandard") == true)
-            //{
-            //    throw new NUnitEngineException($"{runnerAsm.FullName} test runner must target .NET Core or .NET Framework, not .NET Standard");
-            //}
-            //else if (runnerFrameworkName?.StartsWith(".NETCoreApp") == true)
-            //{
-            //    if (extensionFrameworkName?.StartsWith(".NETStandard") != true && extensionFrameworkName?.StartsWith(".NETCoreApp") != true)
-            //    {
-            //        log.Info($".NET Core runners require .NET Core or .NET Standard extension for {extensionAsm.FilePath}");
-            //        return false;
-            //    }
-            //}
-            //else if (extensionFrameworkName?.StartsWith(".NETCoreApp") == true)
-            //{
-            //    log.Info($".NET Framework runners cannot load .NET Core extension {extensionAsm.FilePath}");
-            //    return false;
-            //}
-
-            //return true;
         }
 
         private static System.Runtime.Versioning.FrameworkName GetTargetRuntime(string filePath)

src/NUnitCommon/nunit.extensibility/ExtensionNode.cs

@@ -31,6 +31,7 @@ public ExtensionNode(ExtensionAssembly extensionAssembly, TypeDefinition extensi
             AssemblyPath = extensionAssembly.FilePath;
             AssemblyVersion = extensionAssembly.AssemblyVersion;
             TypeName = extensionType.FullName;
+            Status = ExtensionStatus.Unloaded;
             Enabled = true; // By default
         }
 
@@ -62,6 +63,16 @@ public ExtensionNode(ExtensionAssembly extensionAssembly, TypeDefinition extensi
         /// <value><c>true</c> if enabled; otherwise, <c>false</c>.</value>
         public bool Enabled { get; set; }
 
+        /// <summary>
+        /// Status of this extension
+        /// </summary>
+        public ExtensionStatus Status { get; set; }
+
+        /// <summary>
+        /// Exception thrown in creating the ExtensionObject, if Status is error, otherwise null.
+        /// </summary>
+        public Exception? Exception { get; set; }
+
         /// <summary>
         /// Gets and sets the unique string identifying the ExtensionPoint for which
         /// this Extension is intended. This identifier may be supplied by the attribute
@@ -127,13 +138,18 @@ public object CreateExtensionObject(params object[] args)
             object obj = Activator.CreateInstance(type, args)!;
 #endif
 
+            // TODO: Determine whether to continue support for V3 extensions here,
+            // defer it to the engine or eliminate it entirely.
             return IsV3Extension
                 ? ExtensionWrapper.Wrap(obj, Path)
                 : obj;
         }
 
         #endregion
 
+        /// <summary>
+        /// Used by ExtensionManger to add a value to the node's properties collection.
+        /// </summary>
         public void AddProperty(string name, string val)
         {
             if (_properties.TryGetValue(name, out List<string>? list))
@@ -145,6 +161,9 @@ public void AddProperty(string name, string val)
             }
         }
 
+        /// <summary>
+        /// Gets the string representation of this node.
+        /// </summary>
         public override string ToString()
         {
             return $"{TypeName} - {Path}";