Skip to content

mruby-regexp: let mrbtest exercise Symbol#[] with a regexp - #7060

Merged
matz merged 1 commit into
mruby:masterfrom
takumin:regexp-symbol-ext-test-dep
Aug 10, 2026
Merged

mruby-regexp: let mrbtest exercise Symbol#[] with a regexp#7060
matz merged 1 commit into
mruby:masterfrom
takumin:regexp-symbol-ext-test-dep

Conversation

@takumin

@takumin takumin commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

mrbgems/mruby-regexp/test/symbol_regexp.rb asserts the regexp form of Symbol#[],
guarded like this:

assert("Symbol#[] with regexp") do
  skip unless :hello.respond_to?(:slice)

The guard is always false under rake test, so the assertion never runs.

mrbtest runs each gem's tests in a state of its own: mrbgems/mruby-test/mrbgem.rake
opens an mrb_open_core() state per gem and initialises it with tsort_dependencies([g.name]),
that gem's declared dependency closure and nothing else. mruby-regexp declares
mruby-string-ext (and mruby-enumerator when the build already has it), while
Symbol#[] and Symbol#slice come from mruby-symbol-ext
(mrbgems/mruby-symbol-ext/mrblib/symbol.rb:72, aliased as [] on line 76). So the run
reports:

Skip: Symbol#[] with regexp (mrbgems: mruby-regexp)

and the sym[re] path that the gem README lists among the supported Symbol methods has no
coverage in CI. The other assertions in the file do run: Symbol#match, #match? and #=~
are defined by this gem itself, so they are present in that state.

Nothing is broken

The feature works in a build that has both gems, which is every default.gembox build,
since stdlib.gembox pulls in mruby-regexp and mruby-symbol-ext:

$ ./build/host/bin/mruby -e 'p :hello[/l+/], :hello.slice(/(?<x>l+)/, :x), :hello[/z/]'
"ll"
"ll"
nil

The test is correct as written, and it is skipped for a structural reason rather than a
defect.

The change

mrbgems/mruby-regexp/mrbgem.rake already has the pattern for this, added for
mruby-enumerator: depend on a gem only when the build has it anyway, so that mrbtest
sees it without dragging it into builds that do not want it.

  if build.gems.any? {|g| g.name == 'mruby-symbol-ext'}
    spec.add_dependency 'mruby-symbol-ext', :core => 'mruby-symbol-ext'
  end

The skip guard stays: it is what keeps the test honest in a build that leaves
mruby-symbol-ext out.

Verification

rake test on this branch: the assertion runs and passes, and the skip count drops from
19 to 18 with no failures (Total 1987, OK 1969, KO 0).

One wrinkle when reproducing it: the generated per-gem test wrapper does not depend on the
gem's mrbgem.rake, so an incremental build keeps reporting the old result until the gem's
test files are touched or the build directory is cleaned.

Summary by CodeRabbit

  • Tests
    • Expanded regexp-related test coverage for symbol slicing features when the optional symbol extension is available.

@takumin
takumin requested a review from matz as a code owner August 9, 2026 22:46
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

mruby-regexp now detects mruby-symbol-ext during build configuration and declares it as a core dependency only when the gem is available.

Changes

Regexp symbol extension integration

Layer / File(s) Summary
Conditional symbol extension dependency
mrbgems/mruby-regexp/mrbgem.rake
The build configuration conditionally adds mruby-symbol-ext as a core dependency when the gem is present.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

  • mruby/mruby#6989: Both changes add optional gem dependencies in mruby-regexp/mrbgem.rake.
  • mruby/mruby#6993: This change enables the dependency required by Symbol regexp functionality.
  • mruby/mruby#7054: This change enables tests for Symbol regexp and slicing functionality.

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: enabling mrbtest to exercise Symbol#[] with a regexp.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@takumin takumin changed the title Let mrbtest exercise Symbol#[] with a regexp mruby-regexp: let mrbtest exercise Symbol#[] with a regexp Aug 9, 2026
`mrbgems/mruby-regexp/test/symbol_regexp.rb` guards its `Symbol#[]`
assertion with `skip unless :hello.respond_to?(:slice)`, and under
`rake test` that guard is always false, so the assertion never runs.

mrbtest runs each gem's tests in a state of its own: `mruby-test` opens
an `mrb_open_core()` state per gem and initialises it with that gem's
declared dependency closure and nothing else. `mruby-regexp` declares
`mruby-string-ext`, while `Symbol#[]` and `Symbol#slice` come from
`mruby-symbol-ext`, so the state running these tests has no `Symbol#[]`
and the run reports:

    Skip: Symbol#[] with regexp (mrbgems: mruby-regexp)

Depend on `mruby-symbol-ext` when the build has it anyway, the pattern
already used just above for `mruby-enumerator`, so that mrbtest sees it
without dragging it into builds that leave it out. The `skip` guard
stays: it is what keeps the test honest in such a build.

Nothing is broken in the feature itself, which works wherever both gems
are built in, and that is every `default.gembox` build. With this change
the assertion runs and passes, and the `rake test` skip count drops from
19 to 18 with no failures.
@takumin
takumin force-pushed the regexp-symbol-ext-test-dep branch from e75db4e to 6e1b39c Compare August 9, 2026 23:21
@matz
matz merged commit 94420f0 into mruby:master Aug 10, 2026
21 checks passed
@takumin
takumin deleted the regexp-symbol-ext-test-dep branch August 10, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants