-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adding PCRE2 tests to increase Regex code coverage #73323
Conversation
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue DetailsFixes #61893 This PR is adding all of the tests from the https://github.com/PCRE2Project/pcre2/testdata repo that can be parsed by our engine and don't rely on special syntax not supported by .NET (like I have already separately logged the issues I found in .NET while porting these (and most of them are already addressed by @stephentoub 😄). I also used Steve's technique for batching source-generator engine tests which helped bringing this single test execution from 2.3 minutes down to 1.1 seconds, which is why this test doesn't need to be moved to outerloop. In total, this is adding around 11K different tests into our test suite. cc: @richlander since I'm updating one of our THIRD-PARTY-NOTICES file.
|
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.
Excellent.
src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexPcreTests.cs
Outdated
Show resolved
Hide resolved
Nice! |
PR was green in the previous commit, and last one only addressed feedback on re-wording of comments, so I'll go ahead and merge now |
Fixes #61893
This PR is adding all of the tests from the https://github.com/PCRE2Project/pcre2/testdata repo that can be parsed by our engine and don't rely on special syntax not supported by .NET (like
[[:digit:]]
syntax for example which we are able to parse, but it semantically means something different in our case)I have already separately logged the issues I found in .NET while porting these (and most of them are already addressed by @stephentoub 😄). I also used Steve's technique for batching source-generator engine tests which helped bringing this single test execution from 2.3 minutes down to 1.1 seconds, which is why this test doesn't need to be moved to outerloop. In total, this is adding around 11K different tests into our test suite.
cc: @richlander since I'm updating one of our THIRD-PARTY-NOTICES file.