Skip to content

test: ask what case conversion answers where it follows ASCII - #7223

Merged
matz merged 1 commit into
mruby:masterfrom
takumin:ascii-case-conversion-tests
Aug 17, 2026
Merged

test: ask what case conversion answers where it follows ASCII#7223
matz merged 1 commit into
mruby:masterfrom
takumin:ascii-case-conversion-tests

Conversation

@takumin

@takumin takumin commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

build_config/ci/gcc-clang.rb:65-70 says what the ascii-case build is there for:

  # The one build here that indexes by character and converts case by ASCII.
  # Both halves of that pair are what it covers: core's ASCII conversion, and
  # the refusal mruby-regexp answers a pattern with when /i is asked for a
  # folding the build has no table for.

The second half holds, and holds nowhere else. mrbgems/mruby-regexp/test/ascii_case.rb:12
opens with skip unless __ENCODING__ == "UTF-8", so the byte-string build reaches
neither of its blocks and this build is their only home.

The first half asserts nothing. Every assertion that would show the narrowing is gated
off rather than mirrored:

where how it is gated what runs where case follows ASCII
test/t/string.rb:433,450,468,482,497 end if UNICODECASE the blocks are not defined
mrbgems/mruby-string-ext/test/string.rb:152,327 skip unless UNICODECASE reported as Skip
mrbgems/mruby-encoding/test/string.rb:490 ... if UNICODECASE the line is dropped

So the conversion is held to "nothing raised" rather than to an answer. Counted against
the build beside it, ascii-case reports a Total of 2332 where bintest reports 2336,
and none of the four blocks that went missing came back as a mirror. Nothing in any
build configuration pins this:

"Ä".downcase       #=> "Ä" where case follows ASCII, "ä" where it follows Unicode
"ä".upcase         #=> "ä"
"ä".casecmp?("Ä")  #=> false

Fix

Gate the mirrors the other way. UNICODECASE is false both under MRB_USE_ASCII_CASE
and on a build reading its strings as bytes, so one addition covers both configurations,
and each block sits beside the Unicode block it answers.

Core holds String#downcase, #upcase and #capitalize, and asks for the bytes of a
run that spells no character handed back, the ArgumentError such a run draws elsewhere
belonging to the walk over characters. mruby-string-ext holds String#swapcase and
#casecmp?. mruby-encoding asks what the bytes of "Ä" read as UTF-8 answer, which is
what the same bytes read as bytes answer where there is nothing above ASCII to map.

Verification

rake -m test, the mrbtest counts, on e5517c30 and on this branch:

build before after
ci/gcc-clang, full-debug 2336, OK 2333, Skip 3 2338, OK 2333, Skip 5
ci/gcc-clang, bintest 2336, OK 2325, Skip 11 2338, OK 2325, Skip 13
ci/gcc-clang, cxx_abi 2336, OK 2325, Skip 11 2338, OK 2325, Skip 13
ci/gcc-clang, byte-string 2266, OK 2217, Skip 49 2269, OK 2220, Skip 49
ci/gcc-clang, ascii-case 2332, OK 2319, Skip 13 2335, OK 2322, Skip 13
build_config/default.rb 2112, OK 2063, Skip 49 2115, OK 2066, Skip 49

KO and Crash are 0 in every column. The three builds that convert case by Unicode
gain the two blocks in mruby-string-ext as Skip; the three that convert by ASCII, which
default.rb is one of, gain three blocks that run. The bintests are unchanged, 106 OK
in default.rb and the same in the bintest build.

Thirteen of the twenty one new assertions answer differently on a build converting by
Unicode, which is the narrowing they are there to show. The other eight pin that the
ASCII half still converts, and answer alike either way.

Environment

Details
OS Ubuntu 24.04
Kernel Linux 7.0.0-28-generic x86_64
CPU AMD Ryzen 9 5950X (16 cores, 32 threads)
C compiler gcc 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04.1)
binutils GNU ld 2.47.20260726
CRuby (build host) ruby 4.0.6 (2026-07-14 revision 03b6d3f889) +PRISM

The optimization level is not the same in every build, so these are the lines that
actually compiled src/string.c, with -MMD -c, -I and -o dropped. cxx_abi
compiles with gcc -x c++, not with g++; g++ only links.

