Fix/flaky tests #113
Merged
Fix/flaky tests #113
Conversation
This file contains 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
Fix issues with buffered io on multiple writes to a stream
Fix issues with buffered io on multiple writes to a stream
shannonwells
reviewed
Feb 6, 2020
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
==========================================
+ Coverage 38.15% 38.33% +0.18%
==========================================
Files 41 41
Lines 2797 2805 +8
==========================================
+ Hits 1067 1075 +8
Misses 1506 1506
Partials 224 224
Continue to review full report at Codecov.
|
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.
Goals
Fix our tests, discover underlying bugs
Implementation
So I have discovered the source of at least one bug. When unmarshalling CBOR, the generated cbor-gen Unmarshallers call cbg.GetPeeker(r) which returns a new buffered reader, unless it is passed a reader that is already buffered. This buffered reader reads 16 bytes at a time from the network stream. So, if two responses end up in the stream ahead of each other, it will read extra bytes from the first. However, those bytes will be lost from the underlying reader when we come back to read the second response. The solution is to maintain a buffered reader at the stream level, so that buffered data is preserved between reads. I have verified this seems to resolve our integration test failures. It is an expected and conceivable behavior that the retrieval protocol could write two responses on the server before the client reads the first.