Skip to content

Add test coverage for string_utils.py#1756

Merged
rolandwalker merged 1 commit intomainfrom
RW/add-string-utils-tests
Mar 31, 2026
Merged

Add test coverage for string_utils.py#1756
rolandwalker merged 1 commit intomainfrom
RW/add-string-utils-tests

Conversation

@rolandwalker
Copy link
Copy Markdown
Contributor

Description

Add test coverage for string_utils.py, and fix up the changelog.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

and fix up the changelog
@rolandwalker rolandwalker self-assigned this Mar 30, 2026
@github-actions
Copy link
Copy Markdown

Findings

  1. Missing security-relevant escape-sequence coverage (low)
    File: test/pytests/test_string_utils.py
    Current tests only validate SGR-style ANSI (\x1b[31m...). Terminal-title sanitization should also be locked down for OSC/control-sequence forms (for example \x1b]0;...\x07), since those are common in title-setting/injection paths.
    Action: add a test case for OSC-style sequences to ensure sanitize_terminal_title() strips them.

  2. Missing carriage-return edge case (low)
    File: test/pytests/test_string_utils.py
    \n and \r\n are tested, but bare \r is not. Current implementation removes \r (via control-char regex) rather than spacing it, which can collapse words unexpectedly.
    Action: add a test for bare \r and assert intended behavior ("a b" vs "ab"), so this is explicit.

No correctness or regression bug found in the new test code itself.

Testing note

I could not execute the test locally because uv is not installed in this environment (uv: command not found).

@rolandwalker rolandwalker merged commit 07ea90a into main Mar 31, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/add-string-utils-tests branch March 31, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants