diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 12:11:42 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 12:11:42 +0200 |
| commit | bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd (patch) | |
| tree | 187e6a1fb722d9ab97d2076f69f997f5d371e943 /cmd/krino/history_test.go | |
| parent | cd7425b81f963a948f0abe7df3f9e58e190c2b78 (diff) | |
| download | krino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.tar.gz krino-bddbd74e4a73e8e32bcf648efd1cac5655f6d0cd.zip | |
comments that explain the code, not how it was written
About 340 comments cited the development process: task and plan
numbers, fix waves, rulings, reviewers, and the author in the third
person with a date. None of that exists outside the work itself, so to
a reader it pointed at nothing. Each one now states the engineering
reason it was standing in front of; where a comment was provenance and
nothing else, it is gone.
References to docs/design.md and docs/gui-design.md by section stay:
both ship with the repository. The design documents lose their
amendment diaries - CHANGELOG.md is that record - and the GUI's says
plainly that the window has gone further than the document.
Only comments changed. Every .go file was parsed and its code printed
with comments stripped, before and after: the two hashes are identical
across all 175 files.
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") { |
