Use internal channels

As feedback from a code review, use the struct channels as a way
of self documenting the code. This makes the code more readable.

Author: SimonRichardson

linux_test.go

@@ -98,13 +98,12 @@ func TestCompressMaintainMode(t *testing.T) {
 	isNil(err, t)
 	f.Close()
 
-	notify := make(chan struct{})
 	l := &Logger{
 		Compress:         true,
 		Filename:         filename,
 		MaxBackups:       1,
 		MaxSize:          100, // megabytes
-		notifyCompressed: notify,
+		notifyCompressed: make(chan struct{}),
 	}
 	defer l.Close()
 	b := []byte("boo!")
@@ -117,7 +116,7 @@ func TestCompressMaintainMode(t *testing.T) {
 	err = l.Rotate()
 	isNil(err, t)
 
-	waitForNotify(notify, t)
+	waitForNotify(l.notifyCompressed, t)
 
 	// a compressed version of the log file should now exist with the correct
 	// mode.
@@ -148,13 +147,12 @@ func TestCompressMaintainOwner(t *testing.T) {
 	isNil(err, t)
 	f.Close()
 
-	notify := make(chan struct{})
 	l := &Logger{
 		Compress:         true,
 		Filename:         filename,
 		MaxBackups:       1,
 		MaxSize:          100, // megabytes
-		notifyCompressed: notify,
+		notifyCompressed: make(chan struct{}),
 	}
 	defer l.Close()
 	b := []byte("boo!")
@@ -167,7 +165,7 @@ func TestCompressMaintainOwner(t *testing.T) {
 	err = l.Rotate()
 	isNil(err, t)
 
-	waitForNotify(notify, t)
+	waitForNotify(l.notifyCompressed, t)
 
 	// a compressed version of the log file should now exist with the correct
 	// owner.

lumberjack.go

@@ -175,16 +175,20 @@ func (l *Logger) Write(p []byte) (n int, err error) {
 func (l *Logger) Close() error {
 	l.mu.Lock()
 	defer l.mu.Unlock()
-	if err := l.close(); err != nil {
-		return err
-	}
+
+	// Always close the mill channel, even if the close fails. This way we
+	// guarantee that the mill goroutine will exit.
+	err := l.close()
+
 	if l.millCh != nil {
 		close(l.millCh)
 		l.wg.Wait()
 		l.millCh = nil
 		l.wg = nil
 	}
-	return nil
+
+	// Return the result of the file close.
+	return err
 }
 
 // close closes the file if it is open.
@@ -404,8 +408,8 @@ func (l *Logger) millRunOnce() error {
 
 // millRun runs in a goroutine to manage post-rotation compression and removal
 // of old log files.
-func (l *Logger) millRun(ch <-chan struct{}) {
-	for range ch {
+func (l *Logger) millRun() {
+	for range l.millCh {
 		// what am I going to do, log this?
 		_ = l.millRunOnce()
 	}
@@ -420,7 +424,7 @@ func (l *Logger) mill() {
 		l.wg = &sync.WaitGroup{}
 		l.wg.Add(1)
 		go func() {
-			l.millRun(l.millCh)
+			l.millRun()
 			l.wg.Done()
 		}()
 	}

lumberjack_test.go

@@ -773,11 +773,12 @@ func exists(path string, t testing.TB) {
 }
 
 func waitForNotify(notify <-chan struct{}, t testing.TB) {
+	t.Helper()
+
 	select {
 	case <-notify:
 		// All good.
 	case <-time.After(2 * time.Second):
-		fmt.Println("logger notify not signalled")
-		t.FailNow()
+		t.Fatal("logger notify not signalled")
 	}
 }