Repository navigation
Expose ForwardingRequestBody in the main module - #9808
Amir-Golmoradi wants to merge 2 commits into
Conversation
|
Are you using an LLM to write these? Why are you counting test failures? |
|
After encountering unexpected failures in IPv6-related tests, I wanted some context on how to investigate them. I reported the failures to make the local validation results clear. Two IPv6 tests expected I couldn't identify the root cause locally, so I opened the PR with these failures documented to ask for help investigating them. I wrote all the KDocs, tests, and comments, prepared the PRs, and made all the changes myself. |
JakeWharton
left a comment
There was a problem hiding this comment.
We should have a test that uses reflection to ensure all declared methods on the ResponseBody are forwarded. Otherwise we could add a new function with a default implementation without overriding on this new type.
| delegate.writeTo(sink) | ||
| } | ||
|
|
||
| // Duplex allows the request body to be written while the response body is read. |
There was a problem hiding this comment.
| // Duplex allows the request body to be written while the response body is read. |
This comment provides no value on top of the KDoc of the base method. It's also not visible to anyone since it's not KDoc.
| import okio.BufferedSink | ||
|
|
||
| open class ForwardingRequestBody( | ||
| delegate: RequestBody?, |
There was a problem hiding this comment.
We should not accept nulls
|
|
||
| open class ForwardingRequestBodyTest { | ||
| @Test | ||
| fun forwardsIsOneShot() { |
There was a problem hiding this comment.
This test is redundant given the one that follows it.
@JakeWharton |
Fixes #9800
Wrapping a
RequestBodyto changecontentType()also means forwardingisOneShot()andisDuplex(). Missing either loses the delegate's behavior, and each wrapper has to keep up when new methods are added.I moved the existing
ForwardingRequestBodyfromokhttp-testing-supportinto the main module and added the missingisOneShot()delegation. Subclasses can overridecontentType()while inheriting the rest of the forwarding behavior.Tests cover null and non-null content types, one-shot delegation, and overriding the content type while preserving the content, content length, and all four combinations of one-shot and duplex flags. The JVM and Android API snapshots are updated.
The
ForwardingRequestBodyTesttests,apiCheck, andspotlessCheckpass.Full
checkfailed on nine JVM tests and one Android host test. All nine JVM tests also fail on the unchanged base commite71e32849, thoughconnectTimeoutfails with a different assertion. The Android host failure hasn't been compared with the base.