-
Notifications
You must be signed in to change notification settings - Fork 646
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
MockServer: make MockResponse.body a Flow<ByteString> #4096
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
3dafbe5
to
4a2c9b5
Compare
...lo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerCommon.kt
Show resolved
Hide resolved
...ockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerExtensions.kt
Show resolved
Hide resolved
...ockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerExtensions.kt
Outdated
Show resolved
Hide resolved
import kotlin.test.assertEquals | ||
import kotlin.test.assertTrue | ||
|
||
fun assertMockResponse( | ||
mockResponse: MockResponse, | ||
body: ByteString, |
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.
Instead of adding a new parameter, could you do this instead?
assertEquals(
mockResponse.body.toList().fold(Buffer()) { buffer, byteString ->
buffer.write(byteString)
}.readByteString(),
httpResponse.body!!.readByteString()
)
This can suspend so you don't want to use it when delays are involved but I think so far it's only used in non-suspending code.
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.
Arh it seemed like a good idea, but it's a bit tricky. When using chunked you pass something like c\r\nFirst chunk\n\r\nc\r\nSecond chunk\r\n0\r\n\r\n
to the MockResponse
but what you get in the HttpResponse
is First chunk\nSecond chunk
. So you can't really compare them as-is.
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.
Looks like we need a chunk decoder then ? Should be relatively easy to implement with all the okio/kotlin goodness?
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.
Oh yeah that could work (in fact we already have chunk decoding code here 👍
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.
👍
In this PR,
MockResponse.body
becomes aFlow<ByteString>
(previously wasByteString
). This allows to not treat chunks as a special case.The JS MockServer was also changed to use a
net.Server
(instead ofhttp.Server
), which allows sharing more common code (previously,MockServerCommon.writeResponse
was used on JVM and Apple, but not JS).