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

Re-organize and enhance specs for Regex #12788

Merged
merged 11 commits into from
Nov 29, 2022

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Nov 27, 2022

This patch is a big overhaul of Regex specs, improving both the structure of the tests as well as adding some specs for previously untested behaviour.

The changes affect nearly all lines, but following the individual commits should make it easy to understand each set of changes. The first commit (cba37be) moves stuff around, but does not change any spec code by itself.

See #12789 for similar changes for Regex::MatchData specs.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

Nice work!

@@ -40,17 +44,17 @@ describe "Regex" do
$~.group_size.should eq(0)
end

it "matches ignore case" do
it "ignore case" do
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be "case sensitive" instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

The description is derived from the option name: Options::IGNORE_CASE

@beta-ziliani beta-ziliani added this to the 1.7.0 milestone Nov 28, 2022
@straight-shoota straight-shoota merged commit 1a2d680 into crystal-lang:master Nov 29, 2022
@straight-shoota straight-shoota deleted the spec/regex branch November 29, 2022 16:41
Sign up for free to 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.

None yet

2 participants