Skip to content

Commit

Permalink
Fix tests by only flushing gzip writer once
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
chancez committed Nov 29, 2022
1 parent b15fdfe commit 24d443d
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions lumberjack.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,12 @@ func compressLogFile(src, dst string) (err error) {
return err
}

// Flush the gz writer before Sync()ing
if err := gz.Flush(); err != nil {
// Close the gzip writer.
// Closing also triggers a Flush to the underlying
// io.Writer, and doesnot close the underlying io.Writer.
// We must Close() or Flush() the gz writer before Sync()ing otherwise we may
// see partially written data and a corrupt gzip archive.
if err := gz.Close(); err != nil {
return err
}

Expand All @@ -517,11 +521,6 @@ func compressLogFile(src, dst string) (err error) {
return err
}

// Close the gzip writer
if err := gz.Close(); err != nil {
return err
}

// close the underlying gzip file
if err := gzf.Close(); err != nil {
return err
Expand Down

0 comments on commit 24d443d

Please sign in to comment.