Skip to content
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

Fix tests by only flushing gzip writer once #4

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

chancez
Copy link
Collaborator

@chancez chancez commented Nov 29, 2022

Closing the gzip writer causes a flush, and doesn't actually close the underlying file, so we can switch to closing the gzip writer before closing the underlying file, instead of using Flush(). The previous approach of Flushing and then Closing caused us to flush twice, which had the side-effect of causing the underlying file to have an additional 5 bytes of data that was not expected in our tests.

Closing the gzip writer causes a flush, and doesn't actually close the
underlying file, so we can switch to closing the gzip writer before
closing the underlying file, instead of using Flush(). The previous
approach of Flushing and then Closing caused us to flush twice, which
had the side-effect of causing the underlying file to have an additional 5
bytes of data that was not expected in our tests.

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
@chancez chancez merged commit 821c580 into cilium:master Nov 29, 2022
@chancez chancez deleted the pr/chancez/fix_gzip_double_flush branch November 29, 2022 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants