diff options
| -rw-r--r-- | CHANGELOG.md | 6 | ||||
| -rw-r--r-- | internal/engine/apply.go | 9 | ||||
| -rw-r--r-- | internal/engine/property_test.go | 239 | ||||
| -rw-r--r-- | internal/engine/roundtrip_test.go | 18 |
4 files changed, 272 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b856f3e..10c49ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +- Undo reverses a file its run renamed and then moved, moved twice, or + moved and then sent to the Trash. Before, undo judged the first step + against the disk as it is now, found its destination empty (the later + step had moved the file on) and refused the whole file as "missing". Found + by the new generated apply-and-undo test. + - Folding maps the capital sharp s (ẞ) to "SS", as the other capitals already were: "STRAẞE" in a document now matches the keyword "straße". - Folding replaces invalid UTF-8 with U+FFFD first, so a stray invalid byte diff --git a/internal/engine/apply.go b/internal/engine/apply.go index 878cb18..ce05636 100644 --- a/internal/engine/apply.go +++ b/internal/engine/apply.go @@ -476,6 +476,15 @@ func planUndoFile(file string, ents []journal.Entry) UndoFile { break } step := reverseStep(en) + if (en.Action == "move" || en.Action == "rename") && proj.occupied[en.Dst] { + // A reversal already queued for this same file puts it back at + // en.Dst before this one runs, and that reversal was checked + // against the disk itself. Judged against the disk as it is now, + // en.Dst is empty - a later step of the chain moved the file on - + // and every rename-then-move, move-then-move or move-then-trash + // chain would be refused (plan 8, found by the property test). + step.Refused = "" + } if step.Refused == "" { step.Refused = refuseIfSrcExists(step, proj) } diff --git a/internal/engine/property_test.go b/internal/engine/property_test.go new file mode 100644 index 0000000..39589da --- /dev/null +++ b/internal/engine/property_test.go @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package engine + +import ( + "context" + "fmt" + "math/rand" + "os" + "path/filepath" + "reflect" + "strconv" + "strings" + "testing" + "time" + + "krino/internal/journal" + "krino/internal/plan" +) + +// propertyNames are the file names generated trees draw from: ordinary, +// spaced, Polish, dash-led, newline-holding, double-extension and hidden. +var propertyNames = []string{"a.pdf", "b.pdf", "a copy.pdf", "zażółć.txt", "-dash.pdf", "new\nline.pdf", "notes.txt", "x.tar.gz", ".hidden.pdf"} + +// propertyCase is one generated tree and rule set. +type propertyCase struct { + files map[string]string // path under ~/dl -> content + existing map[string]string // path under ~ -> content, there before planning to force conflicts + rules string // dirs/dl.conf +} + +// genCase draws a case from r: one to six files (some nested, some sharing +// content), up to two files already sitting in destinations, and one to +// three rules mixing moves, copies, renames and trash under every conflict +// policy, with and without (stop). A (delete) is only ever a rule's last +// action - the config refuses any action after it. Permanent delete is left +// out: it cannot be undone by design. +func genCase(r *rand.Rand) propertyCase { + c := propertyCase{files: map[string]string{}, existing: map[string]string{}} + for i := 0; i < 1+r.Intn(6); i++ { + name := propertyNames[r.Intn(len(propertyNames))] + if r.Intn(3) == 0 { + name = "sub/" + name + } + c.files[name] = []string{"one", "two", "one"}[r.Intn(3)] + " " + strconv.Itoa(r.Intn(3)) + } + for i := 0; i < r.Intn(3); i++ { + dir := []string{"dl/Out", "dl/Out/2026", "backup"}[r.Intn(3)] + c.existing[dir+"/"+propertyNames[r.Intn(len(propertyNames))]] = "already here" + } + conds := []string{"", "(when (type pdf))", `(when (name "^a"))`, "(when (not (type txt)))"} + actions := []string{`(move "Out")`, `(move "Out/{mtime:%Y}")`, `(copy "~/backup")`, `(rename "r-{name}")`} + conflicts := []string{"", "(on-conflict suffix)", "(on-conflict skip)", "(on-conflict overwrite)"} + var b strings.Builder + b.WriteString("(path \"~/dl\")\n") + if r.Intn(2) == 0 { + b.WriteString("(recursive yes)\n") + } + for i := 0; i < 1+r.Intn(3); i++ { + fmt.Fprintf(&b, "(rule \"r%d\" %s %s", i, conflicts[r.Intn(len(conflicts))], conds[r.Intn(len(conds))]) + for j := 0; j < 1+r.Intn(2); j++ { + b.WriteString(" " + actions[r.Intn(len(actions))]) + } + if r.Intn(4) == 0 { + b.WriteString(" (delete)") + } + if r.Intn(2) == 0 { + b.WriteString(" (stop)") + } + b.WriteString(")\n") + } + c.rules = b.String() + return c +} + +// propertySeeds is the seeds TestApplyUndoProperty runs: KRINO_PROPERTY_SEED +// alone when set (to rerun a failure), else 1..KRINO_PROPERTY_RUNS, 40 by +// default - raise it (say 2000) before a release. +func propertySeeds(t *testing.T) []int64 { + if v := os.Getenv("KRINO_PROPERTY_SEED"); v != "" { + s, err := strconv.ParseInt(v, 10, 64) + if err != nil { + t.Fatalf("KRINO_PROPERTY_SEED=%q", v) + } + return []int64{s} + } + n := 40 + if v := os.Getenv("KRINO_PROPERTY_RUNS"); v != "" { + var err error + if n, err = strconv.Atoi(v); err != nil || n < 1 { + t.Fatalf("KRINO_PROPERTY_RUNS=%q", v) + } + } + seeds := make([]int64, n) + for i := range seeds { + seeds[i] = int64(i + 1) + } + return seeds +} + +// TestApplyUndoProperty: for generated trees and rules, applying every +// 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. +func TestApplyUndoProperty(t *testing.T) { + for _, seed := range propertySeeds(t) { + t.Run(fmt.Sprintf("seed=%d", seed), func(t *testing.T) { + checkApplyUndo(t, genCase(rand.New(rand.NewSource(seed)))) + }) + } +} + +// contentCounts counts the files of a snapshot by content hash. +func contentCounts(snap map[string]string) map[string]int { + counts := map[string]int{} + for _, v := range snap { + 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) { + h := sandbox(t) + old := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + put := func(p, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, old, old); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(h, "dl"), 0o755); err != nil { + t.Fatal(err) + } + for rel, body := range c.files { + put(filepath.Join(h, "dl", rel), body) + } + for rel, body := range c.existing { + 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 := func() map[string]string { + snap := snapshot(t, h) + for rel := range snap { + if strings.HasPrefix(rel, ".config/") || strings.HasPrefix(rel, ".local/") { + delete(snap, rel) + } + } + return snap + } + before := userTree() + t.Logf("rules:\n%s", c.rules) + + e, errs := Load(main) + if len(errs) > 0 { + t.Fatalf("generated rules do not load: %v", errs) + } + ctx := context.Background() + dp, err := e.Plan(ctx, e.Dirs[0], plan.NewClaims()) + if err != nil { + t.Fatal(err) + } + approved := map[string]bool{} + for _, ch := range dp.Chains { + approved[ch.File.Rel] = true + } + logPath := filepath.Join(h, ".local", "state", "krino", "krino.log") + j, err := journal.Open(logPath) + if err != nil { + t.Fatal(err) + } + run := journal.NewRunID(time.Now()) + res, err := e.Apply(ctx, dp, approved, j, run) + j.Close() + if err != nil { + t.Fatal(err) + } + if res.Failed != 0 { + t.Fatalf("%d files failed to apply: %+v", res.Failed, res.Files) + } + if len(res.Files) == 0 { + // No file had a step, so nothing ran and nothing was logged: there + // is no run to undo, and the tree must simply be untouched. + if after := userTree(); !reflect.DeepEqual(after, before) { + t.Fatalf("nothing was applied, yet the tree changed") + } + return + } + + have := contentCounts(snapshot(t, h)) + for hash, n := range contentCounts(before) { + if have[hash] < n { + t.Errorf("after apply, content %s is in %d files, was in %d", hash[:12], have[hash], n) + } + } + + up, err := e.PlanUndo(run) + if err != nil { + t.Fatal(err) + } + for _, f := range up.Files { + if f.Refused != "" { + t.Fatalf("undo refused %q: %s", f.File, f.Refused) + } + } + j2, err := journal.Open(logPath) + if err != nil { + t.Fatal(err) + } + ures, err := e.ApplyUndo(ctx, up, j2, journal.NewRunID(time.Now())) + j2.Close() + if err != nil { + t.Fatal(err) + } + if ures.Failed != 0 { + t.Fatalf("%d files failed to undo: %+v", ures.Failed, ures.Files) + } + if after := userTree(); !reflect.DeepEqual(after, before) { + for rel, v := range before { + if after[rel] != v { + t.Errorf("%q after undo: %q, want %q", rel, after[rel], v) + } + } + for rel := range after { + if _, ok := before[rel]; !ok { + t.Errorf("%q is left behind after undo", rel) + } + } + } +} diff --git a/internal/engine/roundtrip_test.go b/internal/engine/roundtrip_test.go index 425dc1a..67de507 100644 --- a/internal/engine/roundtrip_test.go +++ b/internal/engine/roundtrip_test.go @@ -159,3 +159,21 @@ func TestApplyThenUndoRestoresTheTree(t *testing.T) { t.Errorf("reading backup dir after undo: %v", err) } } + +// TestUndoReversesChainsWithinOneFile is the property test's first finding +// (plan 8, Task 9): undo judged each move and rename against the disk as it +// is now, so the first step of a chain - a rename then a move, two moves, a +// move then trash - found its destination empty, because the later step had +// already moved the file on, and the whole file was refused. +func TestUndoReversesChainsWithinOneFile(t *testing.T) { + for name, rule := range map[string]string{ + "rename then move": `(rule "r" (rename "r-{name}") (move "Out"))`, + "move then move": `(rule "r" (move "Out") (move "Out/{mtime:%Y}"))`, + "move then trash": `(rule "r" (move "Out") (delete))`, + "rename twice": `(rule "r" (rename "r-{name}") (rename "r-{name}"))`, + } { + t.Run(name, func(t *testing.T) { + checkApplyUndo(t, propertyCase{files: map[string]string{"a.pdf": "one"}, rules: "(path \"~/dl\")\n" + rule + "\n"}) + }) + } +} |