From 49ea29a66254cc23265025a912a73c3e5b0c6ae5 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 21:46:08 +0200 Subject: plan 9: property test compares directories and refuses to pass vacuously; undo tracks directories trash restores recreate --- internal/engine/apply.go | 11 ++++++++-- internal/engine/property_test.go | 42 ++++++++++++++++++++++++++++----------- internal/engine/roundtrip_test.go | 21 ++++++++++++++++++++ 3 files changed, 60 insertions(+), 14 deletions(-) (limited to 'internal') diff --git a/internal/engine/apply.go b/internal/engine/apply.go index e109aaf..09f3aab 100644 --- a/internal/engine/apply.go +++ b/internal/engine/apply.go @@ -1006,12 +1006,19 @@ func runUndoStep(step UndoStep) apply.StepResult { // put it explicitly (fix round 1, item 3) - never re-derived from // Src or Dst's shape, which belong to internal/apply's and this // file's own conventions and must stay free to change independently. + // The directory the file goes back into is created here, tracked, + // rather than silently by trash.Restore, so ApplyUndo removes it again + // when it ends up empty (review undo F6). + made, err := apply.MkdirAllTracked(filepath.Dir(step.Dst)) + if err != nil { + return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} + } restored, err := trash.Restore(step.Original.Detail) if err != nil { - return apply.StepResult{Status: "failed", Detail: err.Error()} + return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made} } size, mtime := statSizeModTime(restored) - return apply.StepResult{Status: "ok", Dst: restored, Size: size, ModTime: mtime} + return apply.StepResult{Status: "ok", Dst: restored, Size: size, ModTime: mtime, Made: made} case "undo-mkdir": if err := os.Remove(step.Src); err != nil { diff --git a/internal/engine/property_test.go b/internal/engine/property_test.go index 39589da..135c20a 100644 --- a/internal/engine/property_test.go +++ b/internal/engine/property_test.go @@ -102,27 +102,41 @@ func propertySeeds(t *testing.T) []int64 { // chain loses no content - every file's content is still somewhere under // the home directory, the Trash included - and undoing the run then puts // the home directory back exactly: every file's content, mode and -// modification time, and nothing extra. A failing subtest names its seed. +// modification time, every directory, and nothing extra. A failing subtest +// names its seed. It also fails when fewer than half the cases applied +// anything, so a broken Apply cannot make it pass vacuously (review undo +// F7). func TestApplyUndoProperty(t *testing.T) { - for _, seed := range propertySeeds(t) { + seeds := propertySeeds(t) + applied := 0 + for _, seed := range seeds { t.Run(fmt.Sprintf("seed=%d", seed), func(t *testing.T) { - checkApplyUndo(t, genCase(rand.New(rand.NewSource(seed)))) + if checkApplyUndo(t, genCase(rand.New(rand.NewSource(seed)))) { + applied++ + } }) } + if len(seeds) >= 20 && applied*2 < len(seeds) { + t.Errorf("only %d of %d cases applied anything; the property is not being exercised", applied, len(seeds)) + } } -// contentCounts counts the files of a snapshot by content hash. +// contentCounts counts the files of a snapshot by content hash; directory +// entries have no content and are not counted. func contentCounts(snap map[string]string) map[string]int { counts := map[string]int{} for _, v := range snap { - counts[strings.Fields(v)[0]]++ + if v != "dir" { + counts[strings.Fields(v)[0]]++ + } } return counts } // checkApplyUndo builds c in a sandbox, applies every chain, checks nothing -// was lost, undoes the run and checks the home directory is as it was. -func checkApplyUndo(t *testing.T, c propertyCase) { +// was lost, undoes the run and checks the home directory - files and +// directories - is as it was. It reports whether any file was applied. +func checkApplyUndo(t *testing.T, c propertyCase) bool { h := sandbox(t) old := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) put := func(p, body string) { @@ -147,12 +161,15 @@ func checkApplyUndo(t *testing.T, c propertyCase) { put(filepath.Join(h, rel), body) } main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": c.rules}) - // userTree is the home directory without krino's own config and state. + // userTree is the home directory, files and directories, without + // krino's own config, state, Trash and cache. userTree := func() map[string]string { - snap := snapshot(t, h) + snap := treeSnapshot(t, h) for rel := range snap { - if strings.HasPrefix(rel, ".config/") || strings.HasPrefix(rel, ".local/") { - delete(snap, rel) + for _, own := range []string{".config/", ".local/", ".cache/"} { + if strings.HasPrefix(rel, own) { + delete(snap, rel) + } } } return snap @@ -193,7 +210,7 @@ func checkApplyUndo(t *testing.T, c propertyCase) { if after := userTree(); !reflect.DeepEqual(after, before) { t.Fatalf("nothing was applied, yet the tree changed") } - return + return false } have := contentCounts(snapshot(t, h)) @@ -236,4 +253,5 @@ func checkApplyUndo(t *testing.T, c propertyCase) { } } } + return true } diff --git a/internal/engine/roundtrip_test.go b/internal/engine/roundtrip_test.go index 67de507..e54b0b7 100644 --- a/internal/engine/roundtrip_test.go +++ b/internal/engine/roundtrip_test.go @@ -44,6 +44,27 @@ func snapshot(t *testing.T, root string) map[string]string { return out } +// treeSnapshot is snapshot plus one "dir" entry per directory under root, +// so a comparison also sees directories left behind or missing. +func treeSnapshot(t *testing.T, root string) map[string]string { + t.Helper() + out := snapshot(t, root) + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && p != root { + rel, _ := filepath.Rel(root, p) + out[rel+"/"] = "dir" + } + return nil + }) + if err != nil { + t.Fatal(err) + } + return out +} + func TestApplyThenUndoRestoresTheTree(t *testing.T) { h := sandbox(t) dl := filepath.Join(h, "dl") -- cgit v1.3