diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go index 8f0a801b..99693fa4 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go @@ -283,20 +283,24 @@ func RunOneBatch(ctx context.Context, store Store, readText ReadTextFunc, judge return recordOutcome(ctx, store, OutcomeOK, "", judgedAt) } -// RunBatches is the loop the running module drives (main.go): pace batch starts at least BatchGap apart +// RunBatches is the loop the running module drives (main.go): pace batch starts at least gap apart // (nextBatchWait), take at most MaxMailsPerBatch of the waiting mails in arrival order, run one batch at a // time, and after each one — win or lose — try the mail signal (SendSignal, tell.go, issue 1522; it is its // own no-op when there is nothing untold that matters, or the reader is unhealthy) — forever, until ctx is -// done. Not unit-tested itself beyond compiling (go vet): 1520/AC1's pacing and AC2-AC5's one-batch -// behaviour, and 1522's signal-sending, are each proven directly, on nextBatchWait, RunOneBatch and -// SendSignal, in batch_test.go and tell_test.go. -func RunBatches(ctx context.Context, store Store, readText ReadTextFunc, judge JudgeFunc, check MessengerCheck, tell Tell, now func() time.Time, sleep func(time.Duration), logf func(string, ...any)) { +// done. gap is given explicitly (main.go's batchLoop passes the var BatchGap, which only a `-tags +// integration` build ever shortens; TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart passes +// MinBatchGap) so this function itself never reads that var — the try on 8000b637 found this test still +// reading it (and so, compiled into the integration binary, pacing at the shortened 3s instead of 160s) +// when RunBatches read BatchGap directly inside its own loop (review round 2, issue 1520). 1520/AC2-AC5's +// one-batch behaviour and 1522's signal-sending are proven directly, on RunOneBatch and SendSignal, in +// batch_test.go and tell_test.go. +func RunBatches(ctx context.Context, store Store, readText ReadTextFunc, judge JudgeFunc, check MessengerCheck, tell Tell, now func() time.Time, sleep func(time.Duration), logf func(string, ...any), gap time.Duration) { var lastStart time.Time for { if ctx.Err() != nil { return } - if wait := nextBatchWait(lastStart, now(), BatchGap); wait > 0 { + if wait := nextBatchWait(lastStart, now(), gap); wait > 0 { sleep(wait) continue } diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go index 2403babd..c882125b 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go @@ -122,7 +122,7 @@ func TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart(t *testing.T) { done := make(chan struct{}) go func() { - RunBatches(ctx, store, noopReadText, judge, check, tell, now, sleep, noopLogf) + RunBatches(ctx, store, noopReadText, judge, check, tell, now, sleep, noopLogf, MinBatchGap) close(done) }() select { diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go index a45480cb..b81cb62b 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/catchup_integration_test.go @@ -128,10 +128,14 @@ func TestAcceptanceRefusedJudgementIsUnhealthyThenCatchesUpInPublishedOrder(t *t if !sinceRE.MatchString(why) { t.Fatalf("health's why = %q, want it to name since when (HH:MM YYYY-MM-DD, health.go's own Answer)", why) } - before := tellCalls.get() + // Exactly 1 (the dropped tell of the healthy phase, above) — never a relative "unchanged since some + // point after unhealthy was already seen", which a tell sent in the SAME loop pass that made the + // reader unhealthy would already be baked into (review round 2, issue 1524): waitFor's own poll above + // only reads health, so a wrongly-sent tell from that very batch would predate this read and hide + // behind it. time.Sleep(2 * time.Second) // give the next failed batch attempt, if any, time to run and fail again - if n := tellCalls.get(); n != before { - t.Fatalf("a tell was sent while unhealthy: %d call(s), want still %d (the untold mail from above)", n, before) + if n := tellCalls.get(); n != 1 { + t.Fatalf("tellCalls = %d, want exactly 1 (the dropped tell from the healthy phase; none while unhealthy)", n) } // The judge answers again (1524/AC2): catch-up in the order the two mails were published. diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/main.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/main.go index 2478127e..e85b95a2 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/main.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/main.go @@ -100,7 +100,7 @@ func batchLoop(storeFor func(context.Context) (Store, error)) { judge := NewJudge(bundleAsk, node, time.Now) check := NewMessengerCheck(bundleAsk) tell := NewTell(bundleAsk) - RunBatches(context.Background(), store, readText, judge, check, tell, time.Now, time.Sleep, logf) + RunBatches(context.Background(), store, readText, judge, check, tell, time.Now, time.Sleep, logf, BatchGap) } // ---- hearing --------------------------------------------------------------------------------------------------