From d5329ff78dc04267434767465e16c5f175eba926 Mon Sep 17 00:00:00 2001
From: Doug Bunting <6431421+dougbu@users.noreply.github.com>
Date: Fri, 25 Feb 2022 15:49:41 -0800
Subject: [PATCH 1/4] Cleanup logs after successful test runs - #39038 - update
`AssemblyTestLog` to perform actual log directory cleanup - add
`ReportTestFailure()` method for tests to report failures, disabling cleanup
- add `IAcceptFailureReports` for `AspNetTestAssemblyRunner` to report
failures to `AssemblyTestLog` - extend `AspNetTestAssemblyRunner` to
optionally create fixture instances using `static ForAssembly(Assembly)` -
add `AssemblyTestLogFixtureAttribute` to register `AssemblyTestLog` as an
assembly fixture - use `AssemblyTestLogFixtureAttribute` in all test
projects - disable log cleanup in three existing tests
nits:
- use `is [not] null` and `new()` more
---
.../Mvc.FunctionalTests/ErrorPageTests.cs | 7 +--
src/Testing/src/AssemblyTestLog.cs | 52 +++++++++++++++++--
.../src/AssemblyTestLogFixtureAttribute.cs | 11 ++++
.../build/Microsoft.AspNetCore.Testing.props | 5 +-
.../src/xunit/AspNetTestAssemblyRunner.cs | 49 +++++++++++++----
.../src/xunit/IAcceptFailureReports.cs | 9 ++++
src/Testing/test/AssemblyTestLogTests.cs | 3 ++
7 files changed, 114 insertions(+), 22 deletions(-)
create mode 100644 src/Testing/src/AssemblyTestLogFixtureAttribute.cs
create mode 100644 src/Testing/src/xunit/IAcceptFailureReports.cs
diff --git a/src/Mvc/test/Mvc.FunctionalTests/ErrorPageTests.cs b/src/Mvc/test/Mvc.FunctionalTests/ErrorPageTests.cs
index cc74a6244d72..d09f99af276b 100644
--- a/src/Mvc/test/Mvc.FunctionalTests/ErrorPageTests.cs
+++ b/src/Mvc/test/Mvc.FunctionalTests/ErrorPageTests.cs
@@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests;
///
/// Functional test to verify the error reporting of Razor compilation by diagnostic middleware.
///
-public class ErrorPageTests : IClassFixture>, IDisposable
+public class ErrorPageTests : IClassFixture>
{
private static readonly string PreserveCompilationContextMessage = HtmlEncoder.Default.Encode(
"One or more compilation references may be missing. " +
@@ -183,9 +183,4 @@ public async Task AggregateException_FlattensInnerExceptions()
Assert.Contains(nullReferenceException, content);
Assert.Contains(indexOutOfRangeException, content);
}
-
- public void Dispose()
- {
- _assemblyTestLog.Dispose();
- }
}
diff --git a/src/Testing/src/AssemblyTestLog.cs b/src/Testing/src/AssemblyTestLog.cs
index 9ca775cd45ee..cf428cb20fc9 100644
--- a/src/Testing/src/AssemblyTestLog.cs
+++ b/src/Testing/src/AssemblyTestLog.cs
@@ -20,20 +20,21 @@
namespace Microsoft.AspNetCore.Testing;
-public class AssemblyTestLog : IDisposable
+public class AssemblyTestLog : IAcceptFailureReports, IDisposable
{
private const string MaxPathLengthEnvironmentVariableName = "ASPNETCORE_TEST_LOG_MAXPATH";
private const string LogFileExtension = ".log";
private static readonly int MaxPathLength = GetMaxPathLength();
- private static readonly object _lock = new object();
- private static readonly Dictionary _logs = new Dictionary();
+ private static readonly object _lock = new();
+ private static readonly Dictionary _logs = new();
private readonly ILoggerFactory _globalLoggerFactory;
private readonly ILogger _globalLogger;
private readonly string _baseDirectory;
private readonly Assembly _assembly;
private readonly IServiceProvider _serviceProvider;
+ private bool _testFailureReported;
private static int GetMaxPathLength()
{
@@ -51,6 +52,9 @@ private AssemblyTestLog(ILoggerFactory globalLoggerFactory, ILogger globalLogger
_serviceProvider = serviceProvider;
}
+ // internal for testing
+ internal bool OnCI { get; set; } = SkipOnCIAttribute.OnCI();
+
[SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Required to maintain compatibility")]
public IDisposable StartTestLog(ITestOutputHelper output, string className, out ILoggerFactory loggerFactory, [CallerMemberName] string testName = null) =>
StartTestLog(output, className, out loggerFactory, LogLevel.Debug, testName);
@@ -218,6 +222,22 @@ public static AssemblyTestLog ForAssembly(Assembly assembly)
{
if (!_logs.TryGetValue(assembly, out var log))
{
+ var stackTrace = Environment.StackTrace;
+ if (!(stackTrace.Contains(
+ "Microsoft.AspNetCore.Testing"
+#if NETCOREAPP
+ , StringComparison.Ordinal
+#endif
+ ) || stackTrace.Contains(
+ "Microsoft.Extensions.Logging.Testing.Tests"
+#if NETCOREAPP
+ , StringComparison.Ordinal
+#endif
+ )))
+ {
+ throw new InvalidOperationException($"Unexpected initial {nameof(ForAssembly)} caller.");
+ }
+
var baseDirectory = TestFileOutputContext.GetOutputDirectory(assembly);
log = Create(assembly, baseDirectory);
@@ -235,10 +255,16 @@ public static AssemblyTestLog ForAssembly(Assembly assembly)
catch { }
}
}
+
return log;
}
}
+ public void ReportTestFailure()
+ {
+ _testFailureReported = true;
+ }
+
private static TestFrameworkFileLoggerAttribute GetFileLoggerAttribute(Assembly assembly)
=> assembly.GetCustomAttribute()
?? throw new InvalidOperationException($"No {nameof(TestFrameworkFileLoggerAttribute)} found on the assembly {assembly.GetName().Name}. "
@@ -268,10 +294,28 @@ private static SerilogLoggerProvider ConfigureFileLogging(string fileName, DateT
return new SerilogLoggerProvider(serilogger, dispose: true);
}
- public void Dispose()
+ void IDisposable.Dispose()
{
(_serviceProvider as IDisposable)?.Dispose();
_globalLoggerFactory.Dispose();
+
+ // Clean up if no tests failed and we're not running local tests. (Ignoring tests of this class, OnCI is
+ // true on both build and Helix agents.) In particular, remove the directory containing the global.log
+ // file. All test class log files for this assembly are in subdirectories of this location.
+ if (!_testFailureReported &&
+ OnCI &&
+ _baseDirectory is not null &&
+ Directory.Exists(_baseDirectory))
+ {
+ try
+ {
+ Directory.Delete(_baseDirectory, recursive: true);
+ }
+ catch
+ {
+ // Best effort. Ignore problems deleting locked logged files.
+ }
+ }
}
private class AssemblyLogTimestampOffsetEnricher : ILogEventEnricher
diff --git a/src/Testing/src/AssemblyTestLogFixtureAttribute.cs b/src/Testing/src/AssemblyTestLogFixtureAttribute.cs
new file mode 100644
index 000000000000..e4a4452cd458
--- /dev/null
+++ b/src/Testing/src/AssemblyTestLogFixtureAttribute.cs
@@ -0,0 +1,11 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Testing;
+
+public class AssemblyTestLogFixtureAttribute : AssemblyFixtureAttribute
+{
+ public AssemblyTestLogFixtureAttribute() : base(typeof(AssemblyTestLog))
+ {
+ }
+}
diff --git a/src/Testing/src/build/Microsoft.AspNetCore.Testing.props b/src/Testing/src/build/Microsoft.AspNetCore.Testing.props
index 063e9094d172..47d06dfef7a7 100644
--- a/src/Testing/src/build/Microsoft.AspNetCore.Testing.props
+++ b/src/Testing/src/build/Microsoft.AspNetCore.Testing.props
@@ -11,8 +11,8 @@
+ BeforeTargets="GetAssemblyAttributes"
+ Condition="'$(GenerateLoggingTestingAssemblyAttributes)' != 'false'">
true
false
@@ -24,6 +24,7 @@
<_Parameter2>Microsoft.AspNetCore.Testing
+
<_Parameter1>$(PreserveExistingLogsInOutput)
<_Parameter2>$(TargetFramework)
diff --git a/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs b/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
index 0d99ffcff58b..aa70d2bc8a04 100644
--- a/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
+++ b/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
@@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
+using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Xunit;
@@ -14,7 +15,7 @@ namespace Microsoft.AspNetCore.Testing;
public class AspNetTestAssemblyRunner : XunitTestAssemblyRunner
{
- private readonly Dictionary _assemblyFixtureMappings = new Dictionary();
+ private readonly Dictionary _assemblyFixtureMappings = new();
public AspNetTestAssemblyRunner(
ITestAssembly testAssembly,
@@ -33,8 +34,8 @@ protected override async Task AfterTestAssemblyStartingAsync()
// Find all the AssemblyFixtureAttributes on the test assembly
await Aggregator.RunAsync(async () =>
{
- var fixturesAttributes = ((IReflectionAssemblyInfo)TestAssembly.Assembly)
- .Assembly
+ var assembly = ((IReflectionAssemblyInfo)TestAssembly.Assembly).Assembly;
+ var fixturesAttributes = assembly
.GetCustomAttributes(typeof(AssemblyFixtureAttribute), false)
.Cast()
.ToList();
@@ -42,15 +43,30 @@ await Aggregator.RunAsync(async () =>
// Instantiate all the fixtures
foreach (var fixtureAttribute in fixturesAttributes)
{
- var ctorWithDiagnostics = fixtureAttribute.FixtureType.GetConstructor(new[] { typeof(IMessageSink) });
object instance = null;
- if (ctorWithDiagnostics != null)
+ var staticCreator = fixtureAttribute.FixtureType.GetMethod(
+ name: "ForAssembly",
+ bindingAttr: BindingFlags.Public | BindingFlags.Static,
+ binder: null,
+ types: new[] { typeof(Assembly) },
+ modifiers: null);
+ if (staticCreator is null)
{
- instance = Activator.CreateInstance(fixtureAttribute.FixtureType, DiagnosticMessageSink);
+ var ctorWithDiagnostics = fixtureAttribute
+ .FixtureType
+ .GetConstructor(new[] { typeof(IMessageSink) });
+ if (ctorWithDiagnostics is null)
+ {
+ instance = Activator.CreateInstance(fixtureAttribute.FixtureType);
+ }
+ else
+ {
+ instance = Activator.CreateInstance(fixtureAttribute.FixtureType, DiagnosticMessageSink);
+ }
}
else
{
- instance = Activator.CreateInstance(fixtureAttribute.FixtureType);
+ instance = staticCreator.Invoke(obj: null, parameters: new[] { assembly });
}
_assemblyFixtureMappings[fixtureAttribute.FixtureType] = instance;
@@ -79,12 +95,13 @@ protected override async Task BeforeTestAssemblyFinishedAsync()
await base.BeforeTestAssemblyFinishedAsync().ConfigureAwait(false);
}
- protected override Task RunTestCollectionAsync(
+ protected override async Task RunTestCollectionAsync(
IMessageBus messageBus,
ITestCollection testCollection,
IEnumerable testCases,
CancellationTokenSource cancellationTokenSource)
- => new AspNetTestCollectionRunner(
+ {
+ var runSummary = await new AspNetTestCollectionRunner(
_assemblyFixtureMappings,
testCollection,
testCases,
@@ -92,5 +109,17 @@ protected override Task RunTestCollectionAsync(
messageBus,
TestCaseOrderer,
new ExceptionAggregator(Aggregator),
- cancellationTokenSource).RunAsync();
+ cancellationTokenSource)
+ .RunAsync()
+ .ConfigureAwait(false);
+ if (runSummary.Failed != 0)
+ {
+ foreach (var fixture in _assemblyFixtureMappings.Values.OfType())
+ {
+ fixture.ReportTestFailure();
+ }
+ }
+
+ return runSummary;
+ }
}
diff --git a/src/Testing/src/xunit/IAcceptFailureReports.cs b/src/Testing/src/xunit/IAcceptFailureReports.cs
new file mode 100644
index 000000000000..30ca366b3f1b
--- /dev/null
+++ b/src/Testing/src/xunit/IAcceptFailureReports.cs
@@ -0,0 +1,9 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Testing;
+
+internal interface IAcceptFailureReports
+{
+ void ReportTestFailure();
+}
diff --git a/src/Testing/test/AssemblyTestLogTests.cs b/src/Testing/test/AssemblyTestLogTests.cs
index c31c6e404b29..2cf0d0470cfb 100644
--- a/src/Testing/test/AssemblyTestLogTests.cs
+++ b/src/Testing/test/AssemblyTestLogTests.cs
@@ -95,6 +95,7 @@ public Task TestLogWritesToGlobalLogFile() =>
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
{
+ testAssemblyLog.OnCI = false;
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
using (testAssemblyLog.StartTestLog(
@@ -142,6 +143,7 @@ public Task TestLogTruncatesTestNameToAvoidLongPaths() =>
var logger = LoggerFactory.CreateLogger("Test");
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
{
+ testAssemblyLog.OnCI = false;
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
using (testAssemblyLog.StartTestLog(
@@ -174,6 +176,7 @@ public Task TestLogEnumerateFilenamesToAvoidCollisions() =>
var logger = LoggerFactory.CreateLogger("Test");
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
{
+ testAssemblyLog.OnCI = false;
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
for (var i = 0; i < 10; i++)
From 55025cec89fd922aa6f835dc77b50492357bb059 Mon Sep 17 00:00:00 2001
From: Doug Bunting <6431421+dougbu@users.noreply.github.com>
Date: Sun, 13 Mar 2022 11:30:58 -0700
Subject: [PATCH 2/4] Add tests of new cleanup features - also cover a few
existing methods
nit: move `AssemblyTestLogTests` to same namespace as `AssemblyTestLog`
---
src/Testing/src/AssemblyTestLog.cs | 12 +-
.../src/xunit/AspNetTestAssemblyRunner.cs | 9 +-
.../test/AspNetTestAssemblyRunnerTest.cs | 219 +++++++++++++++++
src/Testing/test/AssemblyTestLogTests.cs | 220 +++++++++++++++---
.../test/TestableAspNetTestAssemblyRunner.cs | 105 +++++++++
src/Testing/test/TestableAssembly.cs | 95 ++++++++
6 files changed, 620 insertions(+), 40 deletions(-)
create mode 100644 src/Testing/test/AspNetTestAssemblyRunnerTest.cs
create mode 100644 src/Testing/test/TestableAspNetTestAssemblyRunner.cs
create mode 100644 src/Testing/test/TestableAssembly.cs
diff --git a/src/Testing/src/AssemblyTestLog.cs b/src/Testing/src/AssemblyTestLog.cs
index cf428cb20fc9..09a2e7b1096d 100644
--- a/src/Testing/src/AssemblyTestLog.cs
+++ b/src/Testing/src/AssemblyTestLog.cs
@@ -180,7 +180,8 @@ public IServiceProvider CreateLoggerServices(ITestOutputHelper output, string cl
return serviceCollection.BuildServiceProvider();
}
- public static AssemblyTestLog Create(Assembly assembly, string baseDirectory)
+ // internal for testing. Expectation is AspNetTestAssembly runner calls ForAssembly() first for every Assembly.
+ internal static AssemblyTestLog Create(Assembly assembly, string baseDirectory)
{
var logStart = DateTimeOffset.UtcNow;
SerilogLoggerProvider serilogLoggerProvider = null;
@@ -223,17 +224,12 @@ public static AssemblyTestLog ForAssembly(Assembly assembly)
if (!_logs.TryGetValue(assembly, out var log))
{
var stackTrace = Environment.StackTrace;
- if (!(stackTrace.Contains(
+ if (!stackTrace.Contains(
"Microsoft.AspNetCore.Testing"
#if NETCOREAPP
, StringComparison.Ordinal
#endif
- ) || stackTrace.Contains(
- "Microsoft.Extensions.Logging.Testing.Tests"
-#if NETCOREAPP
- , StringComparison.Ordinal
-#endif
- )))
+ ))
{
throw new InvalidOperationException($"Unexpected initial {nameof(ForAssembly)} caller.");
}
diff --git a/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs b/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
index aa70d2bc8a04..5d149fd48a93 100644
--- a/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
+++ b/src/Testing/src/xunit/AspNetTestAssemblyRunner.cs
@@ -27,6 +27,9 @@ public AspNetTestAssemblyRunner(
{
}
+ // internal for testing
+ internal IEnumerable