# ci/gcc-clang, full-debug
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/string.c

# ci/gcc-clang, bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK src/string.c

# ci/gcc-clang, cxx_abi
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/string.c

# ci/gcc-clang, byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/string.c

# ci/gcc-clang, ascii-case
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CASE -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/string.c

# build_config/default.rb
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_COMPLEX -DMRB_USE_BIGINT -DMRB_USE_DEBUG_HOOK src/string.c

🤖 Generated with Claude Code

https://claude.ai/code/session_01PiFpCR4tWrc5iWH8DRfNeQ

Summary by CodeRabbit

  • Tests
    • Added coverage for string case conversion when Unicode support is unavailable.
    • Verified ASCII characters continue to convert correctly while non-ASCII and malformed byte sequences are handled consistently.
    • Added tests for swapcase, casecmp?, and bang methods, including unchanged-result behavior.

Where a build converts case by ASCII, whether by defining `MRB_USE_ASCII_CASE`
or by reading its strings as bytes, nothing asked what the conversion answers.
Every assertion that would show the narrowing is gated off rather than
mirrored: the blocks in core are not defined, those in mruby-string-ext report
as Skip, and the line in mruby-encoding is dropped. The conversion was held to
"nothing raised" rather than to an answer:

```ruby
# where case follows ASCII
"Ä".downcase       #=> "Ä", and "ä" where it follows Unicode
"ä".upcase         #=> "ä"
"ä".casecmp?("Ä")  #=> false
```

Gate the mirrors the other way. One addition then covers both configurations
that convert case by ASCII, and each sits beside the Unicode block it answers.
Core holds `String#downcase`, `#upcase` and `#capitalize`, and asks for the
bytes of a run that spells no character handed back, the refusal such a run
draws elsewhere belonging to the walk over characters. mruby-string-ext holds
`String#swapcase` and `#casecmp?`. mruby-encoding asks what the bytes of `"Ä"`
read as UTF-8 answer, which is what the same bytes read as bytes answer where
there is nothing above ASCII to map.
@takumin
takumin requested a review from matz as a code owner August 17, 2026 03:44
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: da3199c0-b333-4409-85f3-832d3d6359ba

📥 Commits

Reviewing files that changed from the base of the PR and between e784e62 and c0840f4.

📒 Files selected for processing (3)
  • mrbgems/mruby-encoding/test/string.rb
  • mrbgems/mruby-string-ext/test/string.rb
  • test/t/string.rb

Included review availability: Your plan includes up to 8 reviews per rolling hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The PR adds conditional tests for Unicode-aware and ASCII-only string case behavior. It covers case conversion, swapcase, case comparison, no-op bang methods, unchanged non-ASCII bytes, and malformed byte sequences.

Changes

String case behavior tests

Layer / File(s) Summary
Encoding case mapping expectations
mrbgems/mruby-encoding/test/string.rb
The test expects Unicode conversion when mapping is available and unchanged bytes otherwise. Binary-string behavior remains covered.
ASCII-only swapcase coverage
mrbgems/mruby-string-ext/test/string.rb
The tests cover ASCII swapcase, unchanged non-ASCII characters, and nil from no-op swapcase!.
ASCII comparison and conversion coverage
mrbgems/mruby-string-ext/test/string.rb, test/t/string.rb
The tests cover ASCII-only casecmp?, malformed input handling, and ASCII conversion for downcase, upcase, and capitalize. They also verify unchanged bytes and no-op bang results.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c0840

This PR adds targeted tests for ASCII case-conversion behavior without changing production code or runtime configuration. The reported builds pass with no failures or crashes, so no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

  • mruby/mruby#7182: Implements Unicode-aware case-conversion behavior covered by these fallback tests.
  • mruby/mruby#7190: Adds the MRB_USE_ASCII_CASE behavior exercised by the conditional tests.
  • mruby/mruby#7222: Adjusts the ASCII-only case-conversion behavior covered here.

Suggested labels: mrbgems, core

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies tests for case-conversion behavior that follows ASCII, matching the main purpose of the changes.
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.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 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.

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.

2 participants