test(fixtures): simplify client, tool and service scenarios

commands_test.go

@@ -209,9 +209,7 @@ func TestDispatchUnexpectedPositionals(t *testing.T) {
 }
 
 func TestDispatchVersion(t *testing.T) {
-	oldV, oldC := version.Version, version.Commit
-	version.Version, version.Commit = "v9.9.9", "abcdef1"
-	defer func() { version.Version, version.Commit = oldV, oldC }()
+	setVersion(t, "v9.9.9", "abcdef1")
 
 	for _, argv := range [][]string{
 		{"lightcode", "version"},
@@ -233,9 +231,7 @@ func TestDispatchVersion(t *testing.T) {
 }
 
 func TestDispatchVersionJSON(t *testing.T) {
-	oldV, oldC := version.Version, version.Commit
-	version.Version, version.Commit = "v9.9.9", "abcdef1"
-	defer func() { version.Version, version.Commit = oldV, oldC }()
+	setVersion(t, "v9.9.9", "abcdef1")
 
 	_, code, stdout, stderr := capture(t, []string{"lightcode", "version", "--json"})
 	if code != 0 || stderr != "" {

internal/atomicfs/atomicfs_test.go

@@ -103,103 +103,75 @@ func TestCreateExclusiveRefusesOverwrite(t *testing.T) {
 	}
 }
 
-// TestWriteSyncFailureBeforePublicationAborts proves a pre-publication temp
-// sync failure is an ordinary failed write: Write returns the error, leaves
-// no temp behind, and does not publish the file.
-func TestWriteSyncFailureBeforePublicationAborts(t *testing.T) {
-	dir := t.TempDir()
-	path := filepath.Join(dir, "rec.json")
-
-	injected := errors.New("injected temp sync failure")
-	SyncFileFunc = func(*os.File) error { return injected }
-	defer func() { SyncFileFunc = nil }()
-
-	err := Write(path, []byte("data"), 0o600)
-	if err == nil {
-		t.Fatal("Write succeeded; want the injected temp sync failure")
-	}
-	if !strings.Contains(err.Error(), "sync temp") {
-		t.Fatalf("error = %v, want a temp sync error", err)
-	}
-	if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
-		t.Fatalf("path exists after the aborted write: %v", statErr)
-	}
-	entries, _ := os.ReadDir(dir)
-	if len(entries) != 0 {
-		t.Fatalf("leftover entries after the aborted write: %v", entries)
-	}
-}
-
-// TestWriteSyncFailureBeforePublicationPreservesExistingContent proves a
-// pre-publication temp sync failure on a present target is an ordinary
-// failed write that leaves the old complete contents intact: Write returns
-// the error, removes the temp, and the destination still holds the bytes it
-// held before the call. The sibling
-// TestWriteSyncFailureBeforePublicationAborts starts from an absent path, so
-// its assertions cannot distinguish removing the temp from removing the
-// target — only a present target can see the difference.
-func TestWriteSyncFailureBeforePublicationPreservesExistingContent(t *testing.T) {
-	dir := t.TempDir()
-	path := filepath.Join(dir, "rec.json")
-	if err := Write(path, []byte("original"), 0o600); err != nil {
-		t.Fatalf("seed write: %v", err)
-	}
-
-	injected := errors.New("injected temp sync failure")
-	SyncFileFunc = func(*os.File) error { return injected }
-	defer func() { SyncFileFunc = nil }()
-
-	err := Write(path, []byte("replacement"), 0o600)
-	if err == nil {
-		t.Fatal("Write succeeded; want the injected temp sync failure")
-	}
-	got, readErr := os.ReadFile(path)
-	if readErr != nil {
-		t.Fatalf("read destination: %v", readErr)
-	}
-	if string(got) != "original" {
-		t.Fatalf("destination = %q after the failed write, want original", got)
-	}
-	entries, _ := os.ReadDir(dir)
-	if len(entries) != 1 || entries[0].Name() != "rec.json" {
-		t.Fatalf("leftover entries after the aborted write: %v", entries)
-	}
-}
-
-// TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent
-// proves the same present-target rule for the link-based publisher:
-// CreateExclusive returns the error, leaves no temp behind (its defer removes
-// it), and the existing file keeps its original contents.
-func TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent(t *testing.T) {
-	dir := t.TempDir()
-	path := filepath.Join(dir, "rec.json")
+// TestPrePublicationSyncFailureRows covers both publishers with absent and
+// present destinations. A pre-publication temp sync failure is an ordinary
+// failed write: the publisher returns the error, leaves no temp behind, and
+// publishes nothing. A present destination also distinguishes removing the
+// temp from removing the target: only it can see the old complete contents
+// survive the failed replacement.
+func TestPrePublicationSyncFailureRows(t *testing.T) {
+	writePublisher := func(path string, data []byte, mode os.FileMode) (bool, error) {
+		err := Write(path, data, mode)
+		return err == nil, err
+	}
+	cases := []struct {
+		name        string
+		publish     func(string, []byte, os.FileMode) (bool, error)
+		present     bool
+		wantErrPart string
+	}{
+		{"TestWriteSyncFailureBeforePublicationAborts", writePublisher, false, "sync temp"},
+		{"TestWriteSyncFailureBeforePublicationPreservesExistingContent", writePublisher, true, ""},
+		{"TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent", CreateExclusive, true, ""},
+		{"TestCreateExclusiveSyncFailureBeforePublicationAborts", CreateExclusive, false, ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			path := filepath.Join(dir, "rec.json")
+
+			if tc.present {
+				created, err := tc.publish(path, []byte("original"), 0o600)
+				if err != nil || !created {
+					t.Fatalf("seed publish: (%v, %v)", created, err)
+				}
+			}
 
-	created, err := CreateExclusive(path, []byte("original"), 0o600)
-	if err != nil || !created {
-		t.Fatalf("seed create: (%v, %v)", created, err)
-	}
+			injected := errors.New("injected temp sync failure")
+			SyncFileFunc = func(*os.File) error { return injected }
+			defer func() { SyncFileFunc = nil }()
 
-	injected := errors.New("injected temp sync failure")
-	SyncFileFunc = func(*os.File) error { return injected }
-	defer func() { SyncFileFunc = nil }()
+			created, err := tc.publish(path, []byte("replacement"), 0o600)
+			if err == nil {
+				t.Fatal("publish succeeded; want the injected temp sync failure")
+			}
+			if created {
+				t.Fatal("created = true on a failed write")
+			}
+			if tc.wantErrPart != "" && !strings.Contains(err.Error(), tc.wantErrPart) {
+				t.Fatalf("error = %v, want a %s error", err, tc.wantErrPart)
+			}
 
-	created, err = CreateExclusive(path, []byte("replacement"), 0o600)
-	if err == nil {
-		t.Fatal("CreateExclusive succeeded; want the injected temp sync failure")
-	}
-	if created {
-		t.Fatal("created = true on a failed write")
-	}
-	got, readErr := os.ReadFile(path)
-	if readErr != nil {
-		t.Fatalf("read destination: %v", readErr)
-	}
-	if string(got) != "original" {
-		t.Fatalf("destination = %q after the failed write, want original", got)
-	}
-	entries, _ := os.ReadDir(dir)
-	if len(entries) != 1 || entries[0].Name() != "rec.json" {
-		t.Fatalf("leftover entries after the aborted write: %v", entries)
+			if tc.present {
+				got, readErr := os.ReadFile(path)
+				if readErr != nil {
+					t.Fatalf("read destination: %v", readErr)
+				}
+				if string(got) != "original" {
+					t.Fatalf("destination = %q after the failed write, want original", got)
+				}
+			} else if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
+				t.Fatalf("path exists after the aborted write: %v", statErr)
+			}
+			entries, _ := os.ReadDir(dir)
+			if tc.present {
+				if len(entries) != 1 || entries[0].Name() != "rec.json" {
+					t.Fatalf("leftover entries after the aborted write: %v", entries)
+				}
+			} else if len(entries) != 0 {
+				t.Fatalf("leftover entries after the aborted write: %v", entries)
+			}
+		})
 	}
 }
 
@@ -235,34 +207,6 @@ func TestWriteDirSyncFailureAfterPublicationReturnsSuccess(t *testing.T) {
 	}
 }
 
-// TestCreateExclusiveSyncFailureBeforePublicationAborts proves the same
-// pre-publication rule for the link-based publisher: CreateExclusive
-// returns the error, leaves no temp behind (its defer removes it), and
-// does not publish the file.
-func TestCreateExclusiveSyncFailureBeforePublicationAborts(t *testing.T) {
-	dir := t.TempDir()
-	path := filepath.Join(dir, "rec.json")
-
-	injected := errors.New("injected temp sync failure")
-	SyncFileFunc = func(*os.File) error { return injected }
-	defer func() { SyncFileFunc = nil }()
-
-	created, err := CreateExclusive(path, []byte("data"), 0o600)
-	if err == nil {
-		t.Fatal("CreateExclusive succeeded; want the injected temp sync failure")
-	}
-	if created {
-		t.Fatal("created = true on a failed write")
-	}
-	if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
-		t.Fatalf("path exists after the aborted write: %v", statErr)
-	}
-	entries, _ := os.ReadDir(dir)
-	if len(entries) != 0 {
-		t.Fatalf("leftover entries after the aborted write: %v", entries)
-	}
-}
-
 // TestCreateExclusiveDirSyncFailureAfterPublicationReturnsSuccess proves
 // the post-publication rule for the link-based publisher: CreateExclusive
 // returns (true, nil), the file stays published, and the diagnostic is

