Skip to content

test: cover line subscriber UTF-8 and newline boundaries - #11

Open
crowndlt wants to merge 1 commit into
lqhuang:mainfrom
crowndlt:test-line-subscriber-parity
Open

test: cover line subscriber UTF-8 and newline boundaries#11
crowndlt wants to merge 1 commit into
lqhuang:mainfrom
crowndlt:test-line-subscriber-parity

Conversation

@crowndlt

@crowndlt crowndlt commented Sep 8, 2026

Copy link
Copy Markdown

Adds eight tests for UTF-8 decoding and line boundaries in BodySubscribers.fromLineSubscriber, following the discussion in requests-scala#156.

The cases cover multibyte characters split across buffers and onNext calls, byte-at-a-time delivery, CR/LF/CRLF with the default separator, and an incomplete UTF-8 sequence at EOF. An explicit LF separator is included as a control.

Validation on Linux:

  • JVM 25: 8 passed.
  • Scala Native: 1 passed, 7 failed. The explicit LF control passes; the other cases expose the decoding and default-separator differences.

Keeping this as a draft because it adds reproducing tests and the implementation still needs fixing. The PR changes only the test file.

To run the suite, use modules.java-http-client.jvm.test.testOnly snhttp.test.java.net.http.BodySubscribersLineTest or modules.java-http-client.test.testOnly snhttp.test.java.net.http.BodySubscribersLineTest with Mill.

For any follow-up fixes, would these issues fall within the scope of the original bounty?

@lqhuang

lqhuang commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Close since it seems agent spam. Reopen only if you're not a bot.

@lqhuang lqhuang closed this Sep 8, 2026
@crowndlt

crowndlt commented Sep 8, 2026

Copy link
Copy Markdown
Author

No worries, I saw the original bounty was withdrawn, so I'll leave it here. Thanks for taking a look!

@lqhuang

lqhuang commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Hi @crowndlt, Sorry, I've reopened this PR. Too many bots suddenly appeared for unknown reasons. I will take a look soon. Thanks for your contribution!

@lqhuang lqhuang reopened this Sep 9, 2026
@lqhuang
lqhuang marked this pull request as ready for review September 9, 2026 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants