Skip to content

Conversation

@techouse
Copy link
Owner

@techouse techouse commented Sep 26, 2025

This pull request adds comprehensive new unit tests for the encoding, decoding, utility, and weak reference functionality in the codebase. The new tests cover edge cases, error handling, and internal logic, improving test coverage and robustness for the core modules.

Key additions and improvements:

DecodeOptions and Decoding Behavior:

  • Added tests for DecodeOptions to verify handling of various decoder signatures, including keyword-only, positional-only, unannotated, and literal-annotated parameters, as well as fallback logic to legacy and default decoders. Added a test to ensure built-in functions without signatures raise the original error.
  • Added tests for decoding with regex delimiters, parameter limit enforcement, skipping pairs when key decoding returns None, and correct estimation of list length for numeric parent keys in _parse_object.

EncodeOptions and Encoding Internals:

  • Added tests for EncodeOptions initialization, equality checks, and restoration of the default encoder.
  • Added internal tests for cycle detection in _encode, handling of iterable filters for indexable objects, and correct serialization of datetimes in comma format.

Utility Functions:

  • Expanded tests for Utils.merge to cover edge cases with lists, undefined values, and structured lists. Added tests for Utils.compact, _remove_undefined_from_list, and _dicts_are_equal to ensure correct handling of cycles, nested structures, and equality checks. [1] [2] [3]
  • Added tests for encoding/decoding helpers, including surrogate handling, character encoding, and dot-to-bracket conversion.

WeakWrapper and Weak Reference Handling:

  • Added tests for WeakWrapper to verify correct behavior of __repr__, __hash__ (including sets and unhashable objects), and equality checks, including handling after the proxied object is garbage collected.

Test Imports and Setup:

  • Added missing imports to support new tests and ensure proper test isolation. [1] [2] [3] [4]

These changes significantly strengthen the reliability and maintainability of the codebase by ensuring that core behaviors and edge cases are thoroughly tested.

Summary by CodeRabbit

  • Tests
    • Expanded unit tests for decode options argument handling, delimiter behavior, and error propagation.
    • Added tests for encode options initialization recovery and equality semantics.
    • Increased encode-path coverage for circular reference detection, iterable filtering, and comma-style serialization.
    • Extended utilities tests for merge behavior, handling of undefined values, compaction, surrogate/code unit cases, and list/dict edge conditions.
    • Added weak reference wrapper tests covering repr, hashing, equality with non-wrappers, and garbage-collection scenarios.

@techouse techouse self-assigned this Sep 26, 2025
@techouse techouse added the test label Sep 26, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Walkthrough

Adds multiple new unit test modules covering decoder signature handling, regex-delimiter parsing, object parsing internals, EncodeOptions post-init/equality, encode internals (circular detection, iterable filtering, comma format), utilities/merging edge cases, and WeakWrapper hashing/equality/repr behaviors. No production code or public API signatures are changed.

Changes

Cohort / File(s) Summary
Decode Options
tests/unit/decode_options_test.py
Adds tests for decoder signature handling: keyword-only vs positional-only params, unannotated and Literal kinds, and TypeError propagation when using builtin decoders.
Decode
tests/unit/decode_test.py
Adds tests for regex delimiter with infinite limits and limit errors, key-skipping when decoder returns None, and _parse_object list-length estimation via internal import.
Encode Options
tests/unit/encode_options_test.py
Adds tests for EncodeOptions __post_init__ restoring default encoder, recovering missing _encoder, and equality semantics with differing fields and non-type comparisons.
Encode Internals
tests/unit/encode_test.py
Adds tests for _encode circular reference detection/marking, iterable filtering with monkeypatched Utils, and comma-format serialization without a custom callable; imports internal helpers.
Utils & Decode/Encode Utils
tests/unit/utils_test.py
Extends tests for Utils.merge, Undefined handling with DecodeOptions(parse_lists), structured list merging, compaction/cycle handling, surrogate/code-unit behaviors, and DecodeUtils helpers.
Weak References
tests/unit/weakref_test.py
Adds WeakWrapper tests for repr with live/collected proxies, hashing (including sets and unhashables), and equality returning NotImplemented for non-wrappers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

A rabbit taps keys in the unit-test glade,
With whiskered precision, new cases are laid.
Circles are spotted, weak refs are wise,
Lists merge politely, under keen eyes.
Encode, decode—each path now checked—
Bug-burrows sealed, hoppily correct! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning Although the description provides a detailed summary of new unit tests and their coverage scope, it does not follow the repository’s template: it is missing a “Fixes #” line, the “Type of change” section with checkboxes, the “How Has This Been Tested?” section, and the required checklist. Please update the PR description to include all template sections: add the “Fixes #” reference, select the appropriate “Type of change” checkbox, fill in “How Has This Been Tested?” with your test commands and configurations, and complete the standard checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “✅ increase coverage” uses an emoji and a generic phrase that does not clearly convey the scope of the changes, leaving the primary content of the PR (comprehensive new unit tests across multiple modules) unclear at a glance. Consider using a concise sentence that specifies the key change, such as “Add comprehensive unit tests for decoding, encoding, utilities, and WeakWrapper to improve coverage”.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/increase-coverage

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb2333b and be32456.

