tests: add regression tests for audit fixes; record fix status

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
2026-09-24 09:16:04 -07:00
parent ae5193eb5f
commit 66259db177
3 changed files with 255 additions and 13 deletions
@@ -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
@@ -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).
@@ -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 <chrono>
+#include <string>
+#include <thread>
+
+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);
+}