mruby-regexp: test break out of the Regexp#match block - #6992
Merged
Conversation
`Regexp#match` accepts a block and yields the `MatchData`, but the suite only covered the value the block returns normally. A non-local exit from that block was untested, even though it works today because `mrb_yield()` propagates the break. That is easy to lose in a refactor of `regexp_match()` -- moving the yield behind an error-handling wrapper, or replacing it with a call that swallows the break state, would silently make the method return the `MatchData` instead of the break value. Test-only; no implementation change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe test suite adds regression coverage for ChangesRegexp match break behavior
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Regexp#matchaccepts a block (MRB_ARGS_BLOCK()inregexp.c, yielded inregexp_match()), but the suite only covered the value the block returnsnormally:
Nothing exercised a non-local exit from that block. The behaviour is correct
today, because
mrb_yield()propagates the break:That is an easy property to lose when
regexp_match()is refactored -- forexample if the yield moves behind an error-handling wrapper, or is replaced by
a call that swallows the break state. The regression would be silent: the
method would return the
MatchDatainstead of the break value.Test-only; no implementation change.
Related to #6991, which adds the same coverage for
String#matchonce theblock is forwarded there. This one stands on its own and touches a different
part of the test file, so the two do not conflict.
rake testpasses.Summary by CodeRabbit
Bug Fixes
Regexp#matchto return the value provided by a block when it exits early withbreak.Tests