Skip to content

Conversation

techouse
Copy link
Owner

@techouse techouse commented Oct 7, 2025

This pull request expands and improves the test coverage for both decoding and encoding functionality in QsNet.Tests, while also introducing several code quality improvements. The main changes include adding new test cases for edge scenarios (such as strict depth handling, filter behavior, and encoding of various data types), refactoring for clarity, and making use of modern C# features for conciseness and maintainability.

Decode tests:

  • Added tests for strict depth overflow and handling of trailing text in keys, ensuring exceptions are thrown when limits are exceeded (DecodeTests.cs).
  • Added tests to verify correct conversion of hashtable leaves to object-keyed dictionaries and improved coverage for regular expression and custom decoder handling. [1] [2] [3] [4]
  • Refactored tests to use const for query strings and encoded values, and introduced a [GeneratedRegex] partial method for regex test reuse. [1] [2] [3] [4]

Encode tests:

  • Added a comprehensive set of new tests for encoding, including: primitive booleans, enumerables, non-generic dictionaries, filter exception handling, skipping nulls, filter returning hashtables, and index handling in iterables. [1] [2]
  • Refactored repeated array initializations into static readonly fields for clarity and DRYness.
  • Updated buffer encoding tests to use the modern C# UTF-8 string literal syntax ("test"u8.ToArray()) for byte arrays. [1] [2] [3]

General improvements:

  • Improved test maintainability by moving local helper functions and constants to appropriate scopes and using more expressive assertions. [1] [2] [3]
  • Ensured that test coverage includes edge cases for sorting and filtering during encoding, especially at deeper object nesting levels. [1] [2] [3]

These changes significantly enhance the reliability of the test suite and ensure robust validation of both typical and edge-case behaviors in the library.

Summary by CodeRabbit

  • Tests
    • Expanded decoding coverage: depth limits, key-splitting behaviors, nested lists, hashtable/object-keyed parsing, and custom decoder scenarios.
    • Expanded encoding coverage: enumerable materialization, filters, non-generic dictionaries, null handling, index/order expectations, and standardized buffer inputs.
    • Strengthened utilities coverage: merge behaviors (including null/undefined handling), compaction, nested dictionary conversion, cycle/self-reference handling, and identity reuse.
    • Added edge-case tests for URL/Unicode handling: surrogate pairs, 4-byte sequences, invalid percent-encoding fallbacks, and mixed entity decoding.
    • Minor test consistency updates: constants, generated regex usage, and readability improvements.

@techouse techouse self-assigned this Oct 7, 2025
@techouse techouse added the test label Oct 7, 2025
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Converts DecodeTests to a public partial class, adopts generated regex in tests, standardizes shared test data and UTF-8 literals, and adds numerous new unit tests across decode, encode, and utility behaviors. Minor const and forward-declaration cleanups applied. No production code changed; all edits are within test projects.

Changes

Cohort / File(s) Summary
Decode tests overhaul
QsNet.Tests/DecodeTests.cs
Made test class public partial; added extensive new cases for depth handling, key-splitting, lists, custom decoder paths; introduced [GeneratedRegex("^test$")] with private static partial MyRegex(); replaced inline new Regex(...) calls; standardized const query strings; added early returns to gate decoder customization.
Encode tests expansion and data refactor
QsNet.Tests/EncodeTests.cs
Introduced shared static arrays (e.g., Value, ValueArray, Iterable) to replace inline literals; switched buffer construction to "..."u8.ToArray(); added and reorganized cases for enumerable/materialization, iterable filters, dictionary order, null handling, and index behaviors; inserted early returns in some tests.
Utilities behavior coverage and refactors
QsNet.Tests/UtilsTests.cs
Switched some literals to const; used "\0" for null char test; forward-declared CustomType; added many tests for Merge, compacting, nested dictionary/list conversions, cycles, identity preservation, Unicode/URL edge cases, and decode/encode fallbacks.
Sentinel test const cleanup
QsNet.Tests/SentinelExtensionsTests.cs
Changed local sentinel from mutable variable to const Sentinel in one test; no logic changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

