Add cross-chunk boundary regression tests for chunked reader#178
Merged
Conversation
Add follow-up coverage for the chunked reader fast path introduced in PR #177: - doubled quote `""` straddling a chunk boundary, so the cross-buffer next-char lookahead has to find the second `"` at nextBuffer[0] for skipCount=1 to do the right thing - explicit-escape `\\<target>` straddling a chunk boundary, same cross-buffer next-char path with a non-self escape char - lone CR at a chunk end followed by a non-LF char in the next chunk, so the CR terminator must not consume the next field char as part of CRLF - supplementary code point (U+1F600) at the parseRowsFromChunks layer where it just passes through as ordinary chars, and at the kotlinx-io Source layer with the 😀 high surrogate at index `buffer.size - 2` so the low surrogate must land on the reserved last slot — a regression in `limit = buffer.size - 1` would overflow Also rewrite the existing chunked-path test comments to lead with why-the-test-exists instead of restating the parser branch.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version_2_0_0 #178 +/- ##
==============================================
Coverage 94.87% 94.87%
==============================================
Files 22 22
Lines 507 507
Branches 116 116
==============================================
Hits 481 481
Misses 13 13
Partials 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #177. Adds chunk-boundary regression tests for cases that the original PR's tests did not exercise:
""split across a chunk boundary (cross-buffer next-char lookahead,skipCount=1)\\<target>split across a chunk boundary (same lookahead path with a non-self escape char)CRat a chunk end followed by a non-LF char in the next chunk (CR terminator must not consume the following field char as part of CRLF)parseRowsFromChunkslayer, and at the kotlinx-ioSourcelayer with the high surrogate placed atbuffer.size - 2so the low surrogate lands on the reserved last slot — a regression inlimit = buffer.size - 1would overflowAlso rewrites the existing chunked-path test comments added in #177 to lead with why the test exists rather than restating the parser branch.
Verification
./gradlew checkNo production source changes — tests only.