Collapse the duplicate away-detail and panic tests
Two agents wrote the same three test helpers and names for the same two bugs. Kept the real assertions from dispatcher_test.go and removed the skipped placeholders they replace. panicSink stays in durability_test.go since both files use it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
@@ -787,13 +787,8 @@ func TestDispatchNudge_OutboxBeginFailureDoesNotBlockSend(t *testing.T) {
|
|||||||
|
|
||||||
// ----------------------- away channels carry no detail -----------------------
|
// ----------------------- away channels carry no detail -----------------------
|
||||||
|
|
||||||
// panicSink — a broken sink. models the #369 case: the sink blows up mid-send.
|
// panicSink lives in durability_test.go — a second agent wrote the same helper
|
||||||
type panicSink struct{ calls int }
|
// for the same #369 case, so this file just uses that one.
|
||||||
|
|
||||||
func (p *panicSink) Send(_ context.Context, _ Sendable) error {
|
|
||||||
p.calls++
|
|
||||||
panic("sink is broken")
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestAwaySendsGenericLineWhenSummaryEmpty — #368. The phraser is a small
|
// TestAwaySendsGenericLineWhenSummaryEmpty — #368. The phraser is a small
|
||||||
// model and drops fields often. An empty Summary must NOT put the full body
|
// model and drops fields often. An empty Summary must NOT put the full body
|
||||||
|
|||||||
@@ -223,25 +223,6 @@ func TestCompleteFailureLeavesRowPendingForReconciliation(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestPanicMidSendResolvesTheAttempt — a sink that panics leaves the attempt
|
// The panic gap this file used to describe as a skipped test is fixed and
|
||||||
// pending forever while the process keeps running: the dispatcher has no
|
// asserted for real in dispatcher_test.go:TestPanicMidSendResolvesTheAttempt.
|
||||||
// recover, and reconciliation only runs at startup. Written to the promise
|
// panicSink stays here because both files use it.
|
||||||
// ("never silently resent or dropped" implies every attempt gets resolved),
|
|
||||||
// skipped because the code does not keep it.
|
|
||||||
func TestPanicMidSendResolvesTheAttempt(t *testing.T) {
|
|
||||||
t.Skip("real gap: dispatcher.go:168 has no recover around Send, so a panicking sink leaves a permanent pending row (reconciliation only runs at startup, cmd/mavend/main.go:330)")
|
|
||||||
|
|
||||||
ob := &fakeOutbox{}
|
|
||||||
d := NewDispatcher(Config{Ntfy: &panicSink{}, Outbox: ob})
|
|
||||||
|
|
||||||
func() {
|
|
||||||
defer func() { _ = recover() }()
|
|
||||||
_, _ = d.DispatchNudge(context.Background(), PhrasedNudge{
|
|
||||||
Candidate: candidate("cert_expiring", loop.Sev3, store.Away),
|
|
||||||
Body: "detail", Summary: "short",
|
|
||||||
}, refNow())
|
|
||||||
}()
|
|
||||||
if len(ob.attempts) != 1 || ob.attempts[0].status == store.DeliveryPending {
|
|
||||||
t.Fatalf("a panic mid-send must still resolve the attempt, got %+v", ob.attempts)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -2,7 +2,6 @@ package delivery
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"strings"
|
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -200,47 +199,11 @@ func TestAwayChannelsGetMinimalBody(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestSev4AwaySendableCarriesNoDetail — DESIGN.md § Delivery: away channels
|
// Both away-detail gaps this file used to describe as skipped tests are now
|
||||||
// leave the box, so a sev4-away message must not carry detail beyond the short
|
// fixed and asserted for real in dispatcher_test.go:
|
||||||
// form. Today the dispatcher hands the away sink the FULL Body as well as the
|
// TestSev4AwaySendableCarriesNoDetail and TestAwaySendsGenericLineWhenSummaryEmpty.
|
||||||
// Summary (dispatcher.go:153-162 copies pn.Body into every Sendable) and
|
// An empty Summary no longer means "send the whole body" — it means a short
|
||||||
// trusts each sink to pick Summary. That works for the two sinks in-tree, but
|
// generic line — so the old expectation here was wrong as well as duplicated.
|
||||||
// the minimal body is not enforced at the dispatcher, so a new away sink that
|
|
||||||
// reads Body exfils by default.
|
|
||||||
func TestSev4AwaySendableCarriesNoDetail(t *testing.T) {
|
|
||||||
t.Skip("not enforced: dispatcher.go:159 puts the full Body on away sendables; minimal body is only enforced per-sink (ntfysink.go:77, telegramsink.go:148)")
|
|
||||||
|
|
||||||
telegram := &fakeSink{}
|
|
||||||
d := NewDispatcher(Config{Telegram: telegram, Ack: newFakeAck()})
|
|
||||||
detail := "disk /mnt/hdd1 at 97%, biggest offender /var/lib/docker"
|
|
||||||
|
|
||||||
if _, err := d.DispatchNudge(context.Background(), PhrasedNudge{
|
|
||||||
Candidate: candidate("disk_low", loop.Sev4, store.Away),
|
|
||||||
Body: detail, Summary: "disk low on homesrv",
|
|
||||||
}, refNow()); err != nil {
|
|
||||||
t.Fatalf("dispatch: %v", err)
|
|
||||||
}
|
|
||||||
if strings.Contains(telegram.sends[0].Body, "/var/lib/docker") {
|
|
||||||
t.Fatalf("away sendable carries detail: %q", telegram.sends[0].Body)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestAwayFallsBackToFullBodyWhenSummaryEmpty — the other half of the same
|
|
||||||
// gap: with no Summary, the full body leaves the box. The code chooses that on
|
|
||||||
// purpose ("a terse full message is better than no message",
|
|
||||||
// dispatcher.go:345-357), which contradicts the spec's minimal-body rule.
|
|
||||||
// Written to the spec, skipped because the code disagrees.
|
|
||||||
func TestAwayFallsBackToFullBodyWhenSummaryEmpty(t *testing.T) {
|
|
||||||
t.Skip("by design today: dispatcher.go:356 and ntfysink.go:79 fall back to the full Body when Summary is empty, so detail can leave the box")
|
|
||||||
|
|
||||||
msg := messageForChannel(Sendable{
|
|
||||||
Channel: ChannelNtfy,
|
|
||||||
Body: "internal detail that should never leave the box",
|
|
||||||
})
|
|
||||||
if msg != "" {
|
|
||||||
t.Fatalf("empty summary must not fall back to body, got %q", msg)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestCareAwayDropIsRecorded — DESIGN.md's drop is a decision ("a missed water
|
// TestCareAwayDropIsRecorded — DESIGN.md's drop is a decision ("a missed water
|
||||||
// nudge is noise, a missed backup failure isn't"), so it should be visible
|
// nudge is noise, a missed backup failure isn't"), so it should be visible
|
||||||
|
|||||||
Reference in New Issue
Block a user