watcher: save state when it stops and wait for that save (closes #114)

The final save at shutdown came from the state's own stop hook, while
the watcher's stop hook only cancelled its run loop, so a check under
way could change state after that save or be cut off at exit. Run now
saves state as it returns, and the watcher's stop hook waits for Run,
bounded by the shutdown deadline. The state's own save stays; Save
holds the state lock for the whole write, so the two cannot overlap.
The start hook derives the watcher's context with WithoutCancel, so
the linter needs no exception. A new test stops a watcher built by New
and reads the change back from the state file, with no DNS.

Model: opus-5-5

internal/watcher/watcher_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"os"
 	"slices"
 	"sync"
 	"testing"
@@ -135,6 +136,7 @@ type testDeps struct {
 	notifier    *mockNotifier
 	state       *state.State
 	config      *config.Config
+	log         *logger.Logger
 }
 
 func newTestWatcher(
@@ -143,6 +145,23 @@ func newTestWatcher(
 ) (*watcher.Watcher, *testDeps) {
 	t.Helper()
 
+	deps := newTestDeps(t, cfg)
+
+	w := watcher.NewForTest(
+		deps.config,
+		deps.state,
+		resolver.NewFromLogger(slog.Default()),
+		deps.portChecker,
+		deps.tlsChecker,
+		deps.notifier,
+	)
+
+	return w, deps
+}
+
+func newTestDeps(t *testing.T, cfg *config.Config) *testDeps {
+	t.Helper()
+
 	deps := &testDeps{
 		portChecker: &mockPortChecker{},
 		tlsChecker: &mockTLSChecker{
@@ -157,30 +176,21 @@ func newTestWatcher(
 		t.Fatalf("globals.New: %v", err)
 	}
 
-	log, err := logger.New(nil, logger.Params{Globals: g})
+	deps.log, err = logger.New(nil, logger.Params{Globals: g})
 	if err != nil {
 		t.Fatalf("logger.New: %v", err)
 	}
 
 	// The watcher saves state after every check, into cfg.DataDir.
 	deps.state, err = state.New(fxtest.NewLifecycle(t), state.Params{
-		Logger: log,
+		Logger: deps.log,
 		Config: cfg,
 	})
 	if err != nil {
 		t.Fatalf("state.New: %v", err)
 	}
 
-	w := watcher.NewForTest(
-		deps.config,
-		deps.state,
-		resolver.NewFromLogger(slog.Default()),
-		deps.portChecker,
-		deps.tlsChecker,
-		deps.notifier,
-	)
-
-	return w, deps
+	return deps
 }
 
 func defaultTestConfig(t *testing.T) *config.Config {
@@ -539,6 +549,85 @@ func TestGracefulShutdown(t *testing.T) {
 	}
 }
 
+// TestStopSavesState stops a watcher built by New the way fx stops it,
+// and checks that a change made to the state after the last check is in
+// the state file afterwards. The state's own stop hook never runs here,
+// so only the watcher can have saved it. Nothing is configured to
+// check, so no DNS is involved.
+func TestStopSavesState(t *testing.T) {
+	t.Parallel()
+
+	cfg := defaultTestConfig(t)
+	deps := newTestDeps(t, cfg)
+	lc := fxtest.NewLifecycle(t)
+
+	_, err := watcher.New(lc, watcher.Params{
+		Logger:    deps.log,
+		Config:    cfg,
+		State:     deps.state,
+		Resolver:  resolver.NewFromLogger(slog.Default()),
+		PortCheck: deps.portChecker,
+		TLSCheck:  deps.tlsChecker,
+		Notify:    deps.notifier,
+	})
+	if err != nil {
+		t.Fatalf("watcher.New: %v", err)
+	}
+
+	lc.RequireStart()
+
+	// The first check saves state once. Wait for that save before
+	// changing the state, so the change can reach the file only
+	// through the save made at stop.
+	deadline := time.Now().Add(5 * time.Second)
+
+	for {
+		_, err = os.Stat(cfg.StatePath())
+		if err == nil {
+			break
+		}
+
+		if time.Now().After(deadline) {
+			t.Fatalf("the first check saved no state: %v", err)
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	deps.state.SetDomainState(testDomain, &state.DomainState{
+		Nameservers: []string{oldNS1},
+	})
+
+	ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	defer cancel()
+
+	err = lc.Stop(ctx)
+	if err != nil {
+		t.Fatalf("stopping the watcher: %v", err)
+	}
+
+	saved, err := state.New(fxtest.NewLifecycle(t), state.Params{
+		Logger: deps.log,
+		Config: cfg,
+	})
+	if err != nil {
+		t.Fatalf("state.New: %v", err)
+	}
+
+	err = saved.Load()
+	if err != nil {
+		t.Fatalf("loading the state file: %v", err)
+	}
+
+	ds, ok := saved.GetDomainState(testDomain)
+	if !ok || !slices.Equal(ds.Nameservers, []string{oldNS1}) {
+		t.Errorf(
+			"state file after stop has %+v for %s, want nameservers %v",
+			ds, testDomain, []string{oldNS1},
+		)
+	}
+}
+
 func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 	t.Parallel()