From 7397051771f57899e7445b32da483c49a914e697 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 10:53:12 +0200 Subject: [PATCH] =?UTF-8?q?mesh-mail:=20tighten=20tests=20that=20proved=20?= =?UTF-8?q?less=20than=20their=20criterion=20=E2=80=94=20attempt=205=20and?= =?UTF-8?q?=20no=20mail=20received=20(1198/AC4),=20mail=5Fhealth=20actuall?= =?UTF-8?q?y=20turning=20unhealthy=20(1199/AC2),=20and=20the=20words=20mai?= =?UTF-8?q?l=5Fsend=5Fstatus=20gives,=20not=20only=20its=20status=20(1199/?= =?UTF-8?q?AC4)=20(review=20round=201=20finding=206)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mesh-mail/acceptance_integration_test.go | 10 ++++-- .../mesh-mail/cmd/mesh-mail/health_test.go | 36 +++++++++++++++++++ .../cmd/mesh-mail/smtp_classify_test.go | 9 ++++- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/modules/mesh-mail/cmd/mesh-mail/acceptance_integration_test.go b/modules/mesh-mail/cmd/mesh-mail/acceptance_integration_test.go index 958d17f9..a499ed6d 100644 --- a/modules/mesh-mail/cmd/mesh-mail/acceptance_integration_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/acceptance_integration_test.go @@ -273,8 +273,14 @@ func TestMailSenderAcceptanceARefusedSignInFailsAfterItsAttemptsAndNothingIsRece <-done kept, _, ok, err := getSend(store, id) - if err != nil || !ok || kept.Decided == nil || kept.Decided.Outcome == "sent" { - t.Fatalf("the kept send ended as sent, or could not be read: %v %v %+v", err, ok, kept.Decided) + if err != nil || !ok || kept.Decided == nil || !strings.HasPrefix(kept.Decided.Outcome, "failed: ") { + t.Fatalf("the kept send did not end failed: %v %v %+v", err, ok, kept.Decided) + } + if kept.Decided.Attempt != MostAttempts { + t.Fatalf("failed after %d attempt(s), want all %d", kept.Decided.Attempt, MostAttempts) + } + if n := s.receivedCount(); n != 0 { + t.Fatalf("the scripted server received %d message(s), want none: a refused sign-in never reaches DATA", n) } } diff --git a/modules/mesh-mail/cmd/mesh-mail/health_test.go b/modules/mesh-mail/cmd/mesh-mail/health_test.go index 44252013..67a642ee 100644 --- a/modules/mesh-mail/cmd/mesh-mail/health_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/health_test.go @@ -114,3 +114,39 @@ func TestTheWatchJudgesFromTheFilesAndTheIndex(t *testing.T) { t.Fatalf("the status file's shape: %s", raw) } } + +// 1199/AC2: mail_health (the tool, not only Overdue or WithOverdue in isolation) turns unhealthy naming an +// overdue send, even while the watch's own verdict is healthy (novox/hq review round 1 finding 6). +func TestMailHealthToolTurnsUnhealthyNamingAnOverdueSend(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "health.json") + if err := writeStatus(path, Status{Answer: health.Healthy("1 account(s) watched"), At: judgedAt}); err != nil { + t.Fatal(err) + } + sends := newMemSends() + overdueID := "send-overdue1" + req := SendRequest{ID: overdueID, Account: "mesh", At: judgedAt} + req.Decided = &SendDecision{At: judgedAt.Add(-20 * time.Minute), Outcome: "queued"} + if _, err := sends.Create(req.ID, req); err != nil { + t.Fatal(err) + } + if err := addSenderPending(sends, req.ID); err != nil { + t.Fatal(err) + } + b := &Bundle{Env: Env{Status: path}, Sends: sends, Now: func() time.Time { return judgedAt }} + + out, err := b.healthTool(nil) + if err != nil { + t.Fatalf("mail_health: %v", err) + } + a, ok := out.(health.Answer) + if !ok { + t.Fatalf("mail_health answered %T, want health.Answer", out) + } + if a.State != health.StateUnhealthy { + t.Fatalf("mail_health: %+v, want unhealthy", a) + } + if !strings.Contains(a.Why, overdueID) { + t.Fatalf("mail_health's why does not name the overdue send: %q", a.Why) + } +} diff --git a/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go b/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go index 28fcabd0..97bc90f5 100644 --- a/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go @@ -12,6 +12,7 @@ import ( "net" "net/smtp" "strings" + "sync/atomic" "testing" ) @@ -31,6 +32,7 @@ func drop() step { return step{drop: true} } type scriptedSMTPServer struct { ln net.Listener ehlo, auth, mail, rcpt, dataOpen, dataEnd step + received int32 // atomic: how many full DATA bodies this server has read } func newScriptedSMTPServer(t *testing.T) *scriptedSMTPServer { @@ -48,6 +50,10 @@ func newScriptedSMTPServer(t *testing.T) *scriptedSMTPServer { func (s *scriptedSMTPServer) addr() string { return s.ln.Addr().String() } +// receivedCount is how many full DATA bodies this server has read, for a test to prove nothing was +// received at all (novox/hq review round 1 finding 6). +func (s *scriptedSMTPServer) receivedCount() int { return int(atomic.LoadInt32(&s.received)) } + func (s *scriptedSMTPServer) dialer() SMTPDialer { addr := s.addr() return func(Server) (*smtp.Client, error) { return smtp.Dial(addr) } @@ -115,7 +121,8 @@ func (s *scriptedSMTPServer) handle(conn net.Conn) { break } } - s.send(conn, s.dataEnd) // a drop here is "the connection lost after the data was written" + atomic.AddInt32(&s.received, 1) // a full message body was read, whatever dataEnd then says + s.send(conn, s.dataEnd) // a drop here is "the connection lost after the data was written" return case upper == "QUIT": fmt.Fprintf(conn, "221 bye\r\n")