From 1350e681c9e8f67c5f27bb02cab9a54aaa41bdd6 Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Sat, 9 May 2026 20:34:51 -0700
Subject: [PATCH] test(s3/lifecycle): pin Pipeline.Run dependency + shard
 validation (Phase 15 slice) (#9402)

* test(s3/lifecycle): pin Pipeline.Run dependency + shard validation

Pre-existing TestPipelineRunRequiresDependencies only checked that an
empty Pipeline errors; it didn't pin which specific dependency must be
present. A refactor that makes one nilable accidentally would slip
through.

8 new tests pin every validation branch in Pipeline.Run: missing
Engine / Persister / Client / FilerClient each error with "missing
required dependency"; missing BucketsPath errors with its own
distinct message so operators can spot the missing wiring; ShardID =
-1 / ShardCount errors with a range message (covers the half-open
[0, ShardCount) boundary so a < to <= refactor can't introduce a
one-past-the-end shard); and a multi-shard config with one
out-of-range entry refuses the whole run rather than silently
disabling the rest.

* test(s3/lifecycle): refactor Pipeline.Run validation tests as table-driven

Per gemini review on #9402: collapse the eight per-branch tests into
TestPipelineRunValidation with a slice of (name, mutate, wantErr)
cases. Same coverage, ~30 fewer lines, idiomatic Go pattern that
makes adding a new validation case trivial.
---
 .../s3lifecycle/dispatcher/pipeline_test.go   | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/weed/s3api/s3lifecycle/dispatcher/pipeline_test.go b/weed/s3api/s3lifecycle/dispatcher/pipeline_test.go
index 36c7f3628..1020ea519 100644
--- a/weed/s3api/s3lifecycle/dispatcher/pipeline_test.go
+++ b/weed/s3api/s3lifecycle/dispatcher/pipeline_test.go
@@ -2,6 +2,7 @@ package dispatcher
 
 import (
 	"context"
+	"strings"
 	"testing"
 	"time"
 
@@ -82,3 +83,59 @@ func TestPipelineRunRequiresDependencies(t *testing.T) {
 		t.Fatal("expected error for empty Pipeline")
 	}
 }
+
+// stubFilerClient satisfies filer_pb.SeaweedFilerClient just enough to
+// pass the nil-check in Pipeline.Run; methods would panic if called,
+// but the validation tests below all return before any RPC.
+type stubFilerClient struct {
+	filer_pb.SeaweedFilerClient
+}
+
+// fullPipeline assembles a Pipeline whose dependencies all pass the
+// nil-check, so individual tests can knock out one piece at a time
+// to exercise specific validation branches.
+func fullPipeline() *Pipeline {
+	return &Pipeline{
+		Engine:      engine.New(),
+		Persister:   reader.NewInMemoryPersister(),
+		Client:      &fakeClient{},
+		FilerClient: &stubFilerClient{},
+		BucketsPath: "/buckets",
+		ShardID:     0,
+	}
+}
+
+func TestPipelineRunValidation(t *testing.T) {
+	// Each case mutates one piece of a fullPipeline() and asserts the
+	// expected error fragment. Per-dependency cases pin that the nil
+	// check exercises every required field individually; the Buckets-
+	// Path case asserts the distinct error message; the shard cases
+	// pin the half-open [0, ShardCount) range and that any one bad
+	// entry refuses the whole multi-shard run.
+	cases := []struct {
+		name    string
+		mutate  func(*Pipeline)
+		wantErr string
+	}{
+		{"missing Engine", func(p *Pipeline) { p.Engine = nil }, "missing required dependency"},
+		{"missing Persister", func(p *Pipeline) { p.Persister = nil }, "missing required dependency"},
+		{"missing Client", func(p *Pipeline) { p.Client = nil }, "missing required dependency"},
+		{"missing FilerClient", func(p *Pipeline) { p.FilerClient = nil }, "missing required dependency"},
+		{"missing BucketsPath", func(p *Pipeline) { p.BucketsPath = "" }, "BucketsPath required"},
+		{"negative ShardID", func(p *Pipeline) { p.ShardID = -1 }, "out of"},
+		{"ShardID at boundary", func(p *Pipeline) { p.ShardID = s3lifecycle.ShardCount }, "out of"},
+		{"multi-shard out of range", func(p *Pipeline) {
+			p.Shards = []int{0, 1, s3lifecycle.ShardCount + 1}
+		}, "out of"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			p := fullPipeline()
+			tc.mutate(p)
+			err := p.Run(context.Background())
+			if err == nil || !strings.Contains(err.Error(), tc.wantErr) {
+				t.Fatalf("expected error containing %q, got %v", tc.wantErr, err)
+			}
+		})
+	}
+}