From 9e65644f473d75ceb7e3ef67302189eeaba0f922 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 22:39:27 +0200 Subject: plan 10: missing and weak tests (per-step logging, trash path, fuzz oracle, w after error) --- cmd/krino/history_test.go | 2 ++ cmd/krino/review_test.go | 31 +++++++++++++++++++++++++++++++ cmd/krino/sort.go | 40 ++++++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 16 deletions(-) (limited to 'cmd/krino') diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go index 6d8c1e4..7c0f810 100644 --- a/cmd/krino/history_test.go +++ b/cmd/krino/history_test.go @@ -506,6 +506,8 @@ func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) { {"--min-age", "garbage", "undo", "-n"}, {"check", "--min-age", "1d"}, {"log", "--min-age", "1d"}, + {"init", "--min-age", "1d"}, + {"new", "--min-age", "1d", "x", "/tmp"}, {"-n", "--min-age="}, } { if code, _, errOut := runCLI(t, args...); code != 2 || !strings.Contains(errOut, "--min-age") { diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go index a1fbba7..b138451 100644 --- a/cmd/krino/review_test.go +++ b/cmd/krino/review_test.go @@ -3,6 +3,9 @@ package main import ( + "context" + "errors" + "fmt" "io" "strings" "testing" @@ -295,3 +298,31 @@ func TestNotReviewedOutcome(t *testing.T) { t.Errorf("got %q", got) } } + +// TestStopAfterApply: [w] stops krino after its directory whether or not the +// apply succeeded (review cli F3) - a log that cannot be written must not +// lead on to planning and prompting the next directory - and an interrupt +// always stops it. +func TestStopAfterApply(t *testing.T) { + logErr := errors.New("write krino.log: no space left on device") + cases := []struct { + action rune + err error + want bool + }{ + {'a', nil, false}, + {'c', nil, false}, + {'w', nil, true}, + {'a', logErr, false}, + {'c', logErr, false}, + {'w', logErr, true}, + {'a', context.Canceled, true}, + {'c', fmt.Errorf("apply: %w", context.Canceled), true}, + {'a', context.DeadlineExceeded, true}, + } + for _, c := range cases { + if got := stopAfterApply(c.action, c.err); got != c.want { + t.Errorf("stopAfterApply(%q, %v) = %v, want %v", c.action, c.err, got, c.want) + } + } +} diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index 4d5f509..637587d 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -134,7 +134,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { // of polling forever. l, err := lock.Acquire(ctx, e.Config.LockFile(d.Name), !g.yes) if err != nil { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + if interrupted(err) { // Interrupted while waiting for the lock: an interrupt, not // a failure - the ctx.Err() check at the end of this // function already turns this into exit 130, and nothing @@ -258,21 +258,16 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { } res, aerr := e.Apply(ctx, toApply, approved, j, run) if aerr != nil { - if errors.Is(aerr, context.Canceled) || errors.Is(aerr, context.DeadlineExceeded) { - // Interrupted mid-apply (fix round 2026-09-12/item 2): - // treated exactly like the cancelled lock wait above - - // not a failure ("context canceled" is a Go-ism, not - // something to show a user who just pressed Ctrl-C), - // and no further directory is even attempted. The - // ctx.Err() check at the end of this function already - // turns this into exit 130. - return true + // Interrupted mid-apply (fix round 2026-09-12/item 2): + // treated exactly like the cancelled lock wait above - not + // a failure ("context canceled" is a Go-ism, not something + // to show a user who just pressed Ctrl-C). The ctx.Err() + // check at the end of this function turns it into exit 130. + if !interrupted(aerr) { + fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr) + exit = 1 } - fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr) - exit = 1 - // [w] stops krino whether or not its apply succeeded (review - // cli F3): no later directory is planned or asked about. - return action == 'w' + return stopAfterApply(action, aerr) } fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed)) // Ruling 1: only an actual step failure makes the run exit 1 @@ -280,7 +275,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { if res.Failed > 0 { exit = 1 } - return action == 'w' + return stopAfterApply(action, nil) }() if quit { @@ -583,3 +578,16 @@ func padCell(s string, w int) string { } return s + strings.Repeat(" ", w-n) } + +// interrupted reports whether err is the context being cancelled (Ctrl-C, +// SIGTERM, SIGHUP) rather than a failure. +func interrupted(err error) bool { + return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) +} + +// stopAfterApply reports whether krino stops once a directory has been +// applied: an interrupt stops it, and so does [w], whether or not its apply +// succeeded (review cli F3) - no later directory is planned or asked about. +func stopAfterApply(action rune, err error) bool { + return interrupted(err) || action == 'w' +} -- cgit v1.3