✅ increase unit test coverage #17
Conversation
…odeOptionsTests for improved clarity and consistency
WalkthroughAdds a .gitignore rule for ".junie" and extensively expands and reorganizes unit tests across encoding, decoding, enums/extensions, sentinel handling, undefined singleton semantics, and utilities; the test project file is reformatted only. No production code or public API changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
==========================================
+ Coverage 90.42% 94.92% +4.49%
==========================================
Files 15 15
Lines 1379 1379
Branches 391 391
==========================================
+ Hits 1247 1309 +62
+ Misses 132 70 -62
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
QsNet.Tests/SentinelExtensionsTests.cs (1)
1-55: Solid coverage of Sentinel extensions (value, encoded, invalid cases)Explicitly calling the extension ToString via the static class avoids Enum.ToString() shadowing; invalid enum assertions look good.
Consider a small [Theory]/[InlineData] to DRY the Iso/Charset assertions.
QsNet.Tests/DecodeOptionsTests.cs (1)
248-267: CopyWith override trio test reads wellVerifies StrictDepth/ThrowOnLimitExceeded/AllowSparseLists overrides while preserving unrelated fields.
Also assert that Decoder/DecoderWithKind remain unchanged to cover function-property preservation.
QsNet.Tests/EncodeOptionsTests.cs (2)
153-183: Prefer GetEncoding("ISO-8859-1") over Encoding.Latin1 for cross-targeting.If the test project ever multi-targets netstandard2.0, Encoding.Latin1 is unavailable at compile time. Using Encoding.GetEncoding("ISO-8859-1") here avoids that fragility.
- Charset = Encoding.Latin1, + Charset = Encoding.GetEncoding("ISO-8859-1"),
208-249: CopyWith ListFormat vs. Indices relies on initializer evaluation order.The test encodes current behavior (ListFormat resolved before setting Indices), which is correct today but brittle if CopyWith’s initialization order changes. Consider documenting this in EncodeOptions or asserting intended contract at the type level.
If you want the test to be robust against internal order changes, explicitly set listFormat in CopyWith and assert it, rather than relying on implied fallback.
QsNet.Tests/EncodeTests.cs (1)
4625-4630: Consider moving test-only types to a test fixtures namespace.While the
CustomObjectclass serves its purpose for testing property access on non-dictionary objects, consider moving test-only types like this to a dedicated test fixtures namespace (e.g.,QsNet.Tests.Fixtures) to keep test files focused on test methods.Would you like me to help refactor the test helpers and fixtures into a separate file under a fixtures namespace?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.gitignore(1 hunks)QsNet.Tests/DecodeOptionsTests.cs(1 hunks)QsNet.Tests/DecodeTests.cs(1 hunks)QsNet.Tests/EncodeOptionsTests.cs(2 hunks)QsNet.Tests/EncodeTests.cs(2 hunks)QsNet.Tests/FormatExtensionsTests.cs(1 hunks)QsNet.Tests/ListFormatExtensionsTests.cs(1 hunks)QsNet.Tests/QsNet.Tests.csproj(1 hunks)QsNet.Tests/SentinelExtensionsTests.cs(1 hunks)QsNet.Tests/UndefinedTests.cs(1 hunks)QsNet.Tests/UtilsTests.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
QsNet.Tests/SentinelExtensionsTests.cs (1)
QsNet/Enums/Sentinel.cs (2)
GetEncoded(50-58)SentinelExtensions(28-69)
QsNet.Tests/ListFormatExtensionsTests.cs (2)
QsNet.Tests/EncodeOptionsTests.cs (9)
Fact(13-55)Fact(57-115)Fact(117-122)Fact(124-129)Fact(132-150)Fact(153-183)Fact(185-191)Fact(193-206)Fact(209-249)QsNet.Tests/FormatExtensionsTests.cs (3)
Fact(10-17)Fact(19-27)Fact(29-35)
QsNet.Tests/EncodeOptionsTests.cs (1)
QsNet/Models/EncodeOptions.cs (5)
EncodeOptions(33-282)EncodeOptions(42-55)EncodeOptions(235-281)GetEncoder(195-199)GetDateSerializer(207-210)
QsNet.Tests/FormatExtensionsTests.cs (2)
QsNet.Tests/EncodeOptionsTests.cs (9)
Fact(13-55)Fact(57-115)Fact(117-122)Fact(124-129)Fact(132-150)Fact(153-183)Fact(185-191)Fact(193-206)Fact(209-249)QsNet.Tests/ListFormatExtensionsTests.cs (5)
Fact(10-16)Fact(18-24)Fact(26-32)Fact(34-40)Fact(42-48)
QsNet.Tests/DecodeOptionsTests.cs (1)
QsNet/Models/DecodeOptions.cs (4)
DecodeOptions(40-321)DecodeOptions(49-63)DecodeOptions(274-320)DecodeKey(219-230)
QsNet.Tests/UndefinedTests.cs (2)
QsNet/Models/Undefined.cs (4)
Undefined(6-37)Undefined(11-13)Undefined(33-36)ToString(24-27)QsNet.Tests/UtilsTests.cs (16)
Fact(22-48)Fact(50-55)Fact(57-71)Fact(73-88)Fact(90-94)Fact(96-100)Fact(102-107)Fact(109-113)Fact(115-121)Fact(123-128)Fact(130-134)Fact(136-141)Fact(143-148)Fact(150-154)Fact(156-160)Fact(162-168)
QsNet.Tests/DecodeTests.cs (2)
QsNet/Qs.cs (2)
Qs(20-280)Dictionary(30-117)QsNet/Internal/Decoder.cs (1)
Dictionary(107-271)
QsNet.Tests/UtilsTests.cs (1)
QsNet/Internal/Utils.cs (17)
Utils(20-1227)Encode(381-502)Dictionary(546-687)Dictionary(905-911)Dictionary(918-927)Dictionary(934-946)Dictionary(999-1002)Dictionary(1010-1076)Dictionary(1105-1226)IsEmpty(773-783)List(695-723)Apply(731-747)IsNonNullishPrimitive(755-766)Merge(69-268)ConvertNestedValues(953-956)ConvertNestedValues(964-992)Decode(510-538)
QsNet.Tests/EncodeTests.cs (5)
QsNet/Internal/Utils.cs (9)
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)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/Models/Filter.cs (4)
IterableFilter(36-51)IterableFilter(42-45)FunctionFilter(15-30)FunctionFilter(21-24)QsNet/Internal/SideChannelFrame.cs (1)
SideChannelFrame(11-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Consumer smoke (net5.0; net6.0; net7.0; net8.0; net9.0)
- GitHub Check: Consumer smoke (.NET Framework 4.6.1 & 4.8.1 on Windows)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (csharp)
🔇 Additional comments (40)
.gitignore (1)
20-20: LGTM: precise ignore for .junieMatches both files and directories named ".junie" anywhere. No side effects on other patterns.
QsNet.Tests/DecodeTests.cs (1)
4171-4187: Approve: No duplicate tests detected
TheNestedBrackets_AreParsedCorrectlytest appears only once in QsNet.Tests/DecodeTests.cs.QsNet.Tests/QsNet.Tests.csproj (3)
8-12: Package versions: sanity-check restore on CINew pins look fine; recommend confirming restore/test pass across your supported SDKs (locally and CI) given the mix of versions.
18-19: No-op reformat inside coverlet.collectorIncludeAssets/PrivateAssets order change is cosmetic; safe.
23-23: ProjectReference formatting change is cosmeticNo semantic impact.
QsNet.Tests/DecodeOptionsTests.cs (2)
221-233: Implied AllowDots behavior is validatedConfirms AllowDots=true when DecodeDotInKeys=true unless explicitly overridden. Good guardrail.
235-246: Key-decoder type safety is enforcedThe exception assertion when a non-string key is returned is precise (message check included). Nice.
QsNet.Tests/EncodeOptionsTests.cs (5)
1-1: LGTM on the missing using.The added using System; is fine.
116-129: Good coverage of AllowDots implied vs. explicit behavior.These tests clearly pin the precedence between AllowDots and EncodeDotInKeys.
131-151: ListFormat fallback and override precedence looks well specified.Nice job capturing both the Indices fallback and explicit ListFormat override path.
185-191: Default encoder fallback assertion is correct and stable.This accurately reflects that plus substitution is handled by the formatter, not GetEncoder.
193-206: Good: default and custom DateSerializer behavior is pinned.Covers both branches succinctly.
QsNet.Tests/FormatExtensionsTests.cs (3)
10-17: Solid verification of Rfc3986 identity behavior.Covers both non-empty and empty inputs.
19-27: Clear Rfc1738 “%20 → +” behavior test.Good guard that other percent-escapes are preserved.
29-35: Invalid enum case is covered.Happy path and error path both validated.
QsNet.Tests/ListFormatExtensionsTests.cs (4)
10-16: Brackets generator behaves as expected.Nice to assert both null and non-null index paths.
18-32: Comma/Repeat generators verified succinctly.These ensure no accidental index/brackets leakage.
34-40: Indices path is pinned.Covers multiple indices to avoid hard-coded assumptions.
42-48: Invalid enum throws is covered.Good negative case.
QsNet.Tests/UndefinedTests.cs (3)
10-18: Nice, tight singleton semantics test.Covers both identity and ToString().
21-25: Create() vs Instance identity pinned.Ensures the factory doesn’t produce new instances.
28-33: Multiple Create() calls return the singleton.Covers the remaining angle.
QsNet.Tests/UtilsTests.cs (14)
1649-1663: Directly calling private ToDictionary is OK; you already cover the public surface too.Nice to see both the private and public pathways validated without over-relying on internals.
1668-1687: Great branch coverage for IsEmpty/HasAny with non-disposable enumerators.This closes a subtle branch many tests miss.
1698-1704: Default branch of IsNonNullishPrimitive is pinned.This helps prevent regressions if cases are reordered.
1764-1777: Compact removes Undefined from object-keyed maps.Good, minimal and precise assertion set.
1780-1799: Mixed string-keyed/object-keyed Compact walk is covered.Strong regression guard for nested map cases.
1801-1852: Nice: exercises both map and list non-generic IDictionary conversions in Compact.Also validates allowSparseLists behavior.
1854-1872: Cycle handling in Compact verified.Ensures visited set logic stays intact.
1878-1894: Cycle preservation and identity in ToStringKeyDeepNonRecursive are well tested.Good use of ReferenceEquals on the reconstructed graph.
1929-1939: Good: NormalizeForTarget identity on self-referencing maps is asserted.Helps prevent accidental deep copies.
1941-1959: Sequence materialization in ConvertNestedValues is covered.Strongly pins the IEnumerable-to-List behavior and child conversions.
1973-2001: Decode null input returning null is a useful guard.Simple and effective.
2007-2017: Identity preservation for string-keyed child maps validated.Great for ensuring no unnecessary copies.
2019-2030: String-keyed map identity inside list preserved.Another easy-to-regress edge case now covered.
2032-2043: Non-generic keys to strings conversion pinned.Covers numeric-to-string key conversion explicitly.
QsNet.Tests/EncodeTests.cs (4)
2-2: LGTM! Required import for non-generic collection tests.The addition of
System.Collectionsis necessary for theIEnumerableandIDictionarynon-generic interfaces used in the new tests.
4153-4622: Excellent test coverage expansion!This comprehensive test suite significantly improves coverage of encoder edge cases and behaviors. The tests are well-organized, clearly named, and cover important scenarios including:
- Cyclic reference detection
- Empty list handling
- Dot-in-keys encoding behavior
- Various filter types (IterableFilter, FunctionFilter)
- Primitive vs non-primitive encoding paths
- Different dictionary/collection types
- Null handling with/without custom encoders
The tests follow good practices with clear assertion patterns and appropriate use of test helpers.
4280-4285: Clean helper implementation for parsing encoder results.The
Partshelper method effectively extracts string parts from the encoder's output, properly handling both enumerable and string results.
4613-4621: Good test helper implementation.The
YieldEnumerableclass provides a clean way to test non-materialized enumerable sequences with the encoder.
… include ISet parameter for improved functionality
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
QsNet.Tests/UtilsTests.cs (4)
1630-1637: Deduplicate SegmentLimit lookup with a helper.Solid reflection-based lookup and fallback. Consider centralizing it to avoid duplication and to future-proof if SegmentLimit becomes a property.
Apply this diff here:
-var segField = typeof(Utils).GetField("SegmentLimit", - BindingFlags.NonPublic | BindingFlags.Static); -var segmentLimit = segField is null - ? 1024 // fallback to current default to avoid breaking if refactoring hides the field - : (int)(segField.IsLiteral ? segField.GetRawConstantValue()! : segField.GetValue(null)!); +var segmentLimit = GetSegmentLimit();And add this helper inside the test class:
private static int GetSegmentLimit() { const BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Public; var t = typeof(Utils); var f = t.GetField("SegmentLimit", flags); if (f != null) return f.IsLiteral ? (int)f.GetRawConstantValue()! : (int)f.GetValue(null)!; var p = t.GetProperty("SegmentLimit", flags); if (p != null) return (int)p.GetValue(null)!; return 1024; }
1644-1650: Reuse the SegmentLimit helper here too.Mirror the change to keep the two boundary tests in sync.
-var segField = typeof(Utils).GetField("SegmentLimit", - BindingFlags.NonPublic | BindingFlags.Static); -var segmentLimit = segField is null - ? 1024 - : (int)(segField.IsLiteral ? segField.GetRawConstantValue()! : segField.GetValue(null)!); +var segmentLimit = GetSegmentLimit();
1905-1936: Use explicit params array for reflection invoke; prefer FluentAssertions SameAs.Keep compatibility and consistent assertion style.
- var converted = method.Invoke(null, [src, new HashSet<object>()]) as Dictionary<string, object?>; + var converted = method.Invoke(null, new object?[] { src, new HashSet<object>() }) as Dictionary<string, object?>;Optional assertion style tweak:
- ReferenceEquals(outList, list).Should().BeTrue(); + outList.Should().BeSameAs(list);
1658-1670: Use explicit object array for Invoke arguments
Replace C# 12 collection-expression syntax in reflection calls with an explicit array to maintain compatibility. For example:-var result = method.Invoke(null, [src]); +var result = method.Invoke(null, new object?[] { src });Apply this change to all
.Invoke(null, […])occurrences in QsNet.Tests/UtilsTests.cs (lines 1407, 1470, 1663, 1923).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
QsNet.Tests/UtilsTests.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
QsNet.Tests/UtilsTests.cs (1)
QsNet/Internal/Utils.cs (17)
Utils(20-1227)Encode(381-502)Dictionary(546-687)Dictionary(905-911)Dictionary(918-927)Dictionary(934-946)Dictionary(999-1002)Dictionary(1010-1076)Dictionary(1105-1226)IsEmpty(773-783)List(695-723)Apply(731-747)IsNonNullishPrimitive(755-766)Merge(69-268)ConvertNestedValues(953-956)ConvertNestedValues(964-992)Decode(510-538)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Consumer smoke (.NET Framework 4.6.1 & 4.8.1 on Windows)
- GitHub Check: Consumer smoke (net5.0; net6.0; net7.0; net8.0; net9.0)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (csharp)
🔇 Additional comments (15)
QsNet.Tests/UtilsTests.cs (15)
1676-1695: LGTM – good branch coverage for IsEmpty/HasAny.Covers both IDisposable and non-IDisposable enumerators.
1698-1704: LGTM – Apply default branch is exercised.Confirms passthrough on type mismatch.
1707-1712: LGTM – default case for IsNonNullishPrimitive validated.Nicely isolates the catch-all branch.
1714-1762: LGTM – helper enumerables are minimal and effective.Non-disposable enumerators are appropriate for hitting the alternate path.
1772-1860: LGTM – Compact() scenarios are thorough.Validates removal, mixed-map walking, non-generic IDictionary conversion, and sparse lists.
1863-1881: LGTM – cycle handling in Compact() verified.Confirms no infinite recursion and identity preservation.
1886-1902: LGTM – ToStringKeyDeepNonRecursive cycle/identity preserved.Good assertion on parent reference equaling the top dictionary.
1943-1952: LGTM – NormalizeForTarget self-reference preserved.Accurately checks identity on self-referencing map.
1955-1980: LGTM – ConvertNestedValues sequence materialization covered.Exercises IDictionary normalization within yielded values.
1987-1997: LGTM – Merge(null, non-generic IDictionary) returns object-keyed copy.Matches NormalizeForTarget behavior.
2000-2008: LGTM – Merge(null, object-keyed map) returns same instance.Correct identity preservation.
2011-2014: LGTM – Decode(null) → null covered.Simple but important guard.
2021-2030: LGTM – string-keyed child identity preserved.Confirms ToStringKeyDeepNonRecursive doesn’t copy already string-keyed children.
2033-2043: LGTM – string-keyed map identity preserved inside list.Validates preservation within nested list.
2046-2056: LGTM – non-generic keys converted to strings.Covers mixed key types.
1 participant
This pull request significantly expands and improves the unit test coverage for several core option and extension classes, focusing on
EncodeOptions,DecodeOptions, and various extension methods. It also adds new test files for enum extension methods and refines existing tests for singleton behavior. The most important changes are grouped below:EncodeOptions and DecodeOptions Tests
EncodeOptions, including implied and explicitAllowDotslogic, fallback and override priority forListFormatandIndices, custom encoder and date serializer handling, andCopyWithbehavior for function properties.DecodeOptionscovering implied and explicitAllowDots, custom decoder error handling, andCopyWithproperty overrides, ensuring correct option propagation and error reporting.Extension Methods and Enum Behavior
FormatExtensions,ListFormatExtensions, andSentinelExtensions, thoroughly testing formatter and generator behaviors, correct handling of enum values, and exception throwing for invalid enum values. [1] [2] [3]Singleton and Instance Behavior
Undefinedsingleton to check instance uniqueness, correctToString()output, and singleton guarantees across multiple calls.Test File Structure and Minor Cleanups
using System;directive inEncodeOptionsTests.csfor consistency and completeness.Test Organization
NestedBrackets_AreParsedCorrectlytest withinDecodeTests.csto a more appropriate region for better organization and clarity. [1] [2]Summary by CodeRabbit
Tests
Chores
Style