test: seed the group a backward search miss has to clear - #7151
Conversation
The test that asks whether a failing backward search clears every match global, and not `$~` alone, seeded one group and then read six names back. Two of the seven it means to cover were not covered: `$~` itself, which the comment above the block names and no assertion asked for, and `$2`, which was nil before the call because no seed ever filled it, so a clear that missed it would have gone unseen. Seed two groups and assert both. All seven names now stand for something the call cleared: dropping the clear from `__regexp_rsearch` turns 7 of them red where it turned 5 before.
|
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 failed Changesrindex Match Global Clearing
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change only strengthens regression coverage for backward-search misses, with no production behavior change; no actionable merge-blocking risk remains after normal checks and review. 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 |
Follow-up to #7149, on a review comment it left behind.
#7149 added a test that asks what a backward search leaves in the match globals, ending with the miss: a search that finds nothing clears all of them, not
$~alone. The block seeded one group and read six names back, and two of the seven it means to cover were not covered.$~is the one the comment names, and no assertion asked for it.$2was nil before the call, because"zzz" =~ /(z)/never filled it, soassert_nil $2would have passed on a clear that missed it just as well as on one that did not. Seeding/(z)(z)/is what makes the assertion stand for the clear.Every answer is CRuby's.
Verification
Dropping the clear from
__regexp_rsearch, which is the mistake the block is there to catch, turns 7 of its assertions red where it turned 5 before,$~and$2among them.Full suite green on
build_config/ci/gcc-clang.rb: full-core withMRB_GC_STRESS(2286), bintest (2286), the C++ ABI (2285), and the default gembox (2070). 0 failures, 0 crashes, no new compiler warnings.Summary by CodeRabbit