diff options
Diffstat (limited to 'cmd/krino/history_test.go')
| -rw-r--r-- | cmd/krino/history_test.go | 87 |
1 files changed, 43 insertions, 44 deletions
diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go index 9c409bc..d85766f 100644 --- a/cmd/krino/history_test.go +++ b/cmd/krino/history_test.go @@ -47,8 +47,8 @@ func TestLogListsRunsAndUndoReverses(t *testing.T) { if _, out, _ = runCLI(t, "log"); !strings.Contains(out, "undone") { t.Errorf("log does not mark the run undone:\n%s", out) } - // Plain undo after an undo continues the run it undid (review M10): - // everything came back, so nothing is left and nothing moves. + // Plain undo after an undo continues the run it undid: everything came + // back, so nothing is left and nothing moves. if code, out, errOut := runCLI(t, "undo", "-y"); code != 0 || !strings.Contains(out, "0 applied") { t.Errorf("undo after a complete undo: %d\n%s\n%s", code, out, errOut) } @@ -75,21 +75,21 @@ func TestUndoDryRunChangesNothing(t *testing.T) { } } -// TestUndoFailsImmediatelyWithHeldLock is fix round 2026-09-12, item 1: an -// undo moves files just as an apply does, so it needs the same per-directory -// guard sort's TestSecondRunFailsImmediatelyWithYes already pins for the -// forward path (spec §11: "a second krino on the same directory ... fails -// immediately with -y"). The lock file is held under the config NAME "dl", -// not any filesystem path - UndoFile.Dir is the journal's `dir` column, -// which is the directory's name from krino.conf, not its root. +// TestUndoFailsImmediatelyWithHeldLock: an undo moves files just as an +// apply does, so it needs the same per-directory guard sort's +// TestSecondRunFailsImmediatelyWithYes already pins for the forward path +// (spec §11: "a second krino on the same directory ... fails immediately +// with -y"). The lock file is held under the config NAME "dl", not any +// filesystem path - UndoFile.Dir is the journal's `dir` column, which is +// the directory's name from krino.conf, not its root. // -// Ruling (fix round 2026-09-12, follow-up): the lock is acquired before the -// plan is even shown, matching cmdSort's own window (acquired before -// Plan/review, held across both) rather than only around ApplyUndo - so -// this also asserts the refusal is noticed before any plan output reaches -// stdout. A version of this test that only checked the exit code and -// stderr would pass equally whether the lock were taken early or late, and -// so would not be pinning the thing this ruling is actually about. +// The lock is acquired before the plan is even shown, matching cmdSort's +// own window (acquired before Plan/review, held across both) rather than +// only around ApplyUndo - so this also asserts the refusal is noticed +// before any plan output reaches stdout. A version of this test that only +// checked the exit code and stderr would pass equally whether the lock +// were taken early or late, and so would not be pinning the thing this +// test is actually meant to catch. func TestUndoFailsImmediatelyWithHeldLock(t *testing.T) { h := matchingFixture(t) if code, _, errOut := runCLI(t, "-y"); code != 0 { @@ -131,12 +131,12 @@ func undoFiles(rels ...string) []engine.UndoFile { return out } -// TestFinalizeUndoPlanMarksUnapprovedAsDeclined is the wiring point for fix -// round 2026-09-12, item 2: a refused file rides through untouched (its own -// Refused reason is what ApplyUndo checks first), an approved file rides -// through untouched too, and anything else - explicitly declined, or never -// reached because [d]/[q] cut a per-file review short - comes out with -// Declined set rather than being dropped from the plan. +// TestFinalizeUndoPlanMarksUnapprovedAsDeclined: a refused file rides +// through untouched (its own Refused reason is what ApplyUndo checks +// first), an approved file rides through untouched too, and anything else +// - explicitly declined, or never reached because [d]/[q] cut a per-file +// review short - comes out with Declined set rather than being dropped +// from the plan. func TestFinalizeUndoPlanMarksUnapprovedAsDeclined(t *testing.T) { up := &engine.UndoPlan{Run: "r1", Files: []engine.UndoFile{ {File: "a"}, // index 0: approved @@ -239,19 +239,18 @@ func TestReviewUndoRefusedFileNotPrompted(t *testing.T) { } } -// TestPrintUndoPlan is fix wave item 3 (Important), rebuilding fix round -// 2026-09-12's own golden test: that version hand-built its UndoSteps, -// including a Dst on the undo-copy step the real code never sets (Dst is -// deliberately left "" - trash.Put only chooses the entry name at execution -// time), so it was structurally incapable of catching the bug it was meant -// to guard against - an undo-copy row rendering as a bare -// "undo-copy → " with nothing said about what it would do to the +// TestPrintUndoPlan rebuilds an earlier golden test that hand-built its +// UndoSteps, including a Dst on the undo-copy step the real code never +// sets (Dst is deliberately left "" - trash.Put only chooses the entry +// name at execution time), so it was structurally incapable of catching +// the bug it was meant to guard against - an undo-copy row rendering as a +// bare "undo-copy → " with nothing said about what it would do to the // user's backup copy, the single most destructive step an undo plan takes. -// The same lesson as Task 9's review: a hand-assembled fixture hides a test -// that cannot detect a broken copy-undo. This version runs a REAL forward -// apply (copy then move, so mkdir, copy and move all appear in one file's -// own chain) and a REAL PlanUndo, editing one file's result afterward so -// the plan also carries a genuinely refused row, then renders that. +// A hand-assembled fixture hides a test that cannot detect a broken +// copy-undo. This version runs a REAL forward apply (copy then move, so +// mkdir, copy and move all appear in one file's own chain) and a REAL +// PlanUndo, editing one file's result afterward so the plan also carries a +// genuinely refused row, then renders that. func TestPrintUndoPlan(t *testing.T) { h := home(t) dl := filepath.Join(h, "dl") @@ -315,9 +314,9 @@ func TestPrintUndoPlan(t *testing.T) { "undo-move → ~/dl/inv1.pdf\n", "undo-copy ~/backup/inv1.pdf → trash\n", "undo-mkdir ~/backup\n", - // Minor 4 / fix wave item 5: the refusal reason must be abbreviated - // against $HOME exactly like every step cell above it, not printed - // as a raw absolute path. + // The refusal reason must be abbreviated against $HOME exactly + // like every step cell above it, not printed as a raw absolute + // path. "refused: ~/dl/Work/notes.pdf changed since the run\n", } { if !strings.Contains(out, want) { @@ -412,8 +411,8 @@ func TestGlobalYesBeforeUndo(t *testing.T) { } // TestUndoWithoutRunContinuesTheLastUndo: when the most recent run is an -// undo that could not finish, plain `krino undo` offers what that undo left -// instead of refusing because the last run is an undo (review M10). +// undo that could not finish, plain `krino undo` offers what that undo +// left instead of refusing because the last run is an undo. func TestUndoWithoutRunContinuesTheLastUndo(t *testing.T) { h := home(t) dl := filepath.Join(h, "dl") @@ -471,9 +470,9 @@ func TestUndoWithoutRunContinuesTheLastUndo(t *testing.T) { } // TestReviewUndoMatchesReview: undo's per-file review behaves as review's -// does (review cli F2): n is recorded, each choice is echoed in red, and w -// leaves the files it never reached out of the plan, counted as not -// reviewed rather than logged as declined. +// does: n is recorded, each choice is echoed in red, and w leaves the +// files it never reached out of the plan, counted as not reviewed rather +// than logged as declined. func TestReviewUndoMatchesReview(t *testing.T) { out := new(strings.Builder) files := undoFiles("a", "b", "c") @@ -501,7 +500,7 @@ func TestReviewUndoMatchesReview(t *testing.T) { // TestMinAgeRejectedOutsideSortAndExplain: --min-age only changes sorting // and explain; any other command refuses it rather than silently ignoring -// a mistyped value, and an empty value is an error (review cli F4). +// a mistyped value, and an empty value is an error. func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) { matchingFixture(t) for _, args := range [][]string{ @@ -524,7 +523,7 @@ func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) { // TestIgnoredGlobalFlagsAreRefused: a global flag a command does not use is // refused instead of silently ignored, so "krino -n new ..." or "krino -n -// init" - meant as a preview - cannot write config (re-review cli F4). +// init" - meant as a preview - cannot write config. func TestIgnoredGlobalFlagsAreRefused(t *testing.T) { h := home(t) if code, _, errOut := runCLI(t, "-n", "init"); code != 2 || !strings.Contains(errOut, "-n") { |