#317: Slow performance on discovery / running due to discovering TestReporters

Author: bradwilson

Versions.props

@@ -8,7 +8,7 @@
     <NerdbankGitVersioningVersion>3.6.133</NerdbankGitVersioningVersion>
     <NSubstituteVersion>5.0.0</NSubstituteVersion>
     <TunnelVisionLabsReferenceAssemblyAnnotatorVersion>1.0.0-alpha.160</TunnelVisionLabsReferenceAssemblyAnnotatorVersion>
-    <XunitVersion>2.5.0-pre.26</XunitVersion>
+    <XunitVersion>2.5.0-pre.32</XunitVersion>
   </PropertyGroup>
 
 </Project>

src/xunit.runner.visualstudio/Utility/AssemblyExtensions.cs

@@ -1,5 +1,3 @@
-#if NETFRAMEWORK
-
 using System;
 using System.IO;
 using System.Reflection;
@@ -8,6 +6,7 @@ internal static class AssemblyExtensions
 {
 	public static string? GetLocalCodeBase(this Assembly assembly)
 	{
+#if NETFRAMEWORK
 		string? codeBase = assembly.CodeBase;
 		if (codeBase == null)
 			return null;
@@ -20,7 +19,9 @@ internal static class AssemblyExtensions
 			return "/" + codeBase;
 
 		return codeBase.Replace('/', Path.DirectorySeparatorChar);
+#else
+		return assembly.Location;
+#endif
 	}
 }
 
-#endif

src/xunit.runner.visualstudio/VsTestRunner.cs

@@ -4,7 +4,6 @@
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
-using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Threading;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
@@ -608,19 +607,19 @@ void handler()
 		}
 
 		public static IRunnerReporter GetRunnerReporter(
-			LoggerHelper logger,
+			LoggerHelper? logger,
 			RunSettings runSettings,
 			IReadOnlyList<string> assemblyFileNames)
 		{
 			var reporter = default(IRunnerReporter);
-			var availableReporters = new Lazy<IReadOnlyList<IRunnerReporter>>(() => GetAvailableRunnerReporters(assemblyFileNames));
+			var availableReporters = new Lazy<IReadOnlyList<IRunnerReporter>>(() => GetAvailableRunnerReporters(logger, assemblyFileNames));
 
 			try
 			{
 				if (!string.IsNullOrEmpty(runSettings.ReporterSwitch))
 				{
 					reporter = availableReporters.Value.FirstOrDefault(r => string.Equals(r.RunnerSwitch, runSettings.ReporterSwitch, StringComparison.OrdinalIgnoreCase));
-					if (reporter is null)
+					if (reporter is null && logger is not null)
 						logger.LogWarning("Could not find requested reporter '{0}'", runSettings.ReporterSwitch);
 				}
 
@@ -632,119 +631,30 @@ public static IRunnerReporter GetRunnerReporter(
 			return reporter ?? new DefaultRunnerReporterWithTypes();
 		}
 
-		static IReadOnlyList<IRunnerReporter> GetAvailableRunnerReporters(IReadOnlyList<string> sources)
+		public static IReadOnlyList<IRunnerReporter> GetAvailableRunnerReporters(
+			LoggerHelper? logger,
+			IReadOnlyList<string> sources)
 		{
-#if NETCOREAPP
-			// Combine all input libs and merge their contexts to find the potential reporters
 			var result = new List<IRunnerReporter>();
-			var dcjr = new DependencyContextJsonReader();
-			var deps =
-				sources
-					.Select(Path.GetFullPath)
-					.Select(s => s.Replace(".dll", ".deps.json"))
-					.Where(File.Exists)
-					.Select(f => new MemoryStream(Encoding.UTF8.GetBytes(File.ReadAllText(f))))
-					.Select(dcjr.Read);
-			var ctx = deps.Aggregate(DependencyContext.Default, (context, dependencyContext) => context.Merge(dependencyContext));
-			dcjr.Dispose();
-
-			var depsAssms = ctx.GetRuntimeAssemblyNames(InternalRuntimeEnvironment.GetRuntimeIdentifier()).ToList();
-
-			// Make sure to also check assemblies within the directory of the sources
-			var dllsInSources =
+
+			// We need to combine the source folders with our folder to find all potential runners
+			var folders =
 				sources
-					.Select(Path.GetFullPath)
-					.Select(Path.GetDirectoryName)
-					.Distinct(StringComparer.OrdinalIgnoreCase)
+					.Select(s => Path.GetDirectoryName(Path.GetFullPath(s)))
 					.WhereNotNull()
-					.SelectMany(p => Directory.GetFiles(p, "*.dll").Select(f => Path.Combine(p, f)))
-					.Select(f => new AssemblyName { Name = Path.GetFileNameWithoutExtension(f) })
-					.ToList();
+					.Concat(new[] { Path.GetDirectoryName(typeof(VsTestRunner).Assembly.GetLocalCodeBase()) })
+					.Distinct();
 
-			foreach (var assemblyName in depsAssms.Concat(dllsInSources))
+			foreach (var folder in folders)
 			{
-				try
-				{
-					var assembly = Assembly.Load(assemblyName);
-					foreach (var type in assembly.DefinedTypes)
-					{
-#pragma warning disable CS0618
-						if (type == null || type.IsAbstract || type == typeof(DefaultRunnerReporter).GetTypeInfo() || type == typeof(DefaultRunnerReporterWithTypes).GetTypeInfo() || type.ImplementedInterfaces.All(i => i != typeof(IRunnerReporter)))
-							continue;
-#pragma warning restore CS0618
-
-						var ctor = type.DeclaredConstructors.FirstOrDefault(c => c.GetParameters().Length == 0);
-						if (ctor == null)
-						{
-							ConsoleHelper.SetForegroundColor(ConsoleColor.Yellow);
-							Console.WriteLine($"Type {type.FullName} in assembly {assembly} appears to be a runner reporter, but does not have an empty constructor.");
-							ConsoleHelper.ResetColor();
-							continue;
-						}
+				result.AddRange(RunnerReporterUtility.GetAvailableRunnerReporters(folder, out var messages));
 
-						result.Add((IRunnerReporter)ctor.Invoke(Array.Empty<object>()));
-					}
-				}
-				catch
-				{
-					continue;
-				}
+				if (logger is not null)
+					foreach (var message in messages)
+						logger.LogWarning(message);
 			}
 
 			return result;
-#else
-			var result = new List<IRunnerReporter>();
-			var runnerPath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().GetLocalCodeBase());
-			var runnerReporterInterfaceAssemblyFullName = typeof(IRunnerReporter).Assembly.GetName().FullName;
-
-			if (runnerPath != null)
-				foreach (var dllFile in Directory.GetFiles(runnerPath, "*.dll").Select(f => Path.Combine(runnerPath, f)))
-				{
-					Type?[] types;
-
-					try
-					{
-						var assembly = Assembly.LoadFile(dllFile);
-
-						// Calling Assembly.GetTypes can be very expensive, while Assembly.GetReferencedAssemblies
-						// is relatively cheap.  We can avoid loading types for assemblies that couldn't possibly
-						// reference IRunnerReporter.
-						if (!assembly.GetReferencedAssemblies().Where(name => name.FullName == runnerReporterInterfaceAssemblyFullName).Any())
-							continue;
-
-						types = assembly.GetTypes();
-					}
-					catch (ReflectionTypeLoadException ex)
-					{
-						types = ex.Types;
-					}
-					catch
-					{
-						continue;
-					}
-
-					foreach (var type in types)
-					{
-#pragma warning disable CS0618
-						if (type == null || type.IsAbstract || type == typeof(DefaultRunnerReporter) || type == typeof(DefaultRunnerReporterWithTypes) || !type.GetInterfaces().Any(t => t == typeof(IRunnerReporter)))
-							continue;
-#pragma warning restore CS0618
-
-						var ctor = type.GetConstructor(new Type[0]);
-						if (ctor == null)
-						{
-							ConsoleHelper.SetForegroundColor(ConsoleColor.Yellow);
-							Console.WriteLine($"Type {type.FullName} in assembly {dllFile} appears to be a runner reporter, but does not have an empty constructor.");
-							ConsoleHelper.ResetColor();
-							continue;
-						}
-
-						result.Add((IRunnerReporter)ctor.Invoke(new object[0]));
-					}
-				}
-
-			return result;
-#endif
 		}
 
 		static IList<DiscoveredTestCase> GetVsTestCases(

test/test.xunit.runner.visualstudio/RunnerReporterTests.cs

@@ -1,89 +1,58 @@
-using System.ComponentModel;
+using System;
 using System.Diagnostics;
-using System.Reflection;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
 using NSubstitute;
 using Xunit;
-using Xunit.Abstractions;
+using Xunit.Runner.Reporters;
 using Xunit.Runner.VisualStudio;
 
 public class RunnerReporterTests
 {
-	public class TestRunnerReporterNotEnabled : IRunnerReporter
-	{
-		string IRunnerReporter.Description
-			=> "Not auto-enabled runner";
-
-		bool IRunnerReporter.IsEnvironmentallyEnabled
-			=> false;
-
-		string IRunnerReporter.RunnerSwitch
-			=> "notautoenabled";
-
-		IMessageSink IRunnerReporter.CreateMessageHandler(IRunnerLogger logger)
-			=> new NullMessageSink();
-	}
-
-	public class TestRunnerReporter : TestRunnerReporterNotEnabled, IRunnerReporter
-	{
-		bool IRunnerReporter.IsEnvironmentallyEnabled
-			=> true;
-
-		string IRunnerReporter.RunnerSwitch
-			=> null;
-	}
-
 	[Fact]
 	public void WhenNotUsingAutoReporters_ChoosesDefault()
 	{
+		using var _ = EnvironmentHelper.NullifyEnvironmentalReporters();
 		var settings = new RunSettings { NoAutoReporters = true };
 
-		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, new[] { Assembly.GetExecutingAssembly().Location });
+		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, Array.Empty<string>());
 
 		Assert.Equal(typeof(DefaultRunnerReporterWithTypes).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
 	}
 
 	[Fact]
 	public void WhenUsingAutoReporters_DoesNotChooseDefault()
 	{
+		using var _ = EnvironmentHelper.NullifyEnvironmentalReporters();
+		Environment.SetEnvironmentVariable("TEAMCITY_PROJECT_NAME", "foo");  // Force TeamCityReporter to surface environmentally
 		var settings = new RunSettings { NoAutoReporters = false };
 
-		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, new[] { Assembly.GetExecutingAssembly().Location });
+		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, Array.Empty<string>());
 
-		// We just make sure _an_ auto-reporter was chosen, but we can't rely on which one because this code
-		// wil run when we're in CI, and therefore will choose the CI reporter sometimes. It's good enough
-		// that we've provide an option above so that the default never gets chosen.
-		Assert.NotEqual(typeof(DefaultRunnerReporterWithTypes).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
+		Assert.Equal(typeof(TeamCityReporter).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
 	}
 
 	[Fact]
 	public void WhenUsingReporterSwitch_PicksThatReporter()
 	{
-		var settings = new RunSettings { NoAutoReporters = true, ReporterSwitch = "notautoenabled" };
+		using var _ = EnvironmentHelper.NullifyEnvironmentalReporters();
+		var settings = new RunSettings { NoAutoReporters = true, ReporterSwitch = "json" };
 
-		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, new[] { Assembly.GetExecutingAssembly().Location });
+		var runnerReporter = VsTestRunner.GetRunnerReporter(null, settings, Array.Empty<string>());
 
-		Assert.Equal(typeof(TestRunnerReporterNotEnabled).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
+		Assert.Equal(typeof(JsonReporter).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
 	}
 
 	[Fact]
 	public void WhenRequestedReporterDoesntExist_LogsAndFallsBack()
 	{
+		using var _ = EnvironmentHelper.NullifyEnvironmentalReporters();
 		var settings = new RunSettings { NoAutoReporters = true, ReporterSwitch = "thisnotavalidreporter" };
 		var logger = Substitute.For<IMessageLogger>();
 		var loggerHelper = new LoggerHelper(logger, new Stopwatch());
 
-		var runnerReporter = VsTestRunner.GetRunnerReporter(loggerHelper, settings, new[] { Assembly.GetExecutingAssembly().Location });
+		var runnerReporter = VsTestRunner.GetRunnerReporter(loggerHelper, settings, Array.Empty<string>());
 
 		Assert.Equal(typeof(DefaultRunnerReporterWithTypes).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName);
 		logger.Received(1).SendMessage(TestMessageLevel.Warning, "[xUnit.net 00:00:00.00] Could not find requested reporter 'thisnotavalidreporter'");
 	}
-
-	[Fact]
-	public void VSTestRunnerShouldHaveCategoryAttribute_WithValueManaged()
-	{
-		var attribute = typeof(VsTestRunner).GetCustomAttribute(typeof(CategoryAttribute));
-		Assert.NotNull(attribute);
-		Assert.Equal("managed", (attribute as CategoryAttribute)?.Category);
-	}
 }

test/test.xunit.runner.visualstudio/Utility/EnvironmentHelper.cs

@@ -0,0 +1,51 @@
+#nullable enable
+
+using System;
+using System.Collections.Generic;
+
+static class EnvironmentHelper
+{
+	static readonly string[] reporterEnvironmentVariables =
+	{
+		// AppVeyorReporter
+		"APPVEYOR_API_URL",
+		// TeamCityReporter
+		"TEAMCITY_PROJECT_NAME",
+		"TEAMCITY_PROCESS_FLOW_ID",
+		// VstsReporter
+		"VSTS_ACCESS_TOKEN",
+		"SYSTEM_TEAMFOUNDATIONCOLLECTIONURI",
+		"SYSTEM_TEAMPROJECT",
+		"BUILD_BUILDID",
+	};
+
+	public static IDisposable NullifyEnvironmentalReporters()
+	{
+		var result = new EnvironmentRestorer(reporterEnvironmentVariables);
+
+		foreach (var variable in reporterEnvironmentVariables)
+			Environment.SetEnvironmentVariable(variable, null);
+
+		return result;
+	}
+
+	public static IDisposable RestoreEnvironment(params string[] variables) =>
+		new EnvironmentRestorer(variables);
+
+	class EnvironmentRestorer : IDisposable
+	{
+		Dictionary<string, string?> savedVariables = new();
+
+		public EnvironmentRestorer(string[] variables)
+		{
+			foreach (var variable in variables)
+				savedVariables[variable] = Environment.GetEnvironmentVariable(variable);
+		}
+
+		public void Dispose()
+		{
+			foreach (var kvp in savedVariables)
+				Environment.SetEnvironmentVariable(kvp.Key, kvp.Value);
+		}
+	}
+}

test/test.xunit.runner.visualstudio/VsTestRunnerTests.cs

@@ -0,0 +1,17 @@
+using System.ComponentModel;
+using System.Reflection;
+using Xunit;
+using Xunit.Runner.VisualStudio;
+
+public class VsTestRunnerTests
+{
+
+	[Fact]
+	public void VSTestRunnerShouldHaveCategoryAttribute_WithValueManaged()
+	{
+		var attribute = typeof(VsTestRunner).GetCustomAttribute(typeof(CategoryAttribute));
+
+		Assert.NotNull(attribute);
+		Assert.Equal("managed", (attribute as CategoryAttribute)?.Category);
+	}
+}