diff options
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | cmd/krino/sort.go | 7 | ||||
| -rw-r--r-- | cmd/krino/sort_test.go | 68 | ||||
| -rw-r--r-- | internal/engine/undo_identity_test.go | 53 |
4 files changed, 130 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b82a8d..0b3ac7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ could change its answer; `explain` shows `?`. A document read only in part answers the keywords found in what was read, and leaves the others unknown. +- Tests: a genuine apply error exits 1, and `w` in review stops krino even + when applying fails, both through the real command; the undo projection's + "about to be occupied" half. - Build: `make build` and `make install` refuse a `VERSION` that is not a plain version string, as `cross` and `release` did; `make cross` names its directories without the tag's `v`; the dependency gate covers test diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index 163d388..5a175a4 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -37,6 +37,11 @@ import ( // what go test does with stdin. var stdin = os.Stdin +// stdinIsTerminal is the check that review can prompt at all, a seam so a +// test can drive review through a pipe (tui.ReadKey reads a pipe byte by +// byte). +var stdinIsTerminal = func() bool { return term.IsTerminal(int(stdin.Fd())) } + // zeroOutcome is the per-directory outcome line for a directory that had // nothing applied to it - either because nothing was actionable, or // because the user chose [s] or [q] - so the same wording is not retyped @@ -73,7 +78,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { // Spec ยง8.4: with neither -y nor -n, krino asks; asking a non-terminal // stdin would just hang (or read garbage), so it refuses instead. - if !g.yes && !g.dry && !term.IsTerminal(int(stdin.Fd())) { + if !g.yes && !g.dry && !stdinIsTerminal() { return usageError(stderr, "refusing to prompt: stdin is not a terminal (use -y or -n)") } diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go index 2a72e6a..22e00b4 100644 --- a/cmd/krino/sort_test.go +++ b/cmd/krino/sort_test.go @@ -265,3 +265,71 @@ func TestLaterDirectoryIsNotBlockedByAnEarlierOnesClaims(t *testing.T) { t.Errorf("cb's x.txt did not move into the place ca's left free: %q, %v\n%s", b, err, out) } } + +// devFullFixture builds two directories, d1 and d2, each with two files a +// rule moves, and a log at /dev/full, where every write fails: applying +// anything is a genuine apply error, not a step failure. +func devFullFixture(t *testing.T) { + t.Helper() + if _, err := os.Stat("/dev/full"); err != nil { + t.Skip("no /dev/full on this system") + } + h := home(t) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + if code, _, errOut := runCLI(t, "init"); code != 0 { + t.Fatal(errOut) + } + for _, n := range []string{"d1", "d2"} { + for _, name := range []string{"a.pdf", "b.pdf"} { + p := filepath.Join(h, n, name) + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte(n+name), 0o644) + os.Chtimes(p, old, old) + } + if code, _, errOut := runCLI(t, "new", n, filepath.Join(h, n)); code != 0 { + t.Fatal(errOut) + } + os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", n+".conf"), []byte("(path \"~/"+n+"\")\n(rule \"r\" (move \"Out\"))\n"), 0o644) + } + f, err := os.OpenFile(filepath.Join(h, ".config", "krino", "krino.conf"), os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + f.WriteString("(log \"/dev/full\")\n") + f.Close() +} + +// TestApplyErrorExitsOne: an error applying a directory - here the log +// cannot be written - makes krino exit 1 (triage 34m: removing that exit +// code left every test passing). +func TestApplyErrorExitsOne(t *testing.T) { + devFullFixture(t) + code, _, errOut := runCLI(t, "-y", "d1") + if code != 1 || !strings.Contains(errOut, "no space left") { + t.Errorf("exit %d, stderr %q; want 1 and the write error", code, errOut) + } +} + +// TestWriteStopsKrinoWhenApplyFails: [w] in review stops krino after its +// directory even when applying it fails - the next directory is not planned +// or asked about (review cli F3), driven through the real command with a +// pipe standing in for the terminal (triage 28m). +func TestWriteStopsKrinoWhenApplyFails(t *testing.T) { + devFullFixture(t) + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + w.WriteString("cyw") // choose per file, yes to a.pdf, write before b.pdf + w.Close() + oldStdin, oldTerm := stdin, stdinIsTerminal + stdin, stdinIsTerminal = r, func() bool { return true } + t.Cleanup(func() { stdin, stdinIsTerminal = oldStdin, oldTerm }) + code, out, errOut := runCLI(t, "--no-pager") + if code != 1 { + t.Errorf("exit %d, want 1\n%s\n%s", code, out, errOut) + } + if strings.Contains(out, "krino: d2") { + t.Errorf("d2 was planned after [w]:\n%s", out) + } +} diff --git a/internal/engine/undo_identity_test.go b/internal/engine/undo_identity_test.go index f42c5da..01f57d8 100644 --- a/internal/engine/undo_identity_test.go +++ b/internal/engine/undo_identity_test.go @@ -572,3 +572,56 @@ func TestUndoRunWithADamagedLineIsStillAnUndo(t *testing.T) { t.Errorf("PlanUndo(undo run with a damaged line) = %v; want refused as an undo", err) } } + +// TestUndoOverwriteThenMove: a move that replaced an existing file and was +// then moved on is undone whole - the moved file goes back, and the file it +// replaced is restored to the path the later reversal vacates (triage 34m). +func TestUndoOverwriteThenMove(t *testing.T) { + e, run, h, logPath := appliedRun(t, map[string]map[string]string{"dl": {"a.pdf": "one", "Out/a.pdf": "old"}}, + map[string]string{"dl": "(path \"~/dl\")\n(ignore \"Out/\")\n(recursive yes)\n(rule \"r\" (on-conflict overwrite) (move \"Out\") (move \"Out2\"))\n"}) + if b, _ := os.ReadFile(filepath.Join(h, "dl", "Out2", "a.pdf")); string(b) != "one" { + t.Fatalf("setup: the chain did not run as planned") + } + up, err := e.PlanUndo(run) + if err != nil { + t.Fatal(err) + } + for _, f := range up.Files { + if f.Refused != "" { + t.Fatalf("%s refused: %s (%+v)", f.File, f.Refused, f.Steps) + } + } + j, err := journal.Open(logPath) + if err != nil { + t.Fatal(err) + } + defer j.Close() + if _, err := e.ApplyUndo(context.Background(), up, j, journal.NewRunID(time.Now())); err != nil { + t.Fatal(err) + } + for rel, want := range map[string]string{"a.pdf": "one", "Out/a.pdf": "old"} { + if b, err := os.ReadFile(filepath.Join(h, "dl", rel)); err != nil || string(b) != want { + t.Errorf("%s after undo: %q, %v; want %q", rel, b, err, want) + } + } +} + +// TestProjectionSeesAPathAnEarlierStepWillFill: a path not on disk yet that +// a queued reversal will put a file at is occupied for the steps after it, +// and free again once a later one moves that file on (triage 34m: the +// "occupied" half of the projection had no test that could fail). +func TestProjectionSeesAPathAnEarlierStepWillFill(t *testing.T) { + x := filepath.Join(t.TempDir(), "x.pdf") + p := newUndoProjection() + if p.occupiedNow(x) { + t.Fatal("an empty path reads as occupied") + } + p.record(UndoStep{Action: "undo-move", Src: x + ".elsewhere", Dst: x}) + if !p.occupiedNow(x) { + t.Error("a path a queued undo-move fills reads as free") + } + p.record(UndoStep{Action: "undo-rename", Src: x, Dst: x + ".back"}) + if p.occupiedNow(x) { + t.Error("a path a later reversal vacates still reads as occupied") + } +} |