-
-
Notifications
You must be signed in to change notification settings - Fork 0
✅ increase unit test coverage #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds tests across interop, unit, internal, and model specs to cover default overloads, regex option interop, encoding/decoding edge cases, and model validations. Introduces two API additions: Delimiter.regex accepting Kotlin RegexOption set for Java callers, and DecodeOptions.defaults() factory. No other production control flow changes indicated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor JavaCaller as Java Caller
participant Delimiter as Delimiter (Kotlin)
participant RegexDelimiter as RegexDelimiter
JavaCaller->>Delimiter: regex(pattern: String, options: Set<RegexOption>)
Note right of Delimiter: Map Kotlin RegexOption to Java Pattern flags
Delimiter->>RegexDelimiter: construct(pattern, flags)
RegexDelimiter-->>JavaCaller: RegexDelimiter instance
sequenceDiagram
autonumber
actor Caller
participant DecodeOptions as DecodeOptions
Caller->>DecodeOptions: defaults()
DecodeOptions-->>Caller: DecodeOptions()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 #38 +/- ##
============================================
+ Coverage 94.61% 95.48% +0.87%
- Complexity 433 444 +11
============================================
Files 15 15
Lines 1262 1262
Branches 265 265
============================================
+ Hits 1194 1205 +11
+ Misses 15 9 -6
+ Partials 53 48 -5
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:
|
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/DelimiterInteropTest.java
(2 hunks)qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/ExtensionsInteropTest.java
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/ExtensionsSpec.kt
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/SentinelSpec.kt
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/UtilsSpec.kt
(2 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/internal/DecoderInternalSpec.kt
(2 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/internal/EncoderInternalSpec.kt
(2 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/DecodeOptionsSpec.kt
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/DelimiterSpec.kt
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
(1 hunks)qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/WeakWrapperSpec.kt
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/ExtensionsInteropTest.java (1)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/extensions.kt (1)
toQueryString
(24-25)
qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/DelimiterInteropTest.java (1)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/Delimiter.kt (4)
regex
(51-51)regex
(54-54)regex
(57-59)regex
(62-64)
⏰ 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). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (21)
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/SentinelSpec.kt (1)
11-11
: LGTM! Good coverage of encoded sentinel values.The new assertions verify the URL-encoded representations of both sentinel types:
ISO.encoded
: HTML entity format (utf8=✓
)CHARSET.encoded
: UTF-8 encoding (utf8=✓
)Both encode the same checkmark character (U+2713) using different formats, which appropriately tests the encoding behavior.
Also applies to: 13-13
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/UtilsSpec.kt (3)
213-215
: LGTM! Good edge-case coverage.This test correctly verifies that a malformed sequence with both an incomplete UTF-8 byte (
%E0
) and a trailing incomplete percent sign is handled by returning the input unchanged, which differs from the behavior when only%E0
is present (line 203). This distinction in error handling is valuable test coverage.
217-217
: LGTM! Essential null-safety check.
238-243
: LGTM! Important default behavior verification.This test effectively validates that when
allowSparseLists
is not explicitly specified, the default behavior removes undefined entries from lists. This complements the existing tests at lines 225 and 234 that test explicittrue
andfalse
values, providing complete coverage of the parameter's behavior.qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/internal/DecoderInternalSpec.kt (3)
3-3
: LGTM!The new imports are necessary for the added test cases that exercise duplicate key combining and indexed list behavior with sentinel values.
Also applies to: 6-6
49-72
: LGTM!The new test cases for
parseQueryStringValues
provide excellent coverage:
- Default options overload (lines 49-51)
- Bare key handling with
strictNullHandling
(lines 53-58)- Bracket suffix with comma-delimited values (lines 60-65)
- Duplicate key combining (lines 67-72)
Each test clearly documents the expected behavior and follows the existing test structure.
76-149
: LGTM!The new test cases for
parseKeys
provide comprehensive coverage of list parsing edge cases:
- Nested list chains (lines 76-90)
- Empty list handling with
allowEmptyLists
(lines 92-109)- Fallback to map entries when
parseLists
is disabled (lines 111-123)- List limit enforcement (lines 125-137)
- Indexed list creation with
Undefined()
placeholders (lines 139-149)The unchecked cast suppressions are acceptable in test code where runtime type guarantees cannot be statically verified. The use of
Undefined()
as a sentinel for missing indices aligns with the library's internal patterns.qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/internal/EncoderInternalSpec.kt (6)
7-7
: LGTM! Necessary imports for new test coverage.Both imports are used appropriately in the new test cases.
Also applies to: 12-12
142-153
: LGTM! Clear test for empty iterable handling.The test correctly verifies that
allowEmptyLists = true
produces the expected empty suffix format for an empty collection.
155-170
: LGTM! Verifies default temporal type handling.The test confirms that temporal types (Instant and LocalDateTime) are correctly stringified using their default ISO representations when no custom serializer is provided.
172-184
: LGTM! Tests default ISO formatting fallback.The test correctly verifies that LocalDateTime uses default ISO formatting when no custom
serializeDate
function is provided.
186-196
: LGTM! Verifies undefined value handling.The test properly confirms that when the
undefined
flag is true with null data, the encoder returns an empty MutableList, which is the expected behavior for skipping undefined values in query string encoding.
198-211
: LGTM! Robust cyclic reference detection test.The test properly verifies that self-referencing data structures are detected and throw an exception with a clear error message, preventing infinite loops during encoding.
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/ExtensionsSpec.kt (1)
27-30
: LGTM! Default overload test is clear and correct.The test validates that
Map.toQueryString()
uses the defaultEncodeOptions
overload when omitted, producing the expected output.qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/WeakWrapperSpec.kt (1)
65-76
: LGTM! Edge case test for cleared referent equality.The test correctly validates the short-circuit behavior when comparing a
WeakWrapper
with another whose referent has been cleared. Using reflection to forcibly clear the referent is appropriate for testing this edge case.qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/ExtensionsInteropTest.java (1)
42-45
: LGTM! Java interop test for default overload.The test validates that the default overload path works correctly from Java, mirroring the Kotlin test in
ExtensionsSpec.kt
.qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/DecodeOptionsSpec.kt (2)
111-113
: LGTM! Validates construction-time depth constraint.The test correctly verifies that negative depth values are rejected at construction time.
115-117
: LGTM! Validates defaults() factory.The test confirms that
DecodeOptions.defaults()
returns a baseline instance equal toDecodeOptions()
.qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/DelimiterSpec.kt (1)
53-54
: LGTM! Validates named parameter usage with regex options.The test correctly validates that the
Delimiter.regex
factory accepts named parameters and theLITERAL
option, producing aRegexDelimiter
instance.qs-kotlin/src/test/java/io/github/techouse/qskotlin/interop/DelimiterInteropTest.java (2)
15-15
: LGTM! Import required for RegexOption interop test.The import enables Java callers to use Kotlin's
RegexOption
in the new test below.
159-164
: LGTM! Java interop test for Kotlin RegexOption.The test validates that Java callers can use the
Delimiter.regex
factory with aSet<RegexOption>
, and that the options are correctly translated to JavaPattern
flags.
This pull request adds comprehensive new unit tests and test cases across multiple modules to improve coverage and verify correct behavior for edge cases, defaults, and error handling in the Kotlin query string library. The changes include tests for decoders, encoders, delimiters, utility functions, and models, ensuring robustness and correct handling of special scenarios.
Decoder and Encoder Improvements:
DecoderInternalSpec
for default options, handling of bare keys, nested lists, duplicate keys, and list limits, as well as forparseKeys
logic with various options.EncoderInternalSpec
for handling empty lists, serializing temporal types, propagating undefined flags, and detecting cyclic references.Utility and Model Enhancements:
UtilsSpec
for decoding malformed UTF-8, handling null input, and compacting lists with undefined entries. [1] [2]DecodeOptionsSpec
to reject negative depth and expose adefaults()
convenience method.WeakWrapperSpec
to verify equality logic when the referent is cleared.Delimiter and Query String Interop:
DelimiterSpec
andDelimiterInteropTest
for regex delimiters with Kotlin options and literal options. [1] [2]ExtensionsInteropTest
andExtensionsSpec
to verify default overloads fortoQueryString
. [1] [2]Sentinel and Miscellaneous:
SentinelSpec
to verify encoded values for sentinels.Dependency and Import Updates:
RegexOption
,Duplicates
,Undefined
, and temporal types. [1] [2] [3]These changes significantly improve the reliability and maintainability of the codebase by ensuring correct behavior across a wide range of scenarios.
Summary by CodeRabbit