internal/config/managedenv_cross_test.go

@@ -16,7 +16,7 @@ func TestManagedEnvCrossInstanceOperations(t *testing.T) {
 	envPath := filepath.Join(dir, ".env")
 
 	newMgr := func() *ManagedEnv {
-		return &ManagedEnv{path: envPath, managed: map[string]struct{}{}}
+		return NewManagedEnvForTest(envPath)
 	}
 
 	keys := []string{"LIGHTCODE_TEST_A", "LIGHTCODE_TEST_B", "LIGHTCODE_TEST_C"}

internal/lsp/instance_ready_test.go

@@ -147,6 +147,37 @@ func newFakeServerInstanceWithLifetime(t *testing.T, log, failMarker string, ini
 	return inst
 }
 
+// newPIDRecordingFakeServer builds the fake server behind a shell wrapper
+// that records the launched process's pid before exec'ing the real script, so
+// a test can observe whether a process was launched and whether it was
+// reaped. The pidfile and the launch log use separate temporary directories.
+func newPIDRecordingFakeServer(t *testing.T, initDelay string) (home, pidfile, log string, def *server.Definition) {
+	t.Helper()
+	home = t.TempDir()
+	cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake")
+	if err := os.MkdirAll(cacheDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	real := filepath.Join(cacheDir, "fake-lsp.py")
+	if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	wrapper := filepath.Join(cacheDir, "fake-lsp")
+	wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n"
+	if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	pidfile = filepath.Join(t.TempDir(), "lsp.pid")
+	t.Setenv("FAKE_PIDFILE", pidfile)
+	log = filepath.Join(t.TempDir(), "launches.log")
+	def = &server.Definition{
+		Name:    "fake",
+		Command: "fake-lsp",
+		Args:    []string{log, "", initDelay, "0", "", "1"},
+	}
+	return home, pidfile, log, def
+}
+
 func countLaunches(log string) int {
 	b, err := os.ReadFile(log)
 	if err != nil {
@@ -435,31 +466,8 @@ func TestStartRefusedWhileReady(t *testing.T) {
 // between cmd.Start and the post-launch attempt-identity recheck, and the
 // waitCommand seam counts exactly one wait on this side of the handoff.
 func TestStartAfterPermanentCloseSelfReaps(t *testing.T) {
-	home := t.TempDir()
-	cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake")
-	if err := os.MkdirAll(cacheDir, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	real := filepath.Join(cacheDir, "fake-lsp.py")
-	if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	// The wrapper records the launched process's pid before exec'ing the real
-	// script, so the test can assert the close loser was actually reaped.
-	wrapper := filepath.Join(cacheDir, "fake-lsp")
-	wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n"
-	if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	pidfile := filepath.Join(t.TempDir(), "lsp.pid")
-	t.Setenv("FAKE_PIDFILE", pidfile)
-
-	log := filepath.Join(t.TempDir(), "launches.log")
-	inst := newInstance(&server.Definition{
-		Name:    "fake",
-		Command: "fake-lsp",
-		Args:    []string{log, "", "0", "0", "", "1"},
-	}, t.TempDir(), home, context.Background(), nil)
+	home, pidfile, log, def := newPIDRecordingFakeServer(t, "0")
+	inst := newInstance(def, t.TempDir(), home, context.Background(), nil)
 	t.Cleanup(func() { inst.shutdown() })
 
 	// Count every instance-owned process wait through the seam.