📒 Files selected for processing (6)
  • tests/unit/decode_options_test.py (2 hunks)
  • tests/unit/decode_test.py (2 hunks)
  • tests/unit/encode_options_test.py (1 hunks)
  • tests/unit/encode_test.py (2 hunks)
  • tests/unit/utils_test.py (5 hunks)
  • tests/unit/weakref_test.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
tests/unit/decode_test.py (2)
src/qs_codec/models/decode_options.py (2)
  • decode (267-281)
  • DecodeOptions (20-293)
src/qs_codec/decode.py (2)
  • decode (31-101)
  • _parse_object (296-406)
tests/unit/weakref_test.py (1)
src/qs_codec/models/weak_wrapper.py (1)
  • WeakWrapper (104-148)
tests/unit/decode_options_test.py (3)
src/qs_codec/enums/charset.py (1)
  • Charset (25-36)
src/qs_codec/models/decode_options.py (3)
  • DecodeOptions (20-293)
  • decode_value (291-293)
  • decode_key (283-289)
src/qs_codec/enums/decode_kind.py (1)
  • DecodeKind (13-28)
tests/unit/encode_test.py (4)
src/qs_codec/encode.py (2)
  • encode (32-149)
  • _encode (159-400)
src/qs_codec/utils/encode_utils.py (2)
  • encode (85-110)
  • serialize_date (263-265)
src/qs_codec/models/weak_wrapper.py (2)
  • WeakWrapper (104-148)
  • value (125-129)
src/qs_codec/utils/utils.py (1)
  • is_non_nullish_primitive (353-382)
tests/unit/utils_test.py (5)
src/qs_codec/models/decode_options.py (2)
  • DecodeOptions (20-293)
  • decode (267-281)
src/qs_codec/models/undefined.py (1)
  • Undefined (17-69)
src/qs_codec/utils/utils.py (5)
  • merge (40-190)
  • compact (193-240)
  • _remove_undefined_from_list (243-261)
  • _dicts_are_equal (286-325)
  • is_non_nullish_primitive (353-382)
src/qs_codec/utils/encode_utils.py (2)
  • _to_surrogates (233-260)
  • _encode_char (193-214)
src/qs_codec/utils/decode_utils.py (2)
  • dot_to_bracket_top_level (33-128)
  • decode (163-203)
tests/unit/encode_options_test.py (2)
src/qs_codec/models/encode_options.py (1)
  • EncodeOptions (25-154)
src/qs_codec/utils/encode_utils.py (1)
  • EncodeUtils (13-265)
🪛 Ruff (0.13.1)
tests/unit/decode_test.py

1634-1634: Unused function argument: charset

(ARG001)

tests/unit/decode_options_test.py

145-145: Unused function argument: charset

(ARG001)


158-158: Unused function argument: charset

(ARG001)

🔇 Additional comments (7)
tests/unit/encode_options_test.py (1)

6-24: EncodeOptions defaults covered nicely

Great to see the direct assertions against _encoder to ensure __post_init__ restores the class default even after attribute tampering; this locks in the intended defensive branch.

tests/unit/encode_test.py (1)

1701-1808: Cycle-detection regression tests look solid

Exercising _encode with a pre-populated side channel (both the raising and non-raising paths) is a thorough way to pin the sentinel/step bookkeeping; this should prevent future regressions around circular structures.

tests/unit/weakref_test.py (1)

55-84: Appreciate the WeakWrapper edge coverage

The added repr/hash/equality scenarios (alive vs GC’d proxies, unhashable payloads, NotImplemented) make the wrapper guarantees much clearer and protect the proxy cache semantics.

tests/unit/decode_test.py (1)

1624-1648: Regex delimiter and parser hooks well covered

I like the focused checks on regex delimiters with both infinite and strict parameter limits, plus the _parse_object list-length probe—these mirror the tricky branches in decode.py without overreaching.

tests/unit/utils_test.py (2)

533-569: Structured-list merge scenarios nailed

The new Utils.merge cases around Undefined promotion and preserving/appending structured slots align perfectly with the parse-lists contract; thanks for codifying those expectations.


631-690: Great stress tests for compact/remove helpers

The cycle-aware compacting and nested Undefined removals (including tuple coercion) provide excellent guardrails for the in-place cleaners—these will catch regressions quickly.

tests/unit/decode_options_test.py (1)

114-173: Decoder signature matrix is comprehensive

Covering kw-only charset, positional-only kind, literal annotations, and builtin fallbacks in one sweep gives strong assurance that the adapter logic behaves for every callable shape.


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.

@codacy-production
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+4.59% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (cb2333b) 1134 1064 93.83%
Head commit (d0236dd) 1134 (+0) 1116 (+52) 98.41% (+4.59%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#28) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@codacy-production
Copy link

codacy-production bot commented Sep 26, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+6.17% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (cb2333b) 1134 1064 93.83%
Head commit (be32456) 1134 (+0) 1134 (+70) 100.00% (+6.17%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#28) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (cb2333b) to head (be32456).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main       #28      +/-   ##
===========================================
+ Coverage   93.82%   100.00%   +6.17%     
===========================================
  Files          16        16              
  Lines        1134      1134              
===========================================
+ Hits         1064      1134      +70     
+ Misses         70         0      -70     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@techouse techouse marked this pull request as ready for review September 26, 2025 19:01
@techouse techouse merged commit d2fd9b4 into main Sep 26, 2025
16 of 17 checks passed
@techouse techouse deleted the chore/increase-coverage branch September 26, 2025 19:13
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