From 926e99aa7126fcdbd32e408dbc0492ce6e03678e Mon Sep 17 00:00:00 2001
From: Jordan Barrett <jordan.barrett@canonical.com>
Date: Mon, 3 Jul 2023 17:22:50 +1200
Subject: [PATCH] fix TestGathererTimeout - add IsEmpty method to logBuffer
 interface - add buffer empty check to logGatherer.flush

---
 internals/overlord/logstate/gatherer.go      |  5 +++
 internals/overlord/logstate/gatherer_test.go | 35 +++++++++++---------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/internals/overlord/logstate/gatherer.go b/internals/overlord/logstate/gatherer.go
index 4bd3ab8d9..fe12b106e 100644
--- a/internals/overlord/logstate/gatherer.go
+++ b/internals/overlord/logstate/gatherer.go
@@ -104,6 +104,10 @@ func (g *logGatherer) flush(force bool) {
 	g.bufferLock.Lock()
 	defer g.bufferLock.Unlock()
 
+	if g.buffer.IsEmpty() {
+		// No point doing anything
+		return
+	}
 	if !force {
 		if !g.buffer.IsFull() {
 			return
@@ -133,6 +137,7 @@ func (g *logGatherer) stop() {
 // the logBuffer using a sync.Mutex.
 type logBuffer interface {
 	Write(servicelog.Entry)
+	IsEmpty() bool
 	IsFull() bool
 	Request() (io.Reader, error)
 	Reset()
diff --git a/internals/overlord/logstate/gatherer_test.go b/internals/overlord/logstate/gatherer_test.go
index d4e89710b..ef452a13e 100644
--- a/internals/overlord/logstate/gatherer_test.go
+++ b/internals/overlord/logstate/gatherer_test.go
@@ -18,9 +18,11 @@ import (
 	"io"
 	"time"
 
+	"gopkg.in/yaml.v3"
+
 	"github.com/canonical/pebble/internals/servicelog"
+
 	. "gopkg.in/check.v1"
-	"gopkg.in/yaml.v3"
 )
 
 type gathererSuite struct{}
@@ -28,8 +30,8 @@ type gathererSuite struct{}
 var _ = Suite(&gathererSuite{})
 
 func (s *gathererSuite) TestGathererTimeout(c *C) {
-	client := &testClient{}
-	g := newLogGathererForTest(100*time.Nanosecond, 2, client)
+	recv := make(chan []servicelog.Entry)
+	g := newLogGathererForTest(1*time.Microsecond, 2, recv)
 	go g.loop()
 
 	entry := servicelog.Entry{
@@ -39,25 +41,22 @@ func (s *gathererSuite) TestGathererTimeout(c *C) {
 	}
 	g.addLog(entry)
 	c.Assert(g.buffer.IsFull(), Equals, false)
-	c.Assert(client.received, HasLen, 0)
 
-	time.Sleep(110 * time.Nanosecond)
-	c.Assert(client.received, HasLen, 1)
-	c.Assert(client.received[0], HasLen, 1)
-	c.Assert(client.received[0][0], DeepEquals, entry)
+	select {
+	case entries := <-recv:
+		c.Assert(entries, DeepEquals, []servicelog.Entry{entry})
+	case <-time.After(10 * time.Millisecond):
+		c.Fatal("timeout waiting to receive logs")
+	}
 }
 
-func newLogGathererForTest(
-	tickPeriod time.Duration,
-	bufferCapacity int,
-	client logClient,
-) *logGatherer {
+func newLogGathererForTest(tickPeriod time.Duration, bufferCapacity int, recv chan []servicelog.Entry) *logGatherer {
 	return &logGatherer{
 		tickPeriod: tickPeriod,
 		buffer: &testBuffer{
 			capacity: bufferCapacity,
 		},
-		client:  client,
+		client:  &testClient{recv: recv},
 		writeCh: make(chan struct{}),
 		cancel:  make(chan struct{}),
 	}
@@ -73,6 +72,10 @@ func (b *testBuffer) Write(entry servicelog.Entry) {
 	b.entries = append(b.entries, entry)
 }
 
+func (b *testBuffer) IsEmpty() bool {
+	return len(b.entries) == 0
+}
+
 func (b *testBuffer) IsFull() bool {
 	return len(b.entries) > b.capacity
 }
@@ -91,7 +94,7 @@ func (b *testBuffer) Reset() {
 
 // Fake implementation of logClient, to use in testing
 type testClient struct {
-	received [][]servicelog.Entry
+	recv chan []servicelog.Entry
 }
 
 func (c testClient) Send(body io.Reader) error {
@@ -101,6 +104,6 @@ func (c testClient) Send(body io.Reader) error {
 	if err != nil {
 		return err
 	}
-	c.received = append(c.received, entries)
+	c.recv <- entries
 	return nil
 }