diff options
Diffstat (limited to 'internal/journal/read_test.go')
| -rw-r--r-- | internal/journal/read_test.go | 132 |
1 files changed, 65 insertions, 67 deletions
diff --git a/internal/journal/read_test.go b/internal/journal/read_test.go index 4f1c902..d2bbd3a 100644 --- a/internal/journal/read_test.go +++ b/internal/journal/read_test.go @@ -104,17 +104,17 @@ func TestRunsMarksAnUndoneRun(t *testing.T) { } } -// TestRunsDoesNotMarkUndoneWhenEveryFileWasDeclined is fix wave item 2 -// (Important) / final-wave item 17: an undo run's run-start Detail alone -// used to be enough for Runs to mark the original run Undone, even when the -// undo run went on to decline every file (spec §9's "declined files are -// logged even though nothing happens to them", extended to undo) and -// reversed nothing at all. Reproduced by the reviewer via pty: `krino log` -// told the user a run had been undone when the file was still filed. Run B -// here carries the same run-start Detail as TestRunsMarksAnUndoneRun's, but -// every one of its file-scoped entries is "declined", never "ok" - the -// shape ApplyUndo logs when the front end's own review declines everything -// - so run A must come back exactly as untouched. +// TestRunsDoesNotMarkUndoneWhenEveryFileWasDeclined: an undo run's +// run-start Detail alone is not enough for Runs to mark the original run +// Undone, even when the undo run went on to decline every file (spec §9's +// "declined files are logged even though nothing happens to them", +// extended to undo) and reversed nothing at all - otherwise `krino log` +// would tell the user a run had been undone when the file was still +// filed. Run B here carries the same run-start Detail as +// TestRunsMarksAnUndoneRun's, but every one of its file-scoped entries is +// "declined", never "ok" - the shape ApplyUndo logs when the front end's +// own review declines everything - so run A must come back exactly as +// untouched. func TestRunsDoesNotMarkUndoneWhenEveryFileWasDeclined(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -146,19 +146,19 @@ func TestRunsDoesNotMarkUndoneWhenEveryFileWasDeclined(t *testing.T) { } } -// TestRunsDoesNotMarkUndoneWhenOnlyOkEntryIsMkdir is the coordinator's -// tightening of fix wave item 2: "at least one ok undo-* entry" is still -// too loose, by the same shape as the bug it fixes. A file's own chain -// stops after a failed file-affecting reversal, but a failed or refused -// undo-mkdir deliberately does not stop anything (internal/engine's -// isFileAffecting draws exactly this line, and undoFile's stop-on-failure -// check shares it) - so an undo-mkdir belonging to one file can still -// succeed even though every file-affecting reversal in the whole run -// failed. Here x.pdf's own undo-move fails, y.pdf's own undo-move also -// fails, and z.pdf's undo-mkdir - tidying up a directory that turned out -// empty, not restoring anything - is the run's only "ok" entry. Marking -// the original run Undone from that alone would be exactly Important 2's -// bug again, by a narrower route. +// TestRunsDoesNotMarkUndoneWhenOnlyOkEntryIsMkdir: "at least one ok +// undo-* entry" alone is too loose a check, by the same shape as the bug +// it fixes above. A file's own chain stops after a failed file-affecting +// reversal, but a failed or refused undo-mkdir deliberately does not stop +// anything (internal/engine's isFileAffecting draws exactly this line, +// and undoFile's stop-on-failure check shares it) - so an undo-mkdir +// belonging to one file can still succeed even though every +// file-affecting reversal in the whole run failed. Here x.pdf's own +// undo-move fails, y.pdf's own undo-move also fails, and z.pdf's +// undo-mkdir - tidying up a directory that turned out empty, not +// restoring anything - is the run's only "ok" entry. Marking the original +// run Undone from that alone would be the same bug again, by a narrower +// route. func TestRunsDoesNotMarkUndoneWhenOnlyOkEntryIsMkdir(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -196,12 +196,12 @@ func TestRunsDoesNotMarkUndoneWhenOnlyOkEntryIsMkdir(t *testing.T) { } } -// TestEntriesCrashedRunReturnsNilError is item 1: a run-start present, -// run-end absent, and otherwise clean is exactly the crashed-run shape -// Entries' own doc comment says it must accept - "to end of file when there -// is no run-end (a crashed run, which is precisely when corruption is -// likely)". Pinning it as its own test, rather than leaving it implicit in -// tests about something else, is the point of the item. +// TestEntriesCrashedRunReturnsNilError: a run-start present, run-end +// absent, and otherwise clean is exactly the crashed-run shape Entries' +// own doc comment says it must accept - "to end of file when there is no +// run-end (a crashed run, which is precisely when corruption is likely)". +// Pinning it as its own test, rather than leaving it implicit in tests +// about something else, is deliberate. func TestEntriesCrashedRunReturnsNilError(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -227,10 +227,10 @@ func TestEntriesCrashedRunReturnsNilError(t *testing.T) { } } -// TestEntriesIntactRunReturnsNilError is item 2: a complete, clean run - -// run-start, a step, run-end, nothing corrupt - must read back with a nil -// error. Every other test in this file needs this to be true along the way, -// but none of them state it as their own point; this one does. +// TestEntriesIntactRunReturnsNilError: a complete, clean run - run-start, +// a step, run-end, nothing corrupt - must read back with a nil error. +// Every other test in this file needs this to be true along the way, but +// none of them state it as their own point; this one does. func TestEntriesIntactRunReturnsNilError(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -258,9 +258,9 @@ func TestEntriesIntactRunReturnsNilError(t *testing.T) { } } -// TestEntriesBothFailureModesReportsBadLineFirst is item 3: a run that both -// has an unparsable line inside its window AND lacks a readable run-start -// must surface as the unparsable-line error, not the missing-run-start one - +// TestEntriesBothFailureModesReportsBadLineFirst: a run that both has an +// unparsable line inside its window AND lacks a readable run-start must +// surface as the unparsable-line error, not the missing-run-start one - // Entries checks badLine before sawRunStart. The run-start line here is // destroyed unattributably (as in TestEntriesFailsClosedOnMissingRunStart), // and a second, still-attributable line is separately corrupted so badLine @@ -303,9 +303,9 @@ func TestEntriesBothFailureModesReportsBadLineFirst(t *testing.T) { t.Fatal(err) } - // Since plan 10 a damaged line whose file is readable refuses only that - // file (a "damaged" entry); the run as a whole still fails closed here, - // on its missing run-start. + // A damaged line whose file is readable refuses only that file (a + // "damaged" entry); the run as a whole still fails closed here, on its + // missing run-start. got, err := Entries(path, "A") if err == nil { t.Fatal("Entries returned no error with a missing run-start") @@ -318,11 +318,10 @@ func TestEntriesBothFailureModesReportsBadLineFirst(t *testing.T) { } } -// TestEntriesAdjacentRunStartsOneCorrupted is item 4. Ruling R2: this pins -// what journal.Entries does TODAY for two runs whose run-start lines are -// adjacent, one of them corrupted - it does not assert an invented "correct" -// result, and internal/journal is not touched by this task. The log here is -// exactly: +// TestEntriesAdjacentRunStartsOneCorrupted pins what journal.Entries does +// for two runs whose run-start lines are adjacent, one of them corrupted - +// it asserts the observed behaviour, not an invented "correct" result. +// The log here is exactly: // // 1 run-start A (good) // 2 run-start B (corrupted: no tabs, unattributable) @@ -342,11 +341,10 @@ func TestEntriesBothFailureModesReportsBadLineFirst(t *testing.T) { // line), so it reaches the end of the file with no badLine, and instead // fails on B's missing run-start. // -// Concern (not fixed here, per R2 - flagged for judgement, not code -// change): the SAME corrupted line produces two different error shapes -// depending only on which run asks, which is a surprising inconsistency in -// the message a caller sees, even though both directions correctly fail -// closed. +// This is a known inconsistency, left as is rather than fixed: the SAME +// corrupted line produces two different error shapes depending only on +// which run asks, which is a surprising inconsistency in the message a +// caller sees, even though both directions correctly fail closed. func TestEntriesAdjacentRunStartsOneCorrupted(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -415,10 +413,10 @@ func TestEntriesAdjacentRunStartsOneCorrupted(t *testing.T) { // TestEntriesReportsAMangledLine: a corrupt line that is not the log's // final line must not be silently dropped by Entries the way Runs drops it -// - PlanUndo needs to know a step went missing. Since plan 10 (re-review -// N1) a line whose directory and file columns are readable is returned as a -// "damaged" entry for that file, so only that file is refused and the rest -// of the run can still be undone. +// - PlanUndo needs to know a step went missing. A line whose directory and +// file columns are readable is returned as a "damaged" entry for that +// file, so only that file is refused and the rest of the run can still be +// undone. func TestEntriesReportsAMangledLine(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -464,9 +462,9 @@ func TestEntriesReportsAMangledLine(t *testing.T) { } } -// TestEntriesIgnoresAnotherRunsDamagedLine: a damaged line whose run column -// names another run - two directories' runs can interleave in one log - does -// not refuse this run, even inside its window (re-review N1). +// TestEntriesIgnoresAnotherRunsDamagedLine: a damaged line whose run +// column names another run - two directories' runs can interleave in one +// log - does not refuse this run, even inside its window. func TestEntriesIgnoresAnotherRunsDamagedLine(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -657,8 +655,8 @@ func TestEntriesFailsClosedOnMissingRunStart(t *testing.T) { // TestReversedStepsCountsEveryUndoOfARun: the steps earlier undo runs of a // run already reversed - only "ok" undo entries of runs that undo it - so -// a later undo of the same run can offer just what is left (review M10). -// Runs also names the run an undo run reversed. +// a later undo of the same run can offer just what is left. Runs also +// names the run an undo run reversed. func TestReversedStepsCountsEveryUndoOfARun(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, err := Open(path) @@ -705,8 +703,8 @@ func TestReversedStepsCountsEveryUndoOfARun(t *testing.T) { // TestEntriesRefusesALineCutInsideItsRunColumn: a crash that cuts the last // line inside its run column leaves a prefix of some run's ID - it cannot -// be called another run's line, so inside this run's window it refuses the -// run (plan 10 re-check R2). +// be called another run's line, so inside this run's window it refuses +// the run. func TestEntriesRefusesALineCutInsideItsRunColumn(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -733,9 +731,9 @@ func TestEntriesRefusesALineCutInsideItsRunColumn(t *testing.T) { } // TestRunsMarksAPartlyUndoneRun: a run whose undo reversed some of its -// reversible steps but not all is partly undone; once a later undo reverses -// the rest, it is undone in full. A permanent delete counts toward neither -// (triage 34l). +// reversible steps but not all is partly undone; once a later undo +// reverses the rest, it is undone in full. A permanent delete counts +// toward neither. func TestRunsMarksAPartlyUndoneRun(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) @@ -780,9 +778,9 @@ func TestRunsMarksAPartlyUndoneRun(t *testing.T) { } } -// TestRunsIgnoresAPermanentlyDeletedFilesSteps: a file whose chain ended in -// a permanent delete can never be undone, so its earlier steps do not keep -// the run partly undone forever (plan 11 review L8). +// TestRunsIgnoresAPermanentlyDeletedFilesSteps: a file whose chain ended +// in a permanent delete can never be undone, so its earlier steps do not +// keep the run partly undone forever. func TestRunsIgnoresAPermanentlyDeletedFilesSteps(t *testing.T) { path := filepath.Join(t.TempDir(), "krino.log") w, _ := Open(path) |