diff --git a/ingestion/internal/symlinkguard/symlinkguard_test.go b/ingestion/internal/symlinkguard/symlinkguard_test.go index 3bb1ac3..bd867fe 100644 --- a/ingestion/internal/symlinkguard/symlinkguard_test.go +++ b/ingestion/internal/symlinkguard/symlinkguard_test.go @@ -381,18 +381,19 @@ func TestWatchQueueIsBoundedDuringAnOutage(t *testing.T) { require.Eventually(t, func() bool { _, err := os.Lstat(link); return os.IsNotExist(err) }, 2*time.Second, 2*time.Millisecond) } n.down.Store(false) - require.Eventually(t, func() bool { return n.count() >= 1 }, 2*time.Second, 5*time.Millisecond) - time.Sleep(100 * time.Millisecond) - assert.LessOrEqual(t, n.count(), maxPending+1, "queued alerts plus at most one suppression summary") - n.mu.Lock() - defer n.mu.Unlock() - found := false - for _, m := range n.msgs { - if strings.Contains(m, "suppressed") { - found = true + // Synchronise on the summary being delivered (hyperguild#193: a fixed + // sleep flaked under load); once it is, the queue has been flushed. + require.Eventually(t, func() bool { + n.mu.Lock() + defer n.mu.Unlock() + for _, m := range n.msgs { + if strings.Contains(m, "suppressed") { + return true + } } - } - assert.True(t, found, "suppressed alerts are counted in a summary") + return false + }, 10*time.Second, 5*time.Millisecond, "suppressed alerts are counted in a summary") + assert.LessOrEqual(t, n.count(), maxPending+1, "queued alerts plus at most one suppression summary") } // Within the bound, every alert queued during an outage is delivered after @@ -411,11 +412,21 @@ func TestWatchDeliversEveryQueuedAlertAfterAShortOutage(t *testing.T) { require.Eventually(t, func() bool { _, err := os.Lstat(link); return os.IsNotExist(err) }, 2*time.Second, 2*time.Millisecond) } n.down.Store(false) - require.Eventually(t, func() bool { return n.count() == 3 }, 2*time.Second, 5*time.Millisecond) + require.Eventually(t, func() bool { return n.count() >= 3 }, 10*time.Second, 5*time.Millisecond) n.mu.Lock() defer n.mu.Unlock() - for i, name := range []string{"qa", "qb", "qc"} { - assert.Contains(t, n.msgs[i], strconv.Quote(name)) + // Each removal delivered exactly once, in any order: the outage ends + // asynchronously, possibly mid-flush, so a later item can send before an + // earlier one (hyperguild#193). Order is not what this test is about. + assert.Len(t, n.msgs, 3) + for _, name := range []string{"qa", "qb", "qc"} { + hits := 0 + for _, m := range n.msgs { + if strings.Contains(m, strconv.Quote(name)) { + hits++ + } + } + assert.Equal(t, 1, hits, "%s delivered exactly once", name) } } @@ -457,29 +468,30 @@ func TestWatchAccountsForEveryRemovalDuringAnOutage(t *testing.T) { require.Eventually(t, func() bool { _, err := os.Lstat(link); return os.IsNotExist(err) }, 2*time.Second, 1*time.Millisecond) } n.down.Store(false) - require.Eventually(t, func() bool { + // Synchronise on the record being complete, not on a fixed sleep after + // the first summary (hyperguild#193): sum every "removed N" alert and + // every suppression count until they account for all plants. + account := func() (delivered, counted int) { n.mu.Lock() defer n.mu.Unlock() for _, m := range n.msgs { - if strings.Contains(m, "suppressed") { - return true + var k int + if i := strings.Index(m, "removed "); i >= 0 && strings.Contains(m, "symlink(s)") { + if _, err := fmt.Sscanf(m[i+len("removed "):], "%d", &k); err == nil { + delivered += k + } + } + if i := strings.Index(m, "): "); i >= 0 && strings.Contains(m, "suppressed") { + if _, err := fmt.Sscanf(m[i+3:], "%d", &k); err == nil { + counted += k + } } } - return false - }, 2*time.Second, 5*time.Millisecond) - time.Sleep(50 * time.Millisecond) - n.mu.Lock() - defer n.mu.Unlock() - delivered, counted := 0, 0 - for _, m := range n.msgs { - if strings.Contains(m, "removed 1 symlink(s)") { - delivered++ - } - if i := strings.Index(m, "): "); i >= 0 && strings.Contains(m, "suppressed") { - _, _ = fmt.Sscanf(m[i+3:], "%d", &counted) - } + return delivered, counted } - assert.Equal(t, plants, delivered+counted, "delivered %d + counted %d must equal %d removals", delivered, counted, plants) + require.Eventually(t, func() bool { d, c := account(); return d+c >= plants }, 10*time.Second, 5*time.Millisecond) + d, c := account() + assert.Equal(t, plants, d+c, "delivered %d + counted %d must equal %d removals", d, c, plants) } // A poison message is still dropped (others send while it fails), and the