A rabbit taps keys with a hop and a twirl,
New tests sprout like clover in a tidy swirl.
Depths are probed, lists dance in tune,
Regexes bloom under a generated moon.
With consts aplenty and bytes just right—
We nibble on green checks late into the night. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description provides a detailed narrative of the new and refactored tests but does not follow the repository’s required template sections, as it lacks the “Fixes #(issue)” line, the “Type of change” section, the “How Has This Been Tested?” section, and the standard checklist. Please restructure the description to include the “## Description” heading with a summary and linked issue, add the “## Type of change” section with relevant checkboxes, include a “## How Has This Been Tested?” section describing test steps, and complete the “## Checklist” with project-specific items.
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.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title “:white_check_mark: increase unit test coverage” directly reflects the primary change of expanding tests and is concise and specific to the changeset. Despite the emoji prefix, it remains fully related to the main change and clearly conveys the intent of the pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/more-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 and usage tips.

@techouse techouse changed the title ✅ increase coverage ✅ increase unit test coverage Oct 7, 2025
Copy link

codecov bot commented Oct 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.25%. Comparing base (7621017) to head (01ffc1e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
+ Coverage   94.92%   98.25%   +3.33%     
==========================================
  Files          15       15              
  Lines        1379     1379              
  Branches      391      391              
==========================================
+ Hits         1309     1355      +46     
+ Misses         70       24      -46     
Flag Coverage Δ
unittests 98.25% <ø> (+3.33%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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 October 7, 2025 20:17
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
QsNet.Tests/DecodeTests.cs (1)

1920-1922: Compile error: invalid lambda declaration (var action = void () => ...)

C# does not allow void in lambda literals with var. Use an Action delegate.

Apply:

-        var action = void () =>
-            parsed = Qs.Decode(str.ToString(), new DecodeOptions { Depth = depth });
+        Action action = () =>
+            parsed = Qs.Decode(str.ToString(), new DecodeOptions { Depth = depth });
🧹 Nitpick comments (6)
QsNet.Tests/EncodeTests.cs (3)

1875-1875: C# 11 UTF‑8 string literals used

"test"u8.ToArray() and similar require LangVersion >= 11.0. Confirm the test project’s LangVersion; otherwise replace with Encoding.UTF8.GetBytes(...).

If you need a fallback, change e.g.:

- { "a", "test"u8.ToArray() }
+ { "a", Encoding.UTF8.GetBytes("test") }

Also applies to: 1884-1884, 2442-2442, 2473-2473, 2485-2485, 3265-3265, 4377-4377


364-364: Remove early returns; local helpers don’t execute unless called

The return; statements used to “guard” local function declarations are unnecessary and can confuse readers and coverage tools. Local functions are inert unless invoked. Consider removing these returns and keeping helpers below or above within the same method.

-        return;
+        // keep local helper below; not executed unless called

Also applies to: 2183-2183, 2259-2259


21-26: No LangVersion changes needed; alias duplicate static arrays
Test project targets net8.0 (C# 12 by default). ValueArray duplicates Value and Value1 duplicates ValueArray0; consider:

- private static readonly string[] ValueArray = ["b", "c"];
+ private static readonly string[] ValueArray = Value;

- private static readonly string[] Value1 = ["b"];
+ private static readonly string[] Value1 = ValueArray0;
QsNet.Tests/DecodeTests.cs (3)

19-19: Partial class name consistency

Class renamed to DecodeTest and marked partial. Ensure other partials use the exact same name and namespace, or rename to match existing DecodeTests if that’s the intended shared type.


2036-2044: Early return; leaves unreachable code in tests

The return; before local function declarations compiles, but it leaves the declarations in an unreachable region. Consider moving local functions above the return;, or hoist them to private static helpers to keep test bodies clean.

Also applies to: 2185-2192


4820-4822: [GeneratedRegex] requires net7+; consider multi-target guard

Tests use [GeneratedRegex] without a NETSTANDARD2_0 fallback. If the test project multi-targets, mirror the #if NETSTANDARD2_0 ... else ... pattern used in production to keep compatibility.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7621017 and 01ffc1e.

📒 Files selected for processing (4)
  • QsNet.Tests/DecodeTests.cs (11 hunks)
  • QsNet.Tests/EncodeTests.cs (20 hunks)
  • QsNet.Tests/SentinelExtensionsTests.cs (1 hunks)
  • QsNet.Tests/UtilsTests.cs (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
QsNet.Tests/UtilsTests.cs (2)
QsNet/Internal/Utils.cs (15)
  • Utils (20-1227)
  • List (695-723)
  • Dictionary (546-687)
  • Dictionary (905-911)
  • Dictionary (918-927)
  • Dictionary (934-946)
  • Dictionary (999-1002)
  • Dictionary (1010-1076)
  • Dictionary (1105-1226)
  • Merge (69-268)
  • Encode (381-502)
  • Decode (510-538)
  • InterpretNumericEntities (808-898)
  • ReferenceEqualityComparer (1232-1249)
  • ReferenceEqualityComparer (1236-1238)
QsNet/Models/Undefined.cs (3)
  • Undefined (6-37)
  • Undefined (11-13)
  • Undefined (33-36)
QsNet.Tests/DecodeTests.cs (3)
QsNet/Models/DecodeOptions.cs (4)
  • DecodeOptions (40-321)
  • DecodeOptions (49-63)
  • DecodeOptions (274-320)
  • Decode (203-214)
QsNet/Internal/Utils.cs (13)
  • Decode (510-538)
  • Dictionary (546-687)
  • Dictionary (905-911)
  • Dictionary (918-927)
  • Dictionary (934-946)
  • Dictionary (999-1002)
  • Dictionary (1010-1076)
  • Dictionary (1105-1226)
  • Utils (20-1227)
  • GeneratedRegex (41-42)
  • GeneratedRegex (56-57)
  • Regex (36-39)
  • Regex (51-54)
QsNet/Internal/Decoder.cs (3)
  • ParseKeys (470-488)
  • Dictionary (107-271)
  • Decoder (16-761)
QsNet.Tests/EncodeTests.cs (6)
QsNet/Qs.cs (3)
  • Dictionary (30-117)
  • Qs (20-280)
  • Encode (126-279)
QsNet/Internal/Utils.cs (10)
  • Dictionary (546-687)
  • Dictionary (905-911)
  • Dictionary (918-927)
  • Dictionary (934-946)
  • Dictionary (999-1002)
  • Dictionary (1010-1076)
  • Dictionary (1105-1226)
  • Encode (381-502)
  • List (695-723)
  • Utils (20-1227)
QsNet/Internal/Encoder.cs (2)
  • Encode (42-429)
  • Encoder (14-430)
QsNet/Models/EncodeOptions.cs (3)
  • EncodeOptions (33-282)
  • EncodeOptions (42-55)
  • EncodeOptions (235-281)
QsNet/Internal/SideChannelFrame.cs (1)
  • SideChannelFrame (11-37)
QsNet/Models/Filter.cs (4)
  • IterableFilter (36-51)
  • IterableFilter (42-45)
  • FunctionFilter (15-30)
  • FunctionFilter (21-24)
🔇 Additional comments (2)
QsNet.Tests/SentinelExtensionsTests.cs (1)

35-35: Good change: make invalid enum constant

Using a const for the invalid sentinel enforces immutability and intent. LGTM.

QsNet.Tests/EncodeTests.cs (1)

209-214: Nice additions covering primitives, materialized indices, filters, and dictionaries

These tests tighten behavior around non-list enumerables, IterableFilter index handling, non-generic IDictionary, filter exception handling, hashtable conversion, and OOR indices. Good coverage expansion.

Also applies to: 216-222, 224-240, 242-248, 250-258, 260-271, 273-285, 287-299, 300-316

@techouse techouse merged commit 84a6c05 into main Oct 7, 2025
18 checks passed
@techouse techouse deleted the chore/more-tests branch October 7, 2025 20:33
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.

1 participant