From 66259db177e5c04cf57661c58fe0985d8657a737 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 03:17:00 +0000 Subject: [PATCH] tests: add regression tests for audit fixes; record fix status Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB --- CMakeLists.txt | 1 + docs/audits/claude-findings-20260923.md | 40 +++-- tests/test_audit_regressions.cc | 227 ++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 tests/test_audit_regressions.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 1b78427..84cc805 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -372,6 +372,7 @@ if (BUILD_TESTS) tests/test_reflow_undo.cc tests/test_syntax_highlighting.cc tests/test_paste_split.cc + tests/test_audit_regressions.cc # minimal engine sources required by Buffer PieceTable.cc diff --git a/docs/audits/claude-findings-20260923.md b/docs/audits/claude-findings-20260923.md index e9516f4..2bd7121 100644 --- a/docs/audits/claude-findings-20260923.md +++ b/docs/audits/claude-findings-20260923.md @@ -302,16 +302,30 @@ Updated as fixes land on `claude/funny-fermat-gj2vm5`. | ID | Status | |----|--------| -| M1 | open | -| M2 | open | -| M4 | open | -| M5 | open | -| D1 | open | -| D2 | open | -| D6 | open | -| D7 (torn tail) | open | -| D8 | open | -| D9 | open | -| B1 | open | -| B2 | open | -| B12 | open | +| M1 | fixed: assignment operators invalidate the line index | +| M2 | open: needs a decision (drop the warm-up thread, or snapshot) | +| M3 | open | +| M4 | fixed: iterative free/search; `clear()` also freed only the first root branch (leak), now frees all | +| M5 | fixed: jump-to-mark clamps the cursor | +| D1 | fixed: `Begin` before the cursor moves; newline + indent grouped | +| D2 | fixed for scroll up/down and replace-all restore | +| D3 | open | +| D4 | open | +| D5 | replace-all fixed (one undo group); visual-line newline open | +| D6 | fixed: reflow on a blank line is a no-op | +| D7 | open: recovering a torn tail reverses `SwapReplay_TruncatedLog_FailsSafely`; needs a decision | +| D8 | fixed: window checked before recording the failure | +| D9 | fixed: `SetDirty` always bumps the version; reload and `replace_all_bytes` too; reload of a deleted file clears undo | +| B1 | fixed: guard on `NCURSES_VERSION` | +| B2 | fixed: `KEY_RESIZE` issues no command | +| B3–B10, B13 | open | +| B11 | fixed: SmartNewline with a prompt open delegates to Newline | +| B12 | fixed: merged appends invalidate the line index | +| P1 | partial: `ensure_cursor_visible` no longer calls `Rows()`; renderers still do | +| P2 | partial: `ensure_cursor_visible` no longer materializes the buffer | +| P3 | open | +| P4 | replace-all fixed; other region commands open | +| P5, P6 | open | + +Regression tests: `tests/test_audit_regressions.cc`. Each test fails on the +unfixed code (M4 by crashing). diff --git a/tests/test_audit_regressions.cc b/tests/test_audit_regressions.cc new file mode 100644 index 0000000..148d4c6 --- /dev/null +++ b/tests/test_audit_regressions.cc @@ -0,0 +1,227 @@ +/* + * test_audit_regressions.cc - Regression tests for defects recorded in + * docs/audits/claude-findings-20260923.md. Each test names its finding ID. + */ +#include "Test.h" + +#include "tests/TestHarness.h" + +#include "ErrorRecovery.h" +#include "PieceTable.h" +#include "UndoTree.h" + +#include +#include +#include + +using ktet::TestHarness; + + +// M1: assignment must not keep the destination's old line index. +TEST(Audit_M1_PieceTable_MoveAssign_ResetsLineIndex) +{ + const std::string a_text = "line1\nline2\nline3\n"; + PieceTable a; + a.Append(a_text.data(), a_text.size()); + ASSERT_EQ(a.LineCount(), (std::size_t) 4); // builds a's index + + PieceTable b; + b.Append("x", 1); + a = std::move(b); + + ASSERT_EQ(a.Size(), (std::size_t) 1); + ASSERT_EQ(a.LineCount(), (std::size_t) 1); + auto [start, end] = a.GetLineRange(0); + ASSERT_EQ(start, (std::size_t) 0); + ASSERT_EQ(end, (std::size_t) 1); +} + + +TEST(Audit_M1_PieceTable_CopyAssign_ResetsLineIndex) +{ + const std::string a_text = "one\ntwo\nthree\n"; + PieceTable a; + a.Append(a_text.data(), a_text.size()); + ASSERT_EQ(a.LineCount(), (std::size_t) 4); + + PieceTable b; + b.Append("x", 1); + a = b; + + ASSERT_EQ(a.LineCount(), (std::size_t) 1); + ASSERT_EQ(a.GetLine(0), std::string("x")); +} + + +// B12: appends that merge into the last piece must invalidate the line index. +TEST(Audit_B12_PieceTable_MergedAppend_InvalidatesLineIndex) +{ + PieceTable t; + t.Append("ab", 2); + ASSERT_EQ(t.LineCount(), (std::size_t) 1); + t.Append("\n", 1); + ASSERT_EQ(t.LineCount(), (std::size_t) 2); +} + + +// M4: destroying a very long undo history must not recurse per node. +TEST(Audit_M4_UndoTree_DeepHistory_FreesWithoutRecursion) +{ + constexpr int kDepth = 1000000; + auto *tree = new UndoTree(); + UndoNode *prev = nullptr; + for (int i = 0; i < kDepth; ++i) { + auto *n = new UndoNode(); + n->parent = prev; + if (prev) + prev->child = n; + else + tree->root = n; + prev = n; + } + tree->current = prev; + delete tree; // stack overflow before the fix + ASSERT_TRUE(true); +} + + +// D1: undoing a smart newline must restore the original text in one step. +TEST(Audit_D1_SmartNewline_UndoRestoresText) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, " foobar\nnext"); + b.SetCursor(7, 0); + + ASSERT_TRUE(h.Exec(CommandId::SmartNewline)); + ASSERT_EQ(h.Line(0), std::string(" foo")); + ASSERT_EQ(h.Line(1), std::string(" bar")); + ASSERT_EQ(b.Curx(), (std::size_t) 4); + + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string(" foobar\nnext")); + + ASSERT_TRUE(h.Redo()); + ASSERT_EQ(h.Text(), std::string(" foo\n bar\nnext")); +} + + +// B11: with a prompt open, SmartNewline accepts the prompt and edits nothing. +TEST(Audit_B11_SmartNewline_WithPrompt_DoesNotIndentBuffer) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + Buffer &b = h.Buf(); + b.insert_text(0, 0, " foo"); + b.SetCursor(7, 0); + + ASSERT_TRUE(h.Exec(CommandId::FindStart)); + ASSERT_TRUE(ed.PromptActive()); + ASSERT_TRUE(h.Exec(CommandId::SmartNewline)); + ASSERT_TRUE(!ed.PromptActive()); + ASSERT_EQ(h.Text(), std::string(" foo")); +} + + +// D5: replace-all is a single undo step. +TEST(Audit_D5_ReplaceAll_SingleUndoStep) +{ + TestHarness h; + Editor &ed = h.EditorRef(); + Buffer &b = h.Buf(); + b.insert_text(0, 0, "a1 a2\na3"); + b.SetCursor(0, 0); + + ASSERT_TRUE(h.Exec(CommandId::SearchReplace)); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "a")); + ASSERT_TRUE(h.Exec(CommandId::Newline)); + ASSERT_EQ(ed.CurrentPromptKind(), Editor::PromptKind::ReplaceWith); + ASSERT_TRUE(h.Exec(CommandId::InsertText, "bb")); + ASSERT_TRUE(h.Exec(CommandId::Newline)); + ASSERT_TRUE(!ed.PromptActive()); + ASSERT_EQ(h.Text(), std::string("bb1 bb2\nbb3")); + + ASSERT_TRUE(h.Undo()); + ASSERT_EQ(h.Text(), std::string("a1 a2\na3")); +} + + +// D2: scrolling that drags the cursor onto a shorter line clamps its column, +// so a following Backspace edits that line rather than joining lines. +TEST(Audit_D2_ScrollDown_ClampsCursorColumn) +{ + TestHarness h; + Buffer &b = h.Buf(); + std::string text(50, 'x'); + for (int i = 0; i < 60; ++i) + text += "\nab"; + b.insert_text(0, 0, text); + b.SetCursor(40, 0); + b.SetOffsets(0, 0); + + ASSERT_TRUE(h.Exec(CommandId::ScrollDown, std::string(), 5)); + ASSERT_EQ(b.Cury(), (std::size_t) 5); + ASSERT_EQ(b.Curx(), (std::size_t) 2); + + const std::size_t rows_before = b.Nrows(); + ASSERT_TRUE(h.Exec(CommandId::Backspace)); + ASSERT_EQ(b.Nrows(), rows_before); + ASSERT_EQ(h.Line(5), std::string("a")); +} + + +// M5: jumping to a mark that edits have left past the end clamps the cursor. +TEST(Audit_M5_JumpToMark_ClampsToBuffer) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "one\ntwo"); + b.SetMark(5, 40); // stale: no such row or column + b.SetCursor(0, 0); + + ASSERT_TRUE(h.Exec(CommandId::JumpToMark)); + ASSERT_EQ(b.Cury(), (std::size_t) 1); + ASSERT_EQ(b.Curx(), (std::size_t) 3); +} + + +// D6: reflow with the cursor on a blank separator line is a no-op. +TEST(Audit_D6_Reflow_OnBlankLine_KeepsParagraphsSeparate) +{ + TestHarness h; + Buffer &b = h.Buf(); + b.insert_text(0, 0, "aaa\n\nbbb"); + b.SetCursor(0, 1); + + ASSERT_TRUE(h.Exec(CommandId::ReflowParagraph)); + ASSERT_EQ(h.Text(), std::string("aaa\n\nbbb")); +} + + +// D9: clearing the dirty flag (undo to the saved state) changes the version +// so version-keyed highlight caches are refreshed. +TEST(Audit_D9_SetDirtyFalse_BumpsVersion) +{ + Buffer b; + b.SetDirty(true); + const auto v = b.Version(); + b.SetDirty(false); + ASSERT_TRUE(b.Version() != v); +} + + +// D8: failures farther apart than the window must not accumulate. +TEST(Audit_D8_CircuitBreaker_WindowExpires) +{ + kte::CircuitBreaker::Config cfg; + cfg.failure_threshold = 2; + cfg.window = std::chrono::seconds(1); + kte::CircuitBreaker cb(cfg); + + cb.RecordFailure(); + std::this_thread::sleep_for(std::chrono::milliseconds(1100)); + cb.RecordFailure(); + + ASSERT_TRUE(cb.GetState() == kte::CircuitBreaker::State::Closed); + ASSERT_EQ(cb.GetFailureCount(), (std::size_t) 1); +}