Fix integration test isolation for CI reliability

Use zfs inherit instead of zfs set prop=- for property cleanup (the
latter sets a literal "-" value). Add pre-test cleanup to all integration
tests so leftover state from prior runs can't cause false failures. Use
unique timestamps per test to prevent snapshot name collisions.

Author: ndemarco

internal/daemon/daemon_integration_test.go

@@ -41,7 +41,7 @@ func clearAllProps(t *testing.T, client *zfs.Client) {
 			"org.zvolta:snapshot-weekly", "org.zvolta:snapshot-monthly",
 			"org.zvolta:snapshot-yearly",
 		} {
-			_ = client.SetProperty(ds, prop, "-")
+			_ = client.InheritProperty(ds, prop)
 		}
 	}
 }
@@ -56,10 +56,17 @@ func destroyAllTestSnapshots(t *testing.T, mgr *snapshot.Manager, dataset string
 	}
 }
 
+func cleanAll(t *testing.T, d *Daemon) {
+	t.Helper()
+	clearAllProps(t, d.ZFS)
+	destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	destroyAllTestSnapshots(t, d.Snapshots, testPool)
+}
+
 func TestIntegrationTickCreatesSnapshots(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
-	defer destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	// Enable autosnap with hourly and daily on the dataset
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autosnap", "on")
@@ -88,8 +95,8 @@ func TestIntegrationTickCreatesSnapshots(t *testing.T) {
 
 func TestIntegrationTickIdempotentWithinBoundary(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
-	defer destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autosnap", "on")
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:snapshot-hourly", "24")
@@ -119,8 +126,8 @@ func TestIntegrationTickIdempotentWithinBoundary(t *testing.T) {
 
 func TestIntegrationTickCrossesHourBoundary(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
-	defer destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autosnap", "on")
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:snapshot-hourly", "24")
@@ -148,8 +155,8 @@ func TestIntegrationTickCrossesHourBoundary(t *testing.T) {
 
 func TestIntegrationTickPrunes(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
-	defer destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autosnap", "on")
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autoprune", "on")
@@ -185,8 +192,8 @@ func TestIntegrationTickPrunes(t *testing.T) {
 
 func TestIntegrationTickFrequent(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
-	defer destroyAllTestSnapshots(t, d.Snapshots, testDataset)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:autosnap", "on")
 	setProperty(t, d.ZFS, testDataset, "org.zvolta:snapshot-frequent", "12")
@@ -228,7 +235,8 @@ func TestIntegrationTickFrequent(t *testing.T) {
 
 func TestIntegrationTickDisabledDataset(t *testing.T) {
 	d := setupDaemon(t)
-	defer clearAllProps(t, d.ZFS)
+	cleanAll(t, d)
+	defer cleanAll(t, d)
 
 	// Don't set autosnap — should be disabled by default
 	now := time.Date(2026, 6, 15, 14, 30, 0, 0, time.UTC)

internal/policy/policy_integration_test.go

@@ -28,13 +28,15 @@ func setProps(t *testing.T, client *zfs.Client, dataset string, props map[string
 func clearProps(t *testing.T, client *zfs.Client, dataset string) {
 	t.Helper()
 	for _, prop := range AllProperties() {
-		_ = client.SetProperty(dataset, prop, "-")
+		_ = client.InheritProperty(dataset, prop)
 	}
 }
 
 func TestIntegrationResolve(t *testing.T) {
 	engine := testEngine()
 	client := engine.ZFS
+	clearProps(t, client, testPool)
+	clearProps(t, client, testDataset)
 
 	setProps(t, client, testDataset, map[string]string{
 		"org.zvolta:autosnap":                 "on",
@@ -77,6 +79,8 @@ func TestIntegrationResolve(t *testing.T) {
 func TestIntegrationInheritance(t *testing.T) {
 	engine := testEngine()
 	client := engine.ZFS
+	clearProps(t, client, testPool)
+	clearProps(t, client, testDataset)
 
 	// Set on parent pool
 	setProps(t, client, testPool, map[string]string{
@@ -115,6 +119,8 @@ func TestIntegrationInheritance(t *testing.T) {
 func TestIntegrationResolveAll(t *testing.T) {
 	engine := testEngine()
 	client := engine.ZFS
+	clearProps(t, client, testPool)
+	clearProps(t, client, testDataset)
 
 	setProps(t, client, testPool, map[string]string{
 		"org.zvolta:autosnap":        "on",
@@ -128,7 +134,6 @@ func TestIntegrationResolveAll(t *testing.T) {
 		t.Fatalf("ResolveAll: %v", err)
 	}
 
-	// Should find at least the pool and child dataset
 	if len(policies) < 2 {
 		t.Errorf("expected at least 2 policies, got %d", len(policies))
 	}
@@ -150,10 +155,8 @@ func TestIntegrationResolveAll(t *testing.T) {
 func TestIntegrationDefaultDisabled(t *testing.T) {
 	engine := testEngine()
 	client := engine.ZFS
-
-	// Clear everything — should default to autosnap=off
-	clearProps(t, client, testDataset)
 	clearProps(t, client, testPool)
+	clearProps(t, client, testDataset)
 
 	p, err := engine.Resolve(testDataset)
 	if err != nil {

internal/snapshot/manager_integration_test.go

@@ -21,19 +21,31 @@ func testManager() *Manager {
 	}
 }
 
+// cleanSnapshots destroys all zvolta-managed snapshots on the test dataset.
+func cleanSnapshots(t *testing.T, mgr *Manager) {
+	t.Helper()
+	grouped, _ := mgr.ListManaged(testDataset)
+	for _, snaps := range grouped {
+		for _, s := range snaps {
+			_ = mgr.ZFS.DestroySnapshot(s.Snapshot.Dataset, s.Snapshot.SnapName)
+		}
+	}
+}
+
 func TestIntegrationCreateAndList(t *testing.T) {
 	mgr := testManager()
-	now := time.Date(2026, 3, 5, 10, 0, 0, 0, time.UTC)
+	cleanSnapshots(t, mgr)
+	defer cleanSnapshots(t, mgr)
+
+	now := time.Date(2025, 1, 1, 10, 0, 0, 0, time.UTC)
 
-	// Create snapshots across multiple tiers
 	tiers := []Tier{TierHourly, TierDaily}
 	for _, tier := range tiers {
 		if err := mgr.Create(testDataset, tier, now); err != nil {
 			t.Fatalf("Create %s: %v", tier, err)
 		}
 	}
 
-	// List and verify
 	grouped, err := mgr.ListManaged(testDataset)
 	if err != nil {
 		t.Fatalf("ListManaged: %v", err)
@@ -55,27 +67,22 @@ func TestIntegrationCreateAndList(t *testing.T) {
 			t.Errorf("expected snapshot for tier %s at %v", tier, now)
 		}
 	}
-
-	// Clean up
-	for _, tier := range tiers {
-		name := FormatName("zvolta_", tier, now)
-		_ = mgr.ZFS.DestroySnapshot(testDataset, name)
-	}
 }
 
 func TestIntegrationPrune(t *testing.T) {
 	mgr := testManager()
+	cleanSnapshots(t, mgr)
+	defer cleanSnapshots(t, mgr)
 
-	// Create 5 hourly snapshots
-	base := time.Date(2026, 3, 5, 10, 0, 0, 0, time.UTC)
+	// Use unique timestamps that don't overlap with other tests
+	base := time.Date(2025, 2, 1, 10, 0, 0, 0, time.UTC)
 	for i := 0; i < 5; i++ {
 		ts := base.Add(time.Duration(i) * time.Hour)
 		if err := mgr.Create(testDataset, TierHourly, ts); err != nil {
 			t.Fatalf("Create: %v", err)
 		}
 	}
 
-	// Verify 5 exist
 	grouped, err := mgr.ListManaged(testDataset)
 	if err != nil {
 		t.Fatalf("ListManaged: %v", err)
@@ -84,7 +91,6 @@ func TestIntegrationPrune(t *testing.T) {
 		t.Fatalf("expected 5 hourly snapshots, got %d", len(grouped[TierHourly]))
 	}
 
-	// Prune to keep 2
 	removed, err := mgr.Prune(testDataset, TierHourly, 2, false)
 	if err != nil {
 		t.Fatalf("Prune: %v", err)
@@ -93,7 +99,6 @@ func TestIntegrationPrune(t *testing.T) {
 		t.Errorf("expected 3 removed, got %d", len(removed))
 	}
 
-	// Verify 2 remain (the newest)
 	grouped, err = mgr.ListManaged(testDataset)
 	if err != nil {
 		t.Fatalf("ListManaged after prune: %v", err)
@@ -102,32 +107,27 @@ func TestIntegrationPrune(t *testing.T) {
 		t.Errorf("expected 2 hourly after prune, got %d", len(grouped[TierHourly]))
 	}
 
-	// Verify the remaining are the newest two (13:00 and 14:00)
+	// Verify the remaining are the newest two (13:00 and 14:00 UTC)
 	for _, s := range grouped[TierHourly] {
 		if s.Parsed.Timestamp.Hour() < 13 {
-			t.Errorf("expected only 13:00 and 14:00 to remain, got %v", s.Parsed.Timestamp)
+			t.Errorf("expected only newest to remain, got %v", s.Parsed.Timestamp)
 		}
 	}
-
-	// Clean up remaining
-	for _, s := range grouped[TierHourly] {
-		_ = mgr.ZFS.DestroySnapshot(testDataset, s.Snapshot.SnapName)
-	}
 }
 
 func TestIntegrationDryRun(t *testing.T) {
 	mgr := testManager()
+	cleanSnapshots(t, mgr)
+	defer cleanSnapshots(t, mgr)
 
-	// Create 3 snapshots
-	base := time.Date(2026, 3, 5, 20, 0, 0, 0, time.UTC)
+	base := time.Date(2025, 3, 1, 20, 0, 0, 0, time.UTC)
 	for i := 0; i < 3; i++ {
 		ts := base.Add(time.Duration(i) * time.Hour)
 		if err := mgr.Create(testDataset, TierDaily, ts); err != nil {
 			t.Fatalf("Create: %v", err)
 		}
 	}
 
-	// Dry-run prune to keep 1
 	removed, err := mgr.Prune(testDataset, TierDaily, 1, true)
 	if err != nil {
 		t.Fatalf("Prune dry-run: %v", err)
@@ -136,30 +136,32 @@ func TestIntegrationDryRun(t *testing.T) {
 		t.Errorf("dry-run should report 2 removals, got %d", len(removed))
 	}
 
-	// Verify all 3 still exist (dry-run shouldn't delete)
 	grouped, err := mgr.ListManaged(testDataset)
 	if err != nil {
 		t.Fatalf("ListManaged: %v", err)
 	}
 	if len(grouped[TierDaily]) != 3 {
 		t.Errorf("dry-run should not delete, expected 3, got %d", len(grouped[TierDaily]))
 	}
-
-	// Clean up
-	for _, s := range grouped[TierDaily] {
-		_ = mgr.ZFS.DestroySnapshot(testDataset, s.Snapshot.SnapName)
-	}
 }
 
 func TestIntegrationLastSnapshotTimes(t *testing.T) {
 	mgr := testManager()
+	cleanSnapshots(t, mgr)
+	defer cleanSnapshots(t, mgr)
 
-	ts1 := time.Date(2026, 3, 5, 8, 0, 0, 0, time.UTC)
-	ts2 := time.Date(2026, 3, 5, 9, 0, 0, 0, time.UTC)
+	ts1 := time.Date(2025, 4, 1, 8, 0, 0, 0, time.UTC)
+	ts2 := time.Date(2025, 4, 1, 9, 0, 0, 0, time.UTC)
 
-	_ = mgr.Create(testDataset, TierHourly, ts1)
-	_ = mgr.Create(testDataset, TierHourly, ts2)
-	_ = mgr.Create(testDataset, TierDaily, ts1)
+	if err := mgr.Create(testDataset, TierHourly, ts1); err != nil {
+		t.Fatalf("Create hourly ts1: %v", err)
+	}
+	if err := mgr.Create(testDataset, TierHourly, ts2); err != nil {
+		t.Fatalf("Create hourly ts2: %v", err)
+	}
+	if err := mgr.Create(testDataset, TierDaily, ts1); err != nil {
+		t.Fatalf("Create daily ts1: %v", err)
+	}
 
 	times, err := mgr.LastSnapshotTimes(testDataset)
 	if err != nil {
@@ -172,9 +174,4 @@ func TestIntegrationLastSnapshotTimes(t *testing.T) {
 	if !times[TierDaily].Equal(ts1) {
 		t.Errorf("daily last = %v, want %v", times[TierDaily], ts1)
 	}
-
-	// Clean up
-	_ = mgr.ZFS.DestroySnapshot(testDataset, FormatName("zvolta_", TierHourly, ts1))
-	_ = mgr.ZFS.DestroySnapshot(testDataset, FormatName("zvolta_", TierHourly, ts2))
-	_ = mgr.ZFS.DestroySnapshot(testDataset, FormatName("zvolta_", TierDaily, ts1))
 }

internal/zfs/zfs.go

@@ -126,6 +126,14 @@ func (c *Client) SetProperty(dataset, property, value string) error {
 	return err
 }
 
+// InheritProperty removes a local property value so the dataset inherits from
+// its parent. This is different from SetProperty with "-" which sets the literal
+// string "-" as the value.
+func (c *Client) InheritProperty(dataset, property string) error {
+	_, err := c.Cmd.Run("inherit", property, dataset)
+	return err
+}
+
 // CreateSnapshot creates a ZFS snapshot.
 func (c *Client) CreateSnapshot(dataset, snapName string) error {
 	full := fmt.Sprintf("%s@%s", dataset, snapName)