Skip to content

test(utils): add unit test suite for primitive to bytes encoding helper - #4321

Open
gcoinstash-cmd wants to merge 1 commit into
redis:masterfrom
gcoinstash-cmd:test/safe-str-key-encoding-suite
Open

gcoinstash-cmd wants to merge 1 commit into
redis:masterfrom
gcoinstash-cmd:test/safe-str-key-encoding-suite

Conversation

@gcoinstash-cmd

@gcoinstash-cmd gcoinstash-cmd commented Sep 12, 2026

Copy link
Copy Markdown

Summary

Adds isolated unit tests for key/primitive type string encoding to bytes:

  • Bytes pass-through invariance
  • String to UTF-8 conversion
  • Numeric and Boolean primitive casting
  • None handling

Note

Low Risk
Test-only addition with no production code or runtime behavior changes.

Overview
Adds tests/test_safe_str_encoding.py, a new pytest module that defines a local safe_str_encode helper and exercises primitive-to-bytes encoding behavior.

Coverage includes bytes pass-through, UTF-8 encoding of strings, casting of int/float/bool via str() then encode, and None returning None. The helper is defined in the test file only; it does not wire into existing production utilities in this diff.

Reviewed by Cursor Bugbot for commit daa6f86. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: daa6f8616a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@@ -0,0 +1,25 @@
import pytest

def safe_str_encode(val, encoding="utf-8", errors="strict"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Test the production encoder instead of a local copy

Every assertion calls the function defined in this test module, so the suite cannot detect regressions in redis-py's actual encoding code. It also establishes expectations that conflict with redis._parsers.encoders.Encoder.encode, which raises DataError for both bool and None; changes or breakage in that production path will leave all four tests green. Import and exercise the intended production helper rather than reimplementing it in the test.

Useful? React with 👍 / 👎.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit daa6f86. Configure here.

return val
if isinstance(val, (int, float, bool)):
return str(val).encode(encoding, errors)
return str(val).encode(encoding, errors)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tests skip production encoding helper

Medium Severity

safe_str_encode is defined in this test module instead of being imported from the library. The suite only asserts against that local copy, so it never exercises Encoder.encode or redis.utils and cannot catch production encoding regressions.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit daa6f86. Configure here.

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.

1 participant