From f9d61b59b8fe43800f474c45ec37fc3efa2d60ab Mon Sep 17 00:00:00 2001 From: Brad Wilson Date: Sat, 17 Jun 2023 17:12:30 -0700 Subject: [PATCH] #317: Slow performance on discovery / running due to discovering TestReporters --- Versions.props | 2 +- .../Utility/AssemblyExtensions.cs | 7 +- src/xunit.runner.visualstudio/VsTestRunner.cs | 124 +++--------------- .../RunnerReporterTests.cs | 59 ++------- .../Utility/EnvironmentHelper.cs | 51 +++++++ .../VsTestRunnerTests.cs | 17 +++ 6 files changed, 104 insertions(+), 156 deletions(-) create mode 100644 test/test.xunit.runner.visualstudio/Utility/EnvironmentHelper.cs create mode 100644 test/test.xunit.runner.visualstudio/VsTestRunnerTests.cs diff --git a/Versions.props b/Versions.props index 50d24fc6..4cf15ae8 100644 --- a/Versions.props +++ b/Versions.props @@ -8,7 +8,7 @@ 3.6.133 5.0.0 1.0.0-alpha.160 - 2.5.0-pre.26 + 2.5.0-pre.32 diff --git a/src/xunit.runner.visualstudio/Utility/AssemblyExtensions.cs b/src/xunit.runner.visualstudio/Utility/AssemblyExtensions.cs index 7d92079e..5cf731d4 100644 --- a/src/xunit.runner.visualstudio/Utility/AssemblyExtensions.cs +++ b/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 diff --git a/src/xunit.runner.visualstudio/VsTestRunner.cs b/src/xunit.runner.visualstudio/VsTestRunner.cs index 7d970109..9a77c961 100644 --- a/src/xunit.runner.visualstudio/VsTestRunner.cs +++ b/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 assemblyFileNames) { var reporter = default(IRunnerReporter); - var availableReporters = new Lazy>(() => GetAvailableRunnerReporters(assemblyFileNames)); + var availableReporters = new Lazy>(() => 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 GetAvailableRunnerReporters(IReadOnlyList sources) + public static IReadOnlyList GetAvailableRunnerReporters( + LoggerHelper? logger, + IReadOnlyList sources) { -#if NETCOREAPP - // Combine all input libs and merge their contexts to find the potential reporters var result = new List(); - 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())); - } - } - catch - { - continue; - } + if (logger is not null) + foreach (var message in messages) + logger.LogWarning(message); } return result; -#else - var result = new List(); - 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 GetVsTestCases( diff --git a/test/test.xunit.runner.visualstudio/RunnerReporterTests.cs b/test/test.xunit.runner.visualstudio/RunnerReporterTests.cs index 38b56007..4f84394b 100644 --- a/test/test.xunit.runner.visualstudio/RunnerReporterTests.cs +++ b/test/test.xunit.runner.visualstudio/RunnerReporterTests.cs @@ -1,44 +1,20 @@ -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()); Assert.Equal(typeof(DefaultRunnerReporterWithTypes).AssemblyQualifiedName, runnerReporter.GetType().AssemblyQualifiedName); } @@ -46,44 +22,37 @@ public void WhenNotUsingAutoReporters_ChoosesDefault() [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()); - // 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()); - 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(); 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()); 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); - } } diff --git a/test/test.xunit.runner.visualstudio/Utility/EnvironmentHelper.cs b/test/test.xunit.runner.visualstudio/Utility/EnvironmentHelper.cs new file mode 100644 index 00000000..87ec8a0f --- /dev/null +++ b/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 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); + } + } +} diff --git a/test/test.xunit.runner.visualstudio/VsTestRunnerTests.cs b/test/test.xunit.runner.visualstudio/VsTestRunnerTests.cs new file mode 100644 index 00000000..c54d4ac4 --- /dev/null +++ b/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); + } +}