From df47e1e1506feb15d08bda05a165d7d7fddda621 Mon Sep 17 00:00:00 2001 From: Doug Bunting <6431421+dougbu@users.noreply.github.com> Date: Sun, 6 Mar 2022 17:41:40 -0800 Subject: [PATCH] !nits! Slight Testing cleanup - use more modern C# syntax - remove extra `AssemblyTestLog.Create(...)` overload - was just used in tests and other overload already more direct (and worth testing) - remove useless `ILoggedTest` interface - add solution filter and startvs.cmd file - make `LoggedTestBase` an `abstract` class - remove unused `using`s - shorten long Lines - also clean 30_api_proposal.md line endings --- .github/ISSUE_TEMPLATE/30_api_proposal.md | 112 +++++++++---------- src/Testing/Testing.slnf | 10 ++ src/Testing/src/AssemblyTestLog.cs | 8 +- src/Testing/src/LoggedTest/ILoggedTest.cs | 23 ---- src/Testing/src/LoggedTest/LoggedTestBase.cs | 2 +- src/Testing/startvs.cmd | 3 + src/Testing/test/AssemblyFixtureTest.cs | 4 +- src/Testing/test/AssemblyTestLogTests.cs | 66 +++++++---- src/Testing/test/Properties/AssemblyInfo.cs | 1 - 9 files changed, 122 insertions(+), 107 deletions(-) create mode 100644 src/Testing/Testing.slnf delete mode 100644 src/Testing/src/LoggedTest/ILoggedTest.cs create mode 100644 src/Testing/startvs.cmd diff --git a/.github/ISSUE_TEMPLATE/30_api_proposal.md b/.github/ISSUE_TEMPLATE/30_api_proposal.md index c5394c21793e..331141696404 100644 --- a/.github/ISSUE_TEMPLATE/30_api_proposal.md +++ b/.github/ISSUE_TEMPLATE/30_api_proposal.md @@ -1,56 +1,56 @@ ---- -name: API proposal -about: Propose a change to the public API surface -title: '' -labels: api-suggestion -assignees: '' - ---- - -## Background and Motivation - - - -## Proposed API - - - -## Usage Examples - - - -## Alternative Designs - - - -## Risks - - - +--- +name: API proposal +about: Propose a change to the public API surface +title: '' +labels: api-suggestion +assignees: '' + +--- + +## Background and Motivation + + + +## Proposed API + + + +## Usage Examples + + + +## Alternative Designs + + + +## Risks + + + diff --git a/src/Testing/Testing.slnf b/src/Testing/Testing.slnf new file mode 100644 index 000000000000..41ec46229562 --- /dev/null +++ b/src/Testing/Testing.slnf @@ -0,0 +1,10 @@ +{ + "solution": { + "path": "..\\..\\AspNetCore.sln", + "projects": [ + "src\\Analyzers\\Microsoft.AspNetCore.Analyzer.Testing\\src\\Microsoft.AspNetCore.Analyzer.Testing.csproj", + "src\\Testing\\src\\Microsoft.AspNetCore.Testing.csproj", + "src\\Testing\\test\\Microsoft.AspNetCore.Testing.Tests.csproj" + ] + } +} \ No newline at end of file diff --git a/src/Testing/src/AssemblyTestLog.cs b/src/Testing/src/AssemblyTestLog.cs index d979babbc74b..9ca775cd45ee 100644 --- a/src/Testing/src/AssemblyTestLog.cs +++ b/src/Testing/src/AssemblyTestLog.cs @@ -176,10 +176,6 @@ public IServiceProvider CreateLoggerServices(ITestOutputHelper output, string cl return serviceCollection.BuildServiceProvider(); } - // For back compat - public static AssemblyTestLog Create(string assemblyName, string baseDirectory) - => Create(Assembly.Load(new AssemblyName(assemblyName)), baseDirectory); - public static AssemblyTestLog Create(Assembly assembly, string baseDirectory) { var logStart = DateTimeOffset.UtcNow; @@ -229,7 +225,8 @@ public static AssemblyTestLog ForAssembly(Assembly assembly) // Try to clear previous logs, continue if it fails. var assemblyBaseDirectory = TestFileOutputContext.GetAssemblyBaseDirectory(assembly); - if (!string.IsNullOrEmpty(assemblyBaseDirectory) && !TestFileOutputContext.GetPreserveExistingLogsInOutput(assembly)) + if (!string.IsNullOrEmpty(assemblyBaseDirectory) && + !TestFileOutputContext.GetPreserveExistingLogsInOutput(assembly)) { try { @@ -267,6 +264,7 @@ private static SerilogLoggerProvider ConfigureFileLogging(string fileName, DateT .MinimumLevel.Verbose() .WriteTo.File(fileName, outputTemplate: "[{TimestampOffset}] [{SourceContext}] [{Level}] {Message:l}{NewLine}{Exception}", flushToDiskInterval: TimeSpan.FromSeconds(1), shared: true) .CreateLogger(); + return new SerilogLoggerProvider(serilogger, dispose: true); } diff --git a/src/Testing/src/LoggedTest/ILoggedTest.cs b/src/Testing/src/LoggedTest/ILoggedTest.cs deleted file mode 100644 index 1a3777eddcaa..000000000000 --- a/src/Testing/src/LoggedTest/ILoggedTest.cs +++ /dev/null @@ -1,23 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Reflection; -using Microsoft.Extensions.Logging; -using Xunit.Abstractions; - -namespace Microsoft.AspNetCore.Testing; - -public interface ILoggedTest : IDisposable -{ - ILogger Logger { get; } - - ILoggerFactory LoggerFactory { get; } - - ITestOutputHelper TestOutputHelper { get; } - - // For back compat - IDisposable StartLog(out ILoggerFactory loggerFactory, LogLevel minLogLevel, string testName); - - void Initialize(TestContext context, MethodInfo methodInfo, object[] testMethodArguments, ITestOutputHelper testOutputHelper); -} diff --git a/src/Testing/src/LoggedTest/LoggedTestBase.cs b/src/Testing/src/LoggedTest/LoggedTestBase.cs index 7309086cfca2..38459e6e284b 100644 --- a/src/Testing/src/LoggedTest/LoggedTestBase.cs +++ b/src/Testing/src/LoggedTest/LoggedTestBase.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Testing; -public class LoggedTestBase : ILoggedTest, ITestMethodLifecycle +public abstract class LoggedTestBase : ITestMethodLifecycle, IDisposable { private ExceptionDispatchInfo _initializationException; diff --git a/src/Testing/startvs.cmd b/src/Testing/startvs.cmd new file mode 100644 index 000000000000..3ca4ac5adbc9 --- /dev/null +++ b/src/Testing/startvs.cmd @@ -0,0 +1,3 @@ +@ECHO OFF + +%~dp0..\..\startvs.cmd %~dp0Testing.slnf diff --git a/src/Testing/test/AssemblyFixtureTest.cs b/src/Testing/test/AssemblyFixtureTest.cs index ae0fd8cb30cf..16c967402ace 100644 --- a/src/Testing/test/AssemblyFixtureTest.cs +++ b/src/Testing/test/AssemblyFixtureTest.cs @@ -10,7 +10,9 @@ namespace Microsoft.AspNetCore.Testing; [TestCaseOrderer("Microsoft.AspNetCore.Testing.AlphabeticalOrderer", "Microsoft.AspNetCore.Testing.Tests")] public class AssemblyFixtureTest { - public AssemblyFixtureTest(TestAssemblyFixture assemblyFixture, TestCollectionFixture collectionFixture) + public AssemblyFixtureTest( + TestAssemblyFixture assemblyFixture, + TestCollectionFixture collectionFixture) { AssemblyFixture = assemblyFixture; CollectionFixture = collectionFixture; diff --git a/src/Testing/test/AssemblyTestLogTests.cs b/src/Testing/test/AssemblyTestLogTests.cs index 9e9348114de7..c31c6e404b29 100644 --- a/src/Testing/test/AssemblyTestLogTests.cs +++ b/src/Testing/test/AssemblyTestLogTests.cs @@ -5,7 +5,6 @@ using System.IO; using System.Linq; using System.Reflection; -using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using System.Threading.Tasks; using Microsoft.AspNetCore.Testing; @@ -17,7 +16,11 @@ public class AssemblyTestLogTests : LoggedTest { private static readonly Assembly ThisAssembly = typeof(AssemblyTestLogTests).GetTypeInfo().Assembly; private static readonly string ThisAssemblyName = ThisAssembly.GetName().Name; - private static readonly string TFM = ThisAssembly.GetCustomAttributes().OfType().FirstOrDefault().TargetFramework; + private static readonly string TFM = ThisAssembly + .GetCustomAttributes() + .OfType() + .FirstOrDefault() + .TargetFramework; [Fact] public void FunctionalLogs_LogsPreservedFromNonQuarantinedTest() @@ -43,8 +46,8 @@ public void ForAssembly_ReturnsSameInstanceForSameAssembly() public void TestLogWritesToITestOutputHelper() { var output = new TestTestOutputHelper(); - var assemblyLog = AssemblyTestLog.Create(ThisAssemblyName, baseDirectory: null); + using var assemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: null); using (assemblyLog.StartTestLog(output, "NonExistant.Test.Class", out var loggerFactory)) { var logger = loggerFactory.CreateLogger("TestLogger"); @@ -69,11 +72,17 @@ public Task TestLogEscapesIllegalFileNames() => { var illegalTestName = "T:e/s//t"; var escapedTestName = "T_e_s_t"; - using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, baseDirectory: tempDir)) - using (testAssemblyLog.StartTestLog(output: null, className: "FakeTestAssembly.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, resolvedTestName: out var resolvedTestname, out var _, testName: illegalTestName)) - { - Assert.Equal(escapedTestName, resolvedTestname); - } + + using var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir); + using var disposable = testAssemblyLog.StartTestLog( + output: null, + className: "FakeTestAssembly.FakeTestClass", + loggerFactory: out var testLoggerFactory, + minLogLevel: LogLevel.Trace, + resolvedTestName: out var resolvedTestname, + out var _, + testName: illegalTestName); + Assert.Equal(escapedTestName, resolvedTestname); }); [Fact] @@ -84,11 +93,16 @@ public Task TestLogWritesToGlobalLogFile() => // but it's also testing the test logging facility. So this is pretty meta ;) var logger = LoggerFactory.CreateLogger("Test"); - using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir)) + using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir)) { logger.LogInformation("Created test log in {baseDirectory}", tempDir); - using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: "FakeTestName")) + using (testAssemblyLog.StartTestLog( + output: null, + className: $"{ThisAssemblyName}.FakeTestClass", + loggerFactory: out var testLoggerFactory, + minLogLevel: LogLevel.Trace, + testName: "FakeTestName")) { var testLogger = testLoggerFactory.CreateLogger("TestLogger"); testLogger.LogInformation("Information!"); @@ -126,15 +140,21 @@ public Task TestLogTruncatesTestNameToAvoidLongPaths() => { var longTestName = new string('0', 50) + new string('1', 50) + new string('2', 50) + new string('3', 50) + new string('4', 50); var logger = LoggerFactory.CreateLogger("Test"); - using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir)) + using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir)) { logger.LogInformation("Created test log in {baseDirectory}", tempDir); - using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: longTestName)) + using (testAssemblyLog.StartTestLog( + output: null, + className: $"{ThisAssemblyName}.FakeTestClass", + loggerFactory: out var testLoggerFactory, + minLogLevel: LogLevel.Trace, + testName: longTestName)) { testLoggerFactory.CreateLogger("TestLogger").LogInformation("Information!"); } } + logger.LogInformation("Finished test log in {baseDirectory}", tempDir); var testLogFiles = new DirectoryInfo(Path.Combine(tempDir, ThisAssemblyName, TFM, "FakeTestClass")).EnumerateFiles(); @@ -152,18 +172,24 @@ public Task TestLogEnumerateFilenamesToAvoidCollisions() => RunTestLogFunctionalTest((tempDir) => { var logger = LoggerFactory.CreateLogger("Test"); - using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir)) + using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir)) { logger.LogInformation("Created test log in {baseDirectory}", tempDir); for (var i = 0; i < 10; i++) { - using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: "FakeTestName")) + using (testAssemblyLog.StartTestLog( + output: null, + className: $"{ThisAssemblyName}.FakeTestClass", + loggerFactory: out var testLoggerFactory, + minLogLevel: LogLevel.Trace, + testName: "FakeTestName")) { testLoggerFactory.CreateLogger("TestLogger").LogInformation("Information!"); } } } + logger.LogInformation("Finished test log in {baseDirectory}", tempDir); // The first log file exists @@ -176,13 +202,13 @@ public Task TestLogEnumerateFilenamesToAvoidCollisions() => } }); - private static readonly Regex TimestampRegex = new Regex(@"\d+-\d+-\d+T\d+:\d+:\d+"); - private static readonly Regex TimestampOffsetRegex = new Regex(@"\d+\.\d+s"); - private static readonly Regex DurationRegex = new Regex(@"[^ ]+s$"); + private static readonly Regex TimestampRegex = new(@"\d+-\d+-\d+T\d+:\d+:\d+"); + private static readonly Regex TimestampOffsetRegex = new(@"\d+\.\d+s"); + private static readonly Regex DurationRegex = new(@"[^ ]+s$"); - private async Task RunTestLogFunctionalTest(Action action, [CallerMemberName] string testName = null) + private static async Task RunTestLogFunctionalTest(Action action) { - var tempDir = Path.Combine(Path.GetTempPath(), $"TestLogging_{Guid.NewGuid().ToString("N")}"); + var tempDir = Path.Combine(Path.GetTempPath(), $"TestLogging_{Guid.NewGuid():N}"); try { action(tempDir); @@ -209,7 +235,7 @@ private static string MakeConsistent(string input) return string.Join(Environment.NewLine, input.Split(new[] { Environment.NewLine }, StringSplitOptions.None) .Select(line => { - var strippedPrefix = line.IndexOf('[') >= 0 ? line.Substring(line.IndexOf('[')) : line; + var strippedPrefix = line.Contains('[') ? line.Substring(line.IndexOf('[')) : line; var strippedDuration = DurationRegex.Replace(strippedPrefix, "DURATION"); var strippedTimestamp = TimestampRegex.Replace(strippedDuration, "TIMESTAMP"); diff --git a/src/Testing/test/Properties/AssemblyInfo.cs b/src/Testing/test/Properties/AssemblyInfo.cs index 8ffcbfdf3084..b75aeb8cde8a 100644 --- a/src/Testing/test/Properties/AssemblyInfo.cs +++ b/src/Testing/test/Properties/AssemblyInfo.cs @@ -4,7 +4,6 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; -using Xunit; [assembly: Repeat(1)] [assembly: LogLevel(LogLevel.Trace)]