fix(rt): emulate a real response body more closely to help catch subtle codegen bugs #505
Conversation
| // emulate a real response stream which typically can only be consumed once | ||
| // see: https://github.com/awslabs/aws-sdk-kotlin/issues/356 | ||
| object : HttpBody.Streaming() { | ||
| private var cnt = 0 |
There was a problem hiding this comment.
nit
a boolean would suffice here but it hardly matters in a unit test
There was a problem hiding this comment.
Oh it matters! (/s) I had a different version with count because I was getting weird results (turned out to be user error). I'll circle back and update/simplify.
| val body: HttpBody = testBuilder.expected.body?.let { | ||
| ByteArrayContent(it.encodeToByteArray()) | ||
| // emulate a real response stream which typically can only be consumed once | ||
| // see: https://github.com/awslabs/aws-sdk-kotlin/issues/356 | ||
| object : HttpBody.Streaming() { | ||
| private var consumed = false | ||
| override fun readFrom(): SdkByteReadChannel { | ||
| val content = if (consumed) ByteArray(0) else it.encodeToByteArray() | ||
| consumed = true | ||
| return SdkByteReadChannel(content) | ||
| } | ||
| } | ||
| } ?: HttpBody.Empty |
There was a problem hiding this comment.
Question: While I recognize that we were missing how consume-once streams were handled before, it seems like we'll now miss how replayable streams and simple byte arrays are handled. Is there value in testing both cases?
There was a problem hiding this comment.
Response bodies are always going to be a single use stream. At least starting from the engine. What happens in middleware may change this (as we do with wrapped response).
There was a problem hiding this comment.
You're right, I was confusing this with requests. Never mind then.
Issue #
sibling PR: awslabs/aws-sdk-kotlin#358
Description of changes
Use a streaming single use
HttpBodyvariant for protocol tests which will more closely reflect reality.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.