From fc4cb226361b1ebd8272806387abb9f1bc0595a4 Mon Sep 17 00:00:00 2001 From: Christopher Phillips Date: Wed, 12 Jul 2023 11:27:18 -0400 Subject: [PATCH] fix: background reader apart from global handler for testing Signed-off-by: Christopher Phillips --- cmd/syft/cli/ui/handle_attestation.go | 34 +++++++++++++--------- cmd/syft/cli/ui/handle_attestation_test.go | 6 ++-- cmd/syft/cli/ui/util_test.go | 9 +++++- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/cmd/syft/cli/ui/handle_attestation.go b/cmd/syft/cli/ui/handle_attestation.go index 26c58e3890b..f753e7248ac 100644 --- a/cmd/syft/cli/ui/handle_attestation.go +++ b/cmd/syft/cli/ui/handle_attestation.go @@ -21,12 +21,11 @@ import ( ) var ( - _ tea.Model = (*attestLogFrame)(nil) - _ cosignOutputReader = (*backgroundLineReader)(nil) + _ tea.Model = (*attestLogFrame)(nil) ) type attestLogFrame struct { - reader cosignOutputReader + reader *backgroundLineReader prog progress.Progressable lines []string completed bool @@ -47,14 +46,16 @@ type attestLogFrameTickMsg struct { ID uint32 } -type cosignOutputReader interface { - Lines() []string -} - type backgroundLineReader struct { limit int lines *queue.Queue[string] lock *sync.RWMutex + + // This is added specifically for tests to assert when the background reader is done. + // The main UI uses the global ui wait group from the handler to otherwise block + // Shared concerns among multiple model made it difficult to test using the global wait group + // so this is added to allow tests to assert when the background reader is done. + running *sync.WaitGroup } func (m *Handler) handleAttestationStarted(e partybus.Event) []tea.Model { @@ -97,7 +98,7 @@ func (m *Handler) handleAttestationStarted(e partybus.Event) []tea.Model { } } -func newLogFrame(reader cosignOutputReader, prog progress.Progressable, borderStyle lipgloss.Style) attestLogFrame { +func newLogFrame(reader *backgroundLineReader, prog progress.Progressable, borderStyle lipgloss.Style) attestLogFrame { return attestLogFrame{ reader: reader, prog: prog, @@ -108,16 +109,23 @@ func newLogFrame(reader cosignOutputReader, prog progress.Progressable, borderSt } func newBackgroundLineReader(wg *sync.WaitGroup, reader io.Reader, stage *progress.Stage) *backgroundLineReader { - wg.Add(1) r := &backgroundLineReader{ - limit: 7, - lock: &sync.RWMutex{}, - lines: queue.New[string](), + limit: 7, + lock: &sync.RWMutex{}, + lines: queue.New[string](), + running: &sync.WaitGroup{}, } + // tracks the background reader for the global handler wait group + wg.Add(1) + + // tracks the background reader for the local wait group (used in tests to decouple from the global handler wait group) + r.running.Add(1) + go func() { - defer wg.Done() r.read(reader, stage) + wg.Done() + r.running.Done() }() return r diff --git a/cmd/syft/cli/ui/handle_attestation_test.go b/cmd/syft/cli/ui/handle_attestation_test.go index 12f6bf4c314..c79d01d2c2e 100644 --- a/cmd/syft/cli/ui/handle_attestation_test.go +++ b/cmd/syft/cli/ui/handle_attestation_test.go @@ -28,7 +28,7 @@ func TestHandler_handleAttestationStarted(t *testing.T) { // note: this model depends on a background reader. Multiple iterations ensures that the // reader has time to at least start and process the test fixture before the runModel // test harness completes (which is a fake event loop anyway). - iterations: 100, + iterations: 1, eventFn: func(t *testing.T) partybus.Event { reader := strings.NewReader("contents\nof\nstuff!") @@ -61,7 +61,7 @@ func TestHandler_handleAttestationStarted(t *testing.T) { // note: this model depends on a background reader. Multiple iterations ensures that the // reader has time to at least start and process the test fixture before the runModel // test harness completes (which is a fake event loop anyway). - iterations: 100, + iterations: 1, eventFn: func(t *testing.T) partybus.Event { reader := strings.NewReader("contents\nof\nstuff!") @@ -123,7 +123,7 @@ func TestHandler_handleAttestationStarted(t *testing.T) { Time: time.Now(), Sequence: log.sequence, ID: log.id, - }) + }, log.reader.running) t.Log(got) snaps.MatchSnapshot(t, got) }) diff --git a/cmd/syft/cli/ui/util_test.go b/cmd/syft/cli/ui/util_test.go index cfd5ddf5476..745612d7396 100644 --- a/cmd/syft/cli/ui/util_test.go +++ b/cmd/syft/cli/ui/util_test.go @@ -2,13 +2,14 @@ package ui import ( "reflect" + "sync" "testing" "unsafe" tea "github.com/charmbracelet/bubbletea" ) -func runModel(t testing.TB, m tea.Model, iterations int, message tea.Msg) string { +func runModel(t testing.TB, m tea.Model, iterations int, message tea.Msg, h ...*sync.WaitGroup) string { t.Helper() if iterations == 0 { iterations = 1 @@ -29,6 +30,12 @@ func runModel(t testing.TB, m tea.Model, iterations int, message tea.Msg) string } cmd = tea.Batch(nextCmds...) } + + for _, each := range h { + if each != nil { + each.Wait() + } + } return m.View() }