diff --git a/coordinator/internal/passctrl/converge_test.go b/coordinator/internal/passctrl/converge_test.go index 8968599..525febb 100644 --- a/coordinator/internal/passctrl/converge_test.go +++ b/coordinator/internal/passctrl/converge_test.go @@ -1,6 +1,8 @@ package passctrl import ( + "database/sql" + "errors" "testing" "drsync/coordinator/internal/model" @@ -47,6 +49,20 @@ func makeJob(t *testing.T, c *Controller, spec []byte) *store.Job { return job } +// makeDryRunJob is makeJob with dry_run set, for decideNextPass's dry-run +// pass-ceiling override. +func makeDryRunJob(t *testing.T, c *Controller, spec []byte) *store.Job { + t.Helper() + job, err := c.st.CreateJob("t1", spec, true, "") + if err != nil { + t.Fatal(err) + } + if err := c.st.SetJobState(job.ID, model.JobRunning); err != nil { + t.Fatal(err) + } + return job +} + func jobState(t *testing.T, c *Controller, id int64) model.JobState { t.Helper() j, err := c.st.GetJobByID(id) @@ -133,3 +149,33 @@ func TestThresholdConvergesEarly(t *testing.T) { t.Fatalf("delta under threshold should complete, got state %v", got) } } + +// A dry run writes nothing, so a second pass would just re-walk and re-diff +// the same unchanged trees — it must complete after pass 1 regardless of +// passes.max, without requiring the spec to also set max: 1 itself. Mirrors +// TestNonzeroDeltaSeedsNextPass's non-dry-run case, which expects the +// opposite outcome (another pass seeded) for the identical delta/ceiling. +func TestDryRunStopsAfterOnePass(t *testing.T) { + c := newController(t) + job := makeDryRunJob(t, c, withConverge(" max: 5\n")) + + done := &store.Pass{ID: 1, JobID: job.ID, PassNo: 1, FilesCopied: 42} + jobDone, converged, err := c.decideNextPass(job, done) + if err != nil { + t.Fatal(err) + } + if !jobDone { + t.Fatal("dry-run job should complete after pass 1, got jobDone=false") + } + if converged { + t.Fatal("dry-run completion is a ceiling stop, not real convergence (nonzero delta)") + } + if got := jobState(t, c, job.ID); got != model.JobCompleted { + t.Fatalf("dry-run job should be COMPLETED after pass 1, got state %v", got) + } + if p, err := c.st.PassByNo(job.ID, 2); err == nil { + t.Fatalf("dry-run job must not seed a second pass, but found one: %+v", p) + } else if !errors.Is(err, sql.ErrNoRows) { + t.Fatal(err) + } +} diff --git a/coordinator/internal/passctrl/passctrl.go b/coordinator/internal/passctrl/passctrl.go index 4f2822a..5fbfcfc 100644 --- a/coordinator/internal/passctrl/passctrl.go +++ b/coordinator/internal/passctrl/passctrl.go @@ -789,7 +789,16 @@ func (c *Controller) decideNextPass(job *store.Job, done *store.Pass) (jobDone, if cw.DeltaBytesBelow > 0 && uint64(done.BytesCopied) < uint64(cw.DeltaBytesBelow) { converged = true } - if converged || done.PassNo >= spec.Spec.Passes.Max { + // A dry run never writes anything, so a second pass would just re-walk and + // re-diff the same, unchanged source and destination trees — there is + // nothing a later pass could see that pass 1 didn't already report. Cap + // the effective ceiling to 1 regardless of what passes.max says, rather + // than requiring every dry-run job spec to also set passes.max: 1 itself. + maxPasses := spec.Spec.Passes.Max + if job.DryRun { + maxPasses = 1 + } + if converged || done.PassNo >= maxPasses { slog.Info("job converged", "job", job.Name, "passes", done.PassNo, "last_delta_files", done.FilesCopied, "last_delta_bytes", done.BytesCopied) err := c.st.SetJobState(job.ID, model.JobCompleted) diff --git a/docs/DESIGN-jobspec.md b/docs/DESIGN-jobspec.md index a9b1621..021f918 100644 --- a/docs/DESIGN-jobspec.md +++ b/docs/DESIGN-jobspec.md @@ -256,7 +256,11 @@ drsync ca init | issue --agent object internally. - `--dry-run` runs a full pass pipeline with copy/metadata/delete execution stubbed: everything is walked, diffed and journaled (`would_copy`, `would_delete`), giving an - exact preview and a free scan benchmark. + exact preview and a free scan benchmark. Always stops after pass 1 regardless of + `passes.max`: nothing is ever written, so a second pass would just re-walk and + re-diff the same unchanged trees (`passctrl.decideNextPass` caps the ceiling to 1 + whenever the job's dry-run flag is set — no need to also set `passes.max: 1` in + the spec). ## 3. Resolution Pipeline