Skip to content

Commit 5a0e56e

Browse files
dougbudougbu
authored
!nits! Slight Testing cleanup (#40564)
- 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
1 parent 74a6665 commit 5a0e56e

File tree

9 files changed

+122
-107
lines changed

9 files changed

+122
-107
lines changed

‎.github/ISSUE_TEMPLATE/30_api_proposal.md

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,56 @@
1-
---
2-
name: API proposal
3-
about: Propose a change to the public API surface
4-
title: ''
5-
labels: api-suggestion
6-
assignees: ''
7-
8-
---
9-
10-
## Background and Motivation
11-
12-
<!--
13-
We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process [here](https://github.com/dotnet/aspnetcore/blob/main/docs/APIReviewProcess.md). This template will help us gather the information we need to start the review process.
14-
First, please describe the purpose and value of the new API here.
15-
-->
16-
17-
## Proposed API
18-
19-
<!--
20-
Please provide the specific public API signature diff that you are proposing. For example:
21-
```diff
22-
namespace Microsoft.AspNetCore.Http
23-
{
24-
public static class HttpResponseWritingExtensions
25-
{
26-
+ public Task WriteAsync(this HttpResponse response, StringBuilder builder);
27-
}
28-
}
29-
```
30-
You may find the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/framework-design-guidelines-digest.md) helpful.
31-
-->
32-
33-
## Usage Examples
34-
35-
<!--
36-
Please provide code examples that highlight how the proposed API additions are meant to be consumed.
37-
This will help suggest whether the API has the right shape to be functional, performant and useable.
38-
You can use code blocks like this:
39-
``` C#
40-
// some lines of code here
41-
```
42-
-->
43-
44-
## Alternative Designs
45-
46-
<!--
47-
Were there other options you considered, such as alternative API shapes?
48-
How does this compare to analogous APIs in other ecosystems and libraries?
49-
-->
50-
51-
## Risks
52-
53-
<!--
54-
Please mention any risks that to your knowledge the API proposal might entail, such as breaking changes, performance regressions, etc.
55-
-->
56-
1+
---
2+
name: API proposal
3+
about: Propose a change to the public API surface
4+
title: ''
5+
labels: api-suggestion
6+
assignees: ''
7+
8+
---
9+
10+
## Background and Motivation
11+
12+
<!--
13+
We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process [here](https://github.com/dotnet/aspnetcore/blob/main/docs/APIReviewProcess.md). This template will help us gather the information we need to start the review process.
14+
First, please describe the purpose and value of the new API here.
15+
-->
16+
17+
## Proposed API
18+
19+
<!--
20+
Please provide the specific public API signature diff that you are proposing. For example:
21+
```diff
22+
namespace Microsoft.AspNetCore.Http
23+
{
24+
public static class HttpResponseWritingExtensions
25+
{
26+
+ public Task WriteAsync(this HttpResponse response, StringBuilder builder);
27+
}
28+
}
29+
```
30+
You may find the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/framework-design-guidelines-digest.md) helpful.
31+
-->
32+
33+
## Usage Examples
34+
35+
<!--
36+
Please provide code examples that highlight how the proposed API additions are meant to be consumed.
37+
This will help suggest whether the API has the right shape to be functional, performant and useable.
38+
You can use code blocks like this:
39+
``` C#
40+
// some lines of code here
41+
```
42+
-->
43+
44+
## Alternative Designs
45+
46+
<!--
47+
Were there other options you considered, such as alternative API shapes?
48+
How does this compare to analogous APIs in other ecosystems and libraries?
49+
-->
50+
51+
## Risks
52+
53+
<!--
54+
Please mention any risks that to your knowledge the API proposal might entail, such as breaking changes, performance regressions, etc.
55+
-->
56+

‎src/Testing/Testing.slnf

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"solution": {
3+
"path": "..\\..\\AspNetCore.sln",
4+
"projects": [
5+
"src\\Analyzers\\Microsoft.AspNetCore.Analyzer.Testing\\src\\Microsoft.AspNetCore.Analyzer.Testing.csproj",
6+
"src\\Testing\\src\\Microsoft.AspNetCore.Testing.csproj",
7+
"src\\Testing\\test\\Microsoft.AspNetCore.Testing.Tests.csproj"
8+
]
9+
}
10+
}

‎src/Testing/src/AssemblyTestLog.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,6 @@ public IServiceProvider CreateLoggerServices(ITestOutputHelper output, string cl
176176
return serviceCollection.BuildServiceProvider();
177177
}
178178

179-
// For back compat
180-
public static AssemblyTestLog Create(string assemblyName, string baseDirectory)
181-
=> Create(Assembly.Load(new AssemblyName(assemblyName)), baseDirectory);
182-
183179
public static AssemblyTestLog Create(Assembly assembly, string baseDirectory)
184180
{
185181
var logStart = DateTimeOffset.UtcNow;
@@ -229,7 +225,8 @@ public static AssemblyTestLog ForAssembly(Assembly assembly)
229225

230226
// Try to clear previous logs, continue if it fails.
231227
var assemblyBaseDirectory = TestFileOutputContext.GetAssemblyBaseDirectory(assembly);
232-
if (!string.IsNullOrEmpty(assemblyBaseDirectory) && !TestFileOutputContext.GetPreserveExistingLogsInOutput(assembly))
228+
if (!string.IsNullOrEmpty(assemblyBaseDirectory) &&
229+
!TestFileOutputContext.GetPreserveExistingLogsInOutput(assembly))
233230
{
234231
try
235232
{
@@ -267,6 +264,7 @@ private static SerilogLoggerProvider ConfigureFileLogging(string fileName, DateT
267264
.MinimumLevel.Verbose()
268265
.WriteTo.File(fileName, outputTemplate: "[{TimestampOffset}] [{SourceContext}] [{Level}] {Message:l}{NewLine}{Exception}", flushToDiskInterval: TimeSpan.FromSeconds(1), shared: true)
269266
.CreateLogger();
267+
270268
return new SerilogLoggerProvider(serilogger, dispose: true);
271269
}
272270

‎src/Testing/src/LoggedTest/ILoggedTest.cs

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎src/Testing/src/LoggedTest/LoggedTestBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
namespace Microsoft.AspNetCore.Testing;
1717

18-
public class LoggedTestBase : ILoggedTest, ITestMethodLifecycle
18+
public abstract class LoggedTestBase : ITestMethodLifecycle, IDisposable
1919
{
2020
private ExceptionDispatchInfo _initializationException;
2121

‎src/Testing/startvs.cmd

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@ECHO OFF
2+
3+
%~dp0..\..\startvs.cmd %~dp0Testing.slnf

‎src/Testing/test/AssemblyFixtureTest.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ namespace Microsoft.AspNetCore.Testing;
1010
[TestCaseOrderer("Microsoft.AspNetCore.Testing.AlphabeticalOrderer", "Microsoft.AspNetCore.Testing.Tests")]
1111
public class AssemblyFixtureTest
1212
{
13-
public AssemblyFixtureTest(TestAssemblyFixture assemblyFixture, TestCollectionFixture collectionFixture)
13+
public AssemblyFixtureTest(
14+
TestAssemblyFixture assemblyFixture,
15+
TestCollectionFixture collectionFixture)
1416
{
1517
AssemblyFixture = assemblyFixture;
1618
CollectionFixture = collectionFixture;

‎src/Testing/test/AssemblyTestLogTests.cs

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.IO;
66
using System.Linq;
77
using System.Reflection;
8-
using System.Runtime.CompilerServices;
98
using System.Text.RegularExpressions;
109
using System.Threading.Tasks;
1110
using Microsoft.AspNetCore.Testing;
@@ -17,7 +16,11 @@ public class AssemblyTestLogTests : LoggedTest
1716
{
1817
private static readonly Assembly ThisAssembly = typeof(AssemblyTestLogTests).GetTypeInfo().Assembly;
1918
private static readonly string ThisAssemblyName = ThisAssembly.GetName().Name;
20-
private static readonly string TFM = ThisAssembly.GetCustomAttributes().OfType<TestOutputDirectoryAttribute>().FirstOrDefault().TargetFramework;
19+
private static readonly string TFM = ThisAssembly
20+
.GetCustomAttributes()
21+
.OfType<TestOutputDirectoryAttribute>()
22+
.FirstOrDefault()
23+
.TargetFramework;
2124

2225
[Fact]
2326
public void FunctionalLogs_LogsPreservedFromNonQuarantinedTest()
@@ -43,8 +46,8 @@ public void ForAssembly_ReturnsSameInstanceForSameAssembly()
4346
public void TestLogWritesToITestOutputHelper()
4447
{
4548
var output = new TestTestOutputHelper();
46-
var assemblyLog = AssemblyTestLog.Create(ThisAssemblyName, baseDirectory: null);
4749

50+
using var assemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: null);
4851
using (assemblyLog.StartTestLog(output, "NonExistant.Test.Class", out var loggerFactory))
4952
{
5053
var logger = loggerFactory.CreateLogger("TestLogger");
@@ -69,11 +72,17 @@ public Task TestLogEscapesIllegalFileNames() =>
6972
{
7073
var illegalTestName = "T:e/s//t";
7174
var escapedTestName = "T_e_s_t";
72-
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, baseDirectory: tempDir))
73-
using (testAssemblyLog.StartTestLog(output: null, className: "FakeTestAssembly.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, resolvedTestName: out var resolvedTestname, out var _, testName: illegalTestName))
74-
{
75-
Assert.Equal(escapedTestName, resolvedTestname);
76-
}
75+
76+
using var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir);
77+
using var disposable = testAssemblyLog.StartTestLog(
78+
output: null,
79+
className: "FakeTestAssembly.FakeTestClass",
80+
loggerFactory: out var testLoggerFactory,
81+
minLogLevel: LogLevel.Trace,
82+
resolvedTestName: out var resolvedTestname,
83+
out var _,
84+
testName: illegalTestName);
85+
Assert.Equal(escapedTestName, resolvedTestname);
7786
});
7887

7988
[Fact]
@@ -84,11 +93,16 @@ public Task TestLogWritesToGlobalLogFile() =>
8493
// but it's also testing the test logging facility. So this is pretty meta ;)
8594
var logger = LoggerFactory.CreateLogger("Test");
8695

87-
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir))
96+
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
8897
{
8998
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
9099

91-
using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: "FakeTestName"))
100+
using (testAssemblyLog.StartTestLog(
101+
output: null,
102+
className: $"{ThisAssemblyName}.FakeTestClass",
103+
loggerFactory: out var testLoggerFactory,
104+
minLogLevel: LogLevel.Trace,
105+
testName: "FakeTestName"))
92106
{
93107
var testLogger = testLoggerFactory.CreateLogger("TestLogger");
94108
testLogger.LogInformation("Information!");
@@ -126,15 +140,21 @@ public Task TestLogTruncatesTestNameToAvoidLongPaths() =>
126140
{
127141
var longTestName = new string('0', 50) + new string('1', 50) + new string('2', 50) + new string('3', 50) + new string('4', 50);
128142
var logger = LoggerFactory.CreateLogger("Test");
129-
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir))
143+
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
130144
{
131145
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
132146

133-
using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: longTestName))
147+
using (testAssemblyLog.StartTestLog(
148+
output: null,
149+
className: $"{ThisAssemblyName}.FakeTestClass",
150+
loggerFactory: out var testLoggerFactory,
151+
minLogLevel: LogLevel.Trace,
152+
testName: longTestName))
134153
{
135154
testLoggerFactory.CreateLogger("TestLogger").LogInformation("Information!");
136155
}
137156
}
157+
138158
logger.LogInformation("Finished test log in {baseDirectory}", tempDir);
139159

140160
var testLogFiles = new DirectoryInfo(Path.Combine(tempDir, ThisAssemblyName, TFM, "FakeTestClass")).EnumerateFiles();
@@ -152,18 +172,24 @@ public Task TestLogEnumerateFilenamesToAvoidCollisions() =>
152172
RunTestLogFunctionalTest((tempDir) =>
153173
{
154174
var logger = LoggerFactory.CreateLogger("Test");
155-
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssemblyName, tempDir))
175+
using (var testAssemblyLog = AssemblyTestLog.Create(ThisAssembly, baseDirectory: tempDir))
156176
{
157177
logger.LogInformation("Created test log in {baseDirectory}", tempDir);
158178

159179
for (var i = 0; i < 10; i++)
160180
{
161-
using (testAssemblyLog.StartTestLog(output: null, className: $"{ThisAssemblyName}.FakeTestClass", loggerFactory: out var testLoggerFactory, minLogLevel: LogLevel.Trace, testName: "FakeTestName"))
181+
using (testAssemblyLog.StartTestLog(
182+
output: null,
183+
className: $"{ThisAssemblyName}.FakeTestClass",
184+
loggerFactory: out var testLoggerFactory,
185+
minLogLevel: LogLevel.Trace,
186+
testName: "FakeTestName"))
162187
{
163188
testLoggerFactory.CreateLogger("TestLogger").LogInformation("Information!");
164189
}
165190
}
166191
}
192+
167193
logger.LogInformation("Finished test log in {baseDirectory}", tempDir);
168194

169195
// The first log file exists
@@ -176,13 +202,13 @@ public Task TestLogEnumerateFilenamesToAvoidCollisions() =>
176202
}
177203
});
178204

179-
private static readonly Regex TimestampRegex = new Regex(@"\d+-\d+-\d+T\d+:\d+:\d+");
180-
private static readonly Regex TimestampOffsetRegex = new Regex(@"\d+\.\d+s");
181-
private static readonly Regex DurationRegex = new Regex(@"[^ ]+s$");
205+
private static readonly Regex TimestampRegex = new(@"\d+-\d+-\d+T\d+:\d+:\d+");
206+
private static readonly Regex TimestampOffsetRegex = new(@"\d+\.\d+s");
207+
private static readonly Regex DurationRegex = new(@"[^ ]+s$");
182208

183-
private async Task RunTestLogFunctionalTest(Action<string> action, [CallerMemberName] string testName = null)
209+
private static async Task RunTestLogFunctionalTest(Action<string> action)
184210
{
185-
var tempDir = Path.Combine(Path.GetTempPath(), $"TestLogging_{Guid.NewGuid().ToString("N")}");
211+
var tempDir = Path.Combine(Path.GetTempPath(), $"TestLogging_{Guid.NewGuid():N}");
186212
try
187213
{
188214
action(tempDir);
@@ -209,7 +235,7 @@ private static string MakeConsistent(string input)
209235
return string.Join(Environment.NewLine, input.Split(new[] { Environment.NewLine }, StringSplitOptions.None)
210236
.Select(line =>
211237
{
212-
var strippedPrefix = line.IndexOf('[') >= 0 ? line.Substring(line.IndexOf('[')) : line;
238+
var strippedPrefix = line.Contains('[') ? line.Substring(line.IndexOf('[')) : line;
213239

214240
var strippedDuration = DurationRegex.Replace(strippedPrefix, "DURATION");
215241
var strippedTimestamp = TimestampRegex.Replace(strippedDuration, "TIMESTAMP");

‎src/Testing/test/Properties/AssemblyInfo.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using Microsoft.AspNetCore.Testing;
55
using Microsoft.Extensions.Logging;
66
using Microsoft.Extensions.Logging.Testing;
7-
using Xunit;
87

98
[assembly: Repeat(1)]
109
[assembly: LogLevel(LogLevel.Trace)]

0 commit comments

Comments
 (0)