From 16e4f8a1f23aee29dd5e86c32493763e3c75028b Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Tue, 4 Aug 2026 01:05:03 +0000 Subject: [PATCH] =?UTF-8?q?test(transport):=20coverage=20uplift=20to=20?= =?UTF-8?q?=E2=89=A570%=20(T01.3,=20REQ-057)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add retry_test.go (NEW) covering DefaultRetryPolicy, first-attempt success, idempotent-verb retry, idempotency-key retry, MaxAttempts exhaustion, zero-MaxAttempts defaulting, transient+non-idempotent+ no-key bail, ctx-cancel mid-backoff, exponential backoff growth + cap, and contains() substring helper. Extend idempotency_test.go with Sweep, empty-key Put/Get, and empty-key WithIdempotencyKey. Extend handshake_log_test.go with LogHandshakeFromCert happy path (real x509 cert → fingerprint) and FingerprintOfCert round-trip. Coverage: 84.6% → 93.0%. go test -race PASS. No production code changed; no new seams (httptest already covered DispatchClient). ---ci--- project: orca phase: 1 milestone: v0.8 status: execute ---/ci--- --- internal/transport/handshake_log_test.go | 31 ++++ internal/transport/idempotency_test.go | 37 +++++ internal/transport/retry_test.go | 203 +++++++++++++++++++++++ 3 files changed, 271 insertions(+) create mode 100644 internal/transport/retry_test.go diff --git a/internal/transport/handshake_log_test.go b/internal/transport/handshake_log_test.go index aa01092..3461501 100644 --- a/internal/transport/handshake_log_test.go +++ b/internal/transport/handshake_log_test.go @@ -2,8 +2,11 @@ package transport import ( "bytes" + "crypto/x509" + "encoding/pem" "errors" "log/slog" + "os" "strings" "testing" ) @@ -93,6 +96,34 @@ func TestLogHandshakeFromCert_NilCert(t *testing.T) { } } +func TestLogHandshakeFromCert_WithCert(t *testing.T) { + var buf bytes.Buffer + log := newTestLogger(&buf) + dir := t.TempDir() + certPath, _, _ := generateTestCerts(t, dir, "localhost") + certPEM, err := os.ReadFile(certPath) + if err != nil { + t.Fatalf("read cert: %v", err) + } + block, _ := pem.Decode(certPEM) + if block == nil { + t.Fatal("pem.Decode: no cert block") + } + leaf, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatalf("ParseCertificate: %v", err) + } + LogHandshakeFromCert(log, "peer-cert", leaf) + out := buf.String() + if !strings.Contains(out, "result=ok") { + t.Errorf("expected result=ok: %s", out) + } + expectedFP := FingerprintOfCert(leaf) + if !strings.Contains(out, "cert_fp="+expectedFP) { + t.Errorf("expected cert_fp=%s in: %s", expectedFP, out) + } +} + func TestFingerprintOfCert_Nil(t *testing.T) { if got := FingerprintOfCert(nil); got != "" { t.Errorf("FingerprintOfCert(nil) = %q, want empty", got) diff --git a/internal/transport/idempotency_test.go b/internal/transport/idempotency_test.go index bf979a1..135e0f3 100644 --- a/internal/transport/idempotency_test.go +++ b/internal/transport/idempotency_test.go @@ -112,6 +112,43 @@ func TestRetryContextCancel(t *testing.T) { } } +func TestIdempotencyStoreSweep(t *testing.T) { + s := NewIdempotencyStore() + s.Put("live-1", "job-1") + s.entries["expired"] = dedupeEntry{ + key: "expired", + jobID: "old-job", + expiresAt: time.Now().Add(-1 * time.Minute), + } + s.Sweep() + if _, ok := s.entries["expired"]; ok { + t.Error("Sweep did not remove expired entry") + } + if _, ok := s.entries["live-1"]; !ok { + t.Error("Sweep removed live entry") + } +} + +func TestIdempotencyStorePutEmpty(t *testing.T) { + s := NewIdempotencyStore() + s.Put("", "job-1") + s.Put("k1", "") + if _, ok := s.Get("k1"); ok { + t.Error("Put with empty jobID should not store") + } + if _, ok := s.Get(""); ok { + t.Error("Get with empty key should return false") + } +} + +func TestWithIdempotencyKeyEmpty(t *testing.T) { + ctx := context.Background() + got := WithIdempotencyKey(ctx, "") + if got != ctx { + t.Error("WithIdempotencyKey with empty key should return ctx unchanged") + } +} + func TestIsTransient(t *testing.T) { cases := []struct { err error diff --git a/internal/transport/retry_test.go b/internal/transport/retry_test.go new file mode 100644 index 0000000..e492acb --- /dev/null +++ b/internal/transport/retry_test.go @@ -0,0 +1,203 @@ +package transport + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestDefaultRetryPolicy(t *testing.T) { + p := DefaultRetryPolicy() + if p.Initial != RetryInitial { + t.Errorf("Initial = %v, want %v", p.Initial, RetryInitial) + } + if p.Max != RetryMax { + t.Errorf("Max = %v, want %v", p.Max, RetryMax) + } + if p.MaxAttempts != RetryMaxAttempts { + t.Errorf("MaxAttempts = %d, want %d", p.MaxAttempts, RetryMaxAttempts) + } +} + +func TestRetrySucceedsFirstAttempt(t *testing.T) { + calls := 0 + got, err := Do(context.Background(), DefaultRetryPolicy(), + func(_ context.Context, attempt int) (string, bool, error) { + calls++ + if attempt != 1 { + t.Errorf("attempt = %d, want 1", attempt) + } + return "ok", true, nil + }) + if err != nil { + t.Fatalf("Do: %v", err) + } + if got != "ok" { + t.Errorf("got = %q, want ok", got) + } + if calls != 1 { + t.Errorf("calls = %d, want 1", calls) + } +} + +func TestRetryIdempotentVerbRetries(t *testing.T) { + calls := 0 + _, err := Do(context.Background(), DefaultRetryPolicy(), + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", true, errors.New("connection refused") + }) + if err == nil { + t.Fatal("expected error after exhausting attempts") + } + if calls != RetryMaxAttempts { + t.Errorf("calls = %d, want %d", calls, RetryMaxAttempts) + } +} + +func TestRetryWithIdempotencyKeyRetries(t *testing.T) { + calls := 0 + ctx := WithIdempotencyKey(context.Background(), "key-1") + _, err := Do(ctx, DefaultRetryPolicy(), + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", false, errors.New("i/o timeout") + }) + if err == nil { + t.Fatal("expected error after exhausting attempts") + } + if calls != RetryMaxAttempts { + t.Errorf("calls = %d, want %d (idempotency key enables retry)", calls, RetryMaxAttempts) + } +} + +func TestRetryMaxAttemptsReached(t *testing.T) { + p := RetryPolicy{Initial: time.Millisecond, Max: 5 * time.Millisecond, MaxAttempts: 3} + calls := 0 + _, err := Do(context.Background(), p, + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", true, errors.New("EOF") + }) + if err == nil { + t.Fatal("expected error") + } + if !IsTransient(err) { + t.Errorf("expected transient error, got %v", err) + } + if calls != 3 { + t.Errorf("calls = %d, want 3", calls) + } +} + +func TestRetryZeroMaxAttemptsDefaults(t *testing.T) { + calls := 0 + p := RetryPolicy{} + _, err := Do(context.Background(), p, + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "ok", true, nil + }) + if err != nil { + t.Fatalf("Do: %v", err) + } + if calls != 1 { + t.Errorf("calls = %d, want 1", calls) + } +} + +func TestRetryNonTransientIdempotentRetries(t *testing.T) { + calls := 0 + _, err := Do(context.Background(), DefaultRetryPolicy(), + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", true, errors.New("invalid spec") + }) + if err == nil { + t.Fatal("expected error") + } + if calls != RetryMaxAttempts { + t.Errorf("calls = %d, want %d (non-transient idempotent still retries)", calls, RetryMaxAttempts) + } +} + +func TestRetryTransientNonIdempotentNoKeyBails(t *testing.T) { + calls := 0 + _, err := Do(context.Background(), DefaultRetryPolicy(), + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", false, errors.New("connection refused") + }) + if err == nil { + t.Fatal("expected error") + } + if calls != 1 { + t.Errorf("calls = %d, want 1 (transient+non-idempotent+no key = bail)", calls) + } +} + +func TestRetryContextCancelledMidBackoff(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + p := RetryPolicy{Initial: 100 * time.Millisecond, Max: time.Second, MaxAttempts: 5} + calls := 0 + go func() { + time.Sleep(20 * time.Millisecond) + cancel() + }() + _, err := Do(ctx, p, + func(_ context.Context, _ int) (string, bool, error) { + calls++ + return "", true, errors.New("connection refused") + }) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("expected context.Canceled, got %v", err) + } +} + +func TestBackoffGrowsExponentially(t *testing.T) { + initial := 10 * time.Millisecond + max := 1 * time.Second + d1 := backoff(initial, max, 1) + d2 := backoff(initial, max, 2) + d3 := backoff(initial, max, 3) + if d1 < 0 { + t.Errorf("backoff(1) = %v, want >= 0", d1) + } + if d2 < d1 { + t.Errorf("backoff(2)=%v < backoff(1)=%v (should grow)", d2, d1) + } + if d3 < d2 { + t.Errorf("backoff(3)=%v < backoff(2)=%v (should grow)", d3, d2) + } +} + +func TestBackoffCapsAtMax(t *testing.T) { + initial := 100 * time.Millisecond + max := 200 * time.Millisecond + d := backoff(initial, max, 10) + if d > max+max/2 { + t.Errorf("backoff(10) = %v, want <= ~max=%v", d, max) + } +} + +func TestContains(t *testing.T) { + cases := []struct { + s, sub string + want bool + }{ + {"hello world", "world", true}, + {"hello", "xyz", false}, + {"hello", "", true}, + {"", "", true}, + {"abc", "abcd", false}, + } + for _, c := range cases { + if got := contains(c.s, c.sub); got != c.want { + t.Errorf("contains(%q, %q) = %v, want %v", c.s, c.sub, got, c.want) + } + } +}