Skip to content

Conversation

@erlend-aasland
Copy link
Contributor

@erlend-aaslanderlend-aasland commented Nov 2, 2020

@serhiy-storchaka
Copy link
Member

Just for the case, add tests that MAXREPEAT and MAXGROUPS > 0.

@erlend-aaslanderlend-aasland marked this pull request as draft November 2, 2020 09:20
@erlend-aaslanderlend-aasland marked this pull request as ready for review November 2, 2020 10:30
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Nov 2, 2020

Just for the case, add tests that MAXREPEAT and MAXGROUPS > 0.

Would a simple assert() be sufficient, or did you mean adding a unit test?

@serhiy-storchaka
Copy link
Member

Would a simple assert() be sufficient, or did you mean adding a unit test?

I meant adding a test in Lib/test/test_re.py. If somebody will break this code in future it will get clearer test failure.

@erlend-aasland
Copy link
ContributorAuthor

Would a simple assert() be sufficient, or did you mean adding a unit test?

I meant adding a test in Lib/test/test_re.py. If somebody will break this code in future it will get clearer test failure.

Got it. Signedness test added in 8561628.

@erlend-aasland
Copy link
ContributorAuthor

@serhiy-storchaka are you ok with this PR as it stands?

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Nov 19, 2020
@erlend-aasland
Copy link
ContributorAuthor

Closing, as this was incorporated in #23393

@erlend-aaslanderlend-aasland deleted the bpo-42243/sre branch December 26, 2020 19:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@erlend-aasland@serhiy-storchaka@the-knights-who-say-ni@bedevere-bot