Skip to content

test(str): add regression test for isprintable() unicode 15 chars,#7766

Merged
youknowone merged 1 commit into
RustPython:mainfrom
ever0de:test/str-isprintable-regression-7525
May 4, 2026
Merged

test(str): add regression test for isprintable() unicode 15 chars,#7766
youknowone merged 1 commit into
RustPython:mainfrom
ever0de:test/str-isprintable-regression-7525

Conversation

@ever0de

@ever0de ever0de commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #7525

This PR adds a regression test for #7525.
At the time the issue was reported (commit 1a9b10e), str.isprintable() was returning False for '\u0b55' while CPython returns True.
The root cause was that rustpython_literal::char::is_printable() was using unic-ucd-category 0.9.0, which is based on Unicode 10.0 data. U+0B55 was unassigned (Cn) in Unicode 10.0, so it was incorrectly treated as non-printable.
This has since been fixed by migrating to icu_properties 2.2.0, which uses up-to-date Unicode data and correctly classifies U+0B55 as Mn (Nonspacing Mark), making it printable.

RustPython on  HEAD (1a9b10e) [$?] is 📦 v0.5.0 via 🦀 v1.95.0
❯ cargo run -- -c "print('\u0b55'.isprintable())"

False

RustPython on  main (c2910c0) [$?⇡] is 📦 v0.5.0 via 🐍 v3.11.6 via 🦀 v1.95.0
❯ cargo run -- -c "print('\u0b55'.isprintable())"

True

Summary by CodeRabbit

  • Tests
    • Enhanced test coverage for Unicode character validation and classification.
    • Added comprehensive tests to ensure accurate handling of printable and non-printable Unicode characters across updated Unicode standards.

@coderabbitai

coderabbitai Bot commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

A unit test str_isprintable_unicode15 is added to verify PyStr::isprintable() correctly classifies printable and non-printable Unicode characters, including a regression case for U+0B55 which should be printable per CPython.

Changes

Test Coverage for isprintable() Unicode Handling

Layer / File(s) Summary
Test Addition
crates/vm/src/builtins/str.rs		 New regression test str_isprintable_unicode15 asserts PyStr::isprintable() returns correct boolean values for printable characters ("\u{0B55}", "A", " ", "") and non-printable characters ("\x00", "\u{200B}", "\u{E000}", "\u{00A0}") to align with CPython behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • ShaharNaveh
  • youknowone

Poem

🐰 A Unicode heart beats ever true,
0B55 now printable, shiny and new,
Regression caught before it spreads,
CPython harmony fills our heads,
One test to bind them all—hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a regression test for isprintable() with Unicode 15 characters, which is the primary focus of the changeset.
Linked Issues check ✅ Passed The PR adds a regression test that validates str.isprintable() correctly handles Unicode characters like U+0B55, directly addressing the requirement in issue #7525 to align with CPython behavior.
Out of Scope Changes check ✅ Passed All changes are scoped to adding a unit test for isprintable() Unicode handling; no unrelated modifications to codebase logic or implementation are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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 and usage tips.

ShaharNaveh
ShaharNaveh approved these changes May 2, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@ever0de what do you say about using rtest? I see that all the tests in this file are parameterized

@ever0de

ever0de commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@ShaharNaveh

LGTM!

@ever0de what do you say about using rtest? I see that all the tests in this file are parameterized

Sounds great! Though I notice rstest isn't used anywhere else in this crate yet, so we'd need to add it as a dev-dependency. Is that okay?

@ShaharNaveh

ShaharNaveh commented May 2, 2026

Copy link
Copy Markdown
Contributor

@ShaharNaveh

LGTM!
@ever0de what do you say about using rtest? I see that all the tests in this file are parameterized

Sounds great! Though I notice rstest isn't used anywhere else in this crate yet, so we'd need to add it as a dev-dependency. Is that okay?

IMO yes, we can open a ticket about it. perhaps even mark it as a "good first issue"

@youknowone youknowone merged commit cf23884 into RustPython:main May 4, 2026
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Disagreement with CPython for str.isprintable

3 participants

@ever0de @ShaharNaveh @youknowone