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

Make rpartition compatible with Ruby 2.7 #2320

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

gogainda
Copy link
Contributor

@gogainda gogainda commented Apr 8, 2021

Mainly it's needed to fix failing tests of the mechanize gem.

Fixes #2321

@@ -205,7 +205,7 @@ def rpartition(pattern)
if pattern.kind_of? Regexp
if m = Truffle::RegexpOperations.search_region(pattern, self, 0, size, false)
Primitive.regexp_last_match_set(Primitive.caller_special_variables, m)
[m.pre_match, m[0], m.post_match]
return [m.pre_match, m[0], m.post_match]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm was this always effectively a typo then? A missing return?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could be, yes

@chrisseaton
Copy link
Collaborator

Don't worry about the build native failure.

@gogainda gogainda changed the title Fix rpartition result when regexp doesn't match Make rpartition compatible with Ruby 2.7 Apr 9, 2021
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and specs!

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label Apr 12, 2021
end

it "returns new object if doesn't match" do
"hello".rpartition("/no_match/").last.object_id.should_not eql("hello".object_id)
Copy link
Member

Choose a reason for hiding this comment

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

.last.should_not.equal?("hello") is better here, I'll change to that.
Also we need to store the first "hello" in a variable because this file does not use frozen string literals.

@eregon eregon added this to the 21.2.0 milestone Apr 12, 2021
@gogainda
Copy link
Contributor Author

@eregon is anything else I can do here?

@chrisseaton
Copy link
Collaborator

When it gains the in-ci tag that means you're done and it's getting merged.

@eregon
Copy link
Member

eregon commented Apr 15, 2021

In this case it was a transient failure in the internal CI (webrick being not thread-safe), I've restarted it.

graalvmbot pushed a commit that referenced this pull request Apr 15, 2021
@graalvmbot graalvmbot merged commit abfe817 into oracle:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mechanize test failure: undefined method last
4 participants