Skip to content
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 rust tests for match boundaries #72490

Merged
merged 2 commits into from
Jul 20, 2022
Merged

Conversation

joperezr
Copy link
Member

Fixes #61894

This is a port of the tests at https://github.com/rust-lang/regex/tree/6ff285e37555d4adc52f24e97318681f8a5ecd48/tests. Particularly I'm porting all of the patterns and inputs that are using the matitre and mat macros which are checking for match boundaries for given inputs. There were some tests that couldn't be ported since they relied on syntax that wasn't supported by .NET.

cc: @stephentoub

@ghost
Copy link

ghost commented Jul 19, 2022

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #61894

This is a port of the tests at https://github.com/rust-lang/regex/tree/6ff285e37555d4adc52f24e97318681f8a5ecd48/tests. Particularly I'm porting all of the patterns and inputs that are using the matitre and mat macros which are checking for match boundaries for given inputs. There were some tests that couldn't be ported since they relied on syntax that wasn't supported by .NET.

cc: @stephentoub

Author: joperezr
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: 7.0.0

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Thanks.

@stephentoub
Copy link
Member

stephentoub commented Jul 19, 2022

Particularly I'm porting all of the patterns and inputs that are using the matitre and mat macros which are checking for match boundaries for given inputs

This PR states that it fixes the rust test porting. Are there no tests beyond these worth looking into?

@joperezr
Copy link
Member Author

Are there no tests beyond these worth looking into?

There are some isMatch tests but I believe that for all of those they had a corresponding match boundaries test that I did port. They also have some Replace tests which I didn't port since they were using different syntax for the replacement's catpuring groups backreferences so it would be hard to point each test manually. They also have some unit tests around their DFA engine which are not really applicable, since they are not full scenario tests and just inspect internal state. Finally they have few split tests as well which I didn't port mostly because I believe we already had coverage for those.

So, to answer your question, I think we can probably find a few more interesting tests from them to port, but I expect that the ones in this PR are at least most of the ones we care about.

@joperezr
Copy link
Member Author

I forgot to add stats, but this is roughly adding 1600 new tests, which locally take ~20 seconds to run, so this brings the total test execution time for regex tests from ~40 seconds to ~1 minute in my machine.

@stephentoub
Copy link
Member

I expect that the ones in this PR are at least most of the ones we care about.

Ok, great, thanks

@joperezr joperezr merged commit bc3ca49 into dotnet:main Jul 20, 2022
@danmoseley
Copy link
Member

Do we need any addition to third party notices file?

@joperezr
Copy link
Member Author

Good question, I suppose the answer is yes, if so I can send a follow up PR. @richlander is that the case?

@richlander
Copy link
Member

We have a simple pattern:

@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port rust regex tests
4 participants