-
Notifications
You must be signed in to change notification settings - Fork 5k
Use RegexGenerator in applicable tests #66179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Also improve test some.
Also remove some uses that didn't require regex.
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @dotnet/area-meta Issue DetailsUse RegexGenerator where possible in tests. Also added to
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for what's obviously a lot of work on this.
I starting going through it and left some comments, but the short of it is that I think most of the changes in here should be reverted, and we should only keep:
- The src changes for DataContractSerializer
- Changes where you made better use of built-in xunit helpers, like places you switched to use Assert.Matches(...) rather than Assert.True(Regex.IsMatch(...))
Many of the other changes to test projects won't work because those test projects also build for downlevel surface area, and the regex generator requires .NET 7 or newer surface area. And for others that do work, I think many of the changes negatively impact the clarity of the tests, and I think that's more important than any startup benefits that might come from using the source generator in the tests. The other benefit would be if we think these uses of the source generator are filling gaps from our dedicated regex test suite, but if that's the case, we should augment the test suite.
...ns.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleFormatterTests.cs
Outdated
Show resolved
Hide resolved
...nsions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/ConsoleLoggerTest.cs
Outdated
Show resolved
Hide resolved
...ogging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/JsonConsoleFormatterTests.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Extensions.Logging.Console.Tests/Microsoft.Extensions.Logging.Console.Tests.csproj
Outdated
Show resolved
Hide resolved
...sions.Logging/tests/DI.Common/Common/tests/Microsoft.Extensions.Logging.Testing.Tests.csproj
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestsWriteEvent.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem/tests/Directory/GetLogicalDrives.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Ports/tests/System.IO.Ports.Tests.csproj
Outdated
Show resolved
Hide resolved
...ibraries/System.Runtime/tests/System/Runtime/ExceptionServices/ExceptionDispatchInfoTests.cs
Outdated
Show resolved
Hide resolved
@stephentoub - Run seems to have failed due to OOM, but I'm not sure which/if I should just launch an azp run. |
The out of memory is #65791. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Use RegexGenerator where possible in tests.
Also added to
System.Private.DataContractSerialization