From 0bd0fbf03eff9a79d61fde62d7b0593051d76741 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 09:32:25 +0200 Subject: [PATCH] test(brainfstest): swap-race asserts fail when nothing was tested AssertNoLeakFrom and the search Query race test passed vacuously when every symlink swap failed or every read errored. Both now count successful symlink swaps and reads that returned the inside note and fail if either is zero; the Query race reader no longer swallows Query's error. Verified by flipping: a swapper whose os.Symlink always fails turns notes, wiki and search race tests red; Query on a missing brain dir turns the search race test red. Closes #177 Co-Authored-By: Claude Opus 5.5 --- .../brainfs/brainfstest/brainfstest.go | 50 +++++++++--- .../brainfs/brainfstest/brainfstest_test.go | 79 +++++++++++++++++++ ingestion/internal/search/brainroot_test.go | 35 +++++--- 3 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 ingestion/internal/brainfs/brainfstest/brainfstest_test.go diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest.go b/ingestion/internal/brainfs/brainfstest/brainfstest.go index fdfe968..c352194 100644 --- a/ingestion/internal/brainfs/brainfstest/brainfstest.go +++ b/ingestion/internal/brainfs/brainfstest/brainfstest.go @@ -27,7 +27,7 @@ const outsideContent = "---\ntitle: " + Sentinel + "\naliases:\n - " + Sentinel "\n\n" + Sentinel + " [[" + Sentinel + "]]\n" // Outside writes a file outside any brain dir containing Sentinel. -func Outside(t *testing.T) string { +func Outside(t testing.TB) string { t.Helper() p := filepath.Join(t.TempDir(), "secret.md") if err := os.WriteFile(p, []byte(outsideContent), 0o644); err != nil { @@ -42,9 +42,12 @@ var swapSeq atomic.Int64 // a symlink to outside or a FIFO into path, until the test ends. FIFOs get a // writer held open until the swapper exits, so a reader that skips the // handle check parks forever instead of reading EOF. The swapper is stopped -// and awaited before t.TempDir cleanup. -func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { +// and awaited before t.TempDir cleanup. It returns the number of symlinks +// or FIFOs actually renamed into path so far, so a caller can tell a race +// that ran from one whose every swap failed (hyperguild#177). +func StartSwap(t testing.TB, path, inside, outside string, fifo bool) *atomic.Int64 { t.Helper() + var swaps atomic.Int64 var stop atomic.Bool var wg sync.WaitGroup wg.Add(1) @@ -67,22 +70,26 @@ func StartSwap(t *testing.T, path, inside, outside string, fifo bool) { }() for i := 0; !stop.Load(); i++ { _ = os.Remove(tmp) + var made error switch { case i%2 == 0: _ = os.WriteFile(tmp, []byte(inside), 0o644) case fifo: - if err := syscall.Mkfifo(tmp, 0o644); err == nil { + if made = syscall.Mkfifo(tmp, 0o644); made == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { writers = append(writers, w) } } default: - _ = os.Symlink(link, tmp) + made = os.Symlink(link, tmp) + } + if err := os.Rename(tmp, path); err == nil && made == nil && i%2 == 1 { + swaps.Add(1) } - _ = os.Rename(tmp, path) } }() t.Cleanup(func() { stop.Store(true); wg.Wait() }) + return &swaps } // WriteSentinelAt writes a file carrying Sentinel at p (creating its @@ -107,18 +114,41 @@ func AssertNoLeak(t *testing.T, path, inside string, d time.Duration, read func( AssertNoLeakFrom(t, path, inside, Outside(t), d, read) } +// InsideMarker must appear in the inside note's content and in read's +// output whenever read saw that note: the floor below counts such reads. +const InsideMarker = "inside" + // AssertNoLeakFrom is AssertNoLeak with the symlink pointing at target, e.g. // a file elsewhere in the brain written with WriteSentinelAt. -func AssertNoLeakFrom(t *testing.T, path, inside, target string, d time.Duration, read func() string) { +// +// Beyond "no Sentinel in any output" it sets two floors, because without +// them the assertion passes when nothing was tested (hyperguild#177): at +// least one symlink must have been swapped into path, and at least one read +// must have returned InsideMarker (a reader that always errors returns +// nothing, and so never leaks). +func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration, read func() string) { t.Helper() - StartSwap(t, path, inside, target, false) - leaks, iters := 0, 0 + if !strings.Contains(inside, InsideMarker) { + t.Fatalf("inside content must contain %q", InsideMarker) + } + swaps := StartSwap(t, path, inside, target, false) + leaks, iters, insideReads := 0, 0, 0 for deadline := time.Now().Add(d); time.Now().Before(deadline); iters++ { - leaks += strings.Count(read(), Sentinel) + out := read() + leaks += strings.Count(out, Sentinel) + if strings.Contains(out, InsideMarker) { + insideReads++ + } } if leaks > 0 { t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) } + if n := swaps.Load(); n == 0 { + t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters) + } + if insideReads == 0 { + t.Fatalf("no read in %d iterations returned the inside note (%q): the reader read nothing", iters, InsideMarker) + } } // AssertNoHang loops read for d while a FIFO swapper (held writers) runs on diff --git a/ingestion/internal/brainfs/brainfstest/brainfstest_test.go b/ingestion/internal/brainfs/brainfstest/brainfstest_test.go new file mode 100644 index 0000000..7dfc857 --- /dev/null +++ b/ingestion/internal/brainfs/brainfstest/brainfstest_test.go @@ -0,0 +1,79 @@ +package brainfstest + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "syscall" + "testing" + "time" +) + +// recordingTB stands in for the test under AssertNoLeakFrom so a fatal +// there can be observed instead of failing this test. Fatalf ends the +// calling goroutine the way testing.T does. +type recordingTB struct { + testing.TB + failed bool + msg string +} + +func (r *recordingTB) Fatalf(format string, args ...any) { + r.failed, r.msg = true, fmt.Sprintf(format, args...) + runtime.Goexit() +} + +func runAssert(t *testing.T, path, target string, read func() string) *recordingTB { + t.Helper() + r := &recordingTB{TB: t} + done := make(chan struct{}) + go func() { + defer close(done) + AssertNoLeakFrom(r, path, "inside", target, 200*time.Millisecond, read) + }() + <-done + return r +} + +// hyperguild#177: with every swap failing, no leak can occur, so the leak +// assertion passed without testing anything. +func TestAssertNoLeakFromFailsWhenNoSymlinkSwapSucceeds(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing-dir", "r.md") + r := runAssert(t, path, Outside(t), func() string { return "inside" }) + if !r.failed { + t.Fatal("AssertNoLeakFrom passed with zero successful symlink swaps") + } +} + +// hyperguild#177: a reader that always errors returns nothing, so it never +// leaks either. +func TestAssertNoLeakFromFailsWhenNoReadReturnsTheInsideNote(t *testing.T) { + path := filepath.Join(t.TempDir(), "r.md") + r := runAssert(t, path, Outside(t), func() string { return "" }) + if !r.failed { + t.Fatal("AssertNoLeakFrom passed with zero reads returning the inside note") + } +} + +// Positive control: a reader that refuses symlinks and reads regular files +// passes, so the floors do not fail a working reader. +func TestAssertNoLeakFromPassesAReaderThatRefusesSymlinks(t *testing.T) { + path := filepath.Join(t.TempDir(), "r.md") + read := func() string { + f, err := os.OpenFile(path, os.O_RDONLY|noFollow, 0) + if err != nil { + return "" + } + defer func() { _ = f.Close() }() + b := make([]byte, 64) + n, _ := f.Read(b) + return string(b[:n]) + } + r := runAssert(t, path, Outside(t), read) + if r.failed { + t.Fatalf("a symlink-refusing reader failed: %s", r.msg) + } +} + +const noFollow = syscall.O_NOFOLLOW diff --git a/ingestion/internal/search/brainroot_test.go b/ingestion/internal/search/brainroot_test.go index 6acb636..7c70ad0 100644 --- a/ingestion/internal/search/brainroot_test.go +++ b/ingestion/internal/search/brainroot_test.go @@ -93,16 +93,20 @@ func TestQueryDoesNotFollowASymlinkedWingUnderAWingAndHallRoot(t *testing.T) { // bypass of readOpened hung Query after 5 iterations). // startSwap runs swapInto until the test ends and waits for it before // t.TempDir's cleanup (hyperguild#162). -func startSwap(t *testing.T, path, outside string, fifo bool) { +// It returns the count of symlinks or FIFOs actually renamed into path +// (hyperguild#177: a swapper whose every swap fails tests nothing). +func startSwap(t *testing.T, path, outside string, fifo bool) *atomic.Int64 { t.Helper() var stop atomic.Bool + var swaps atomic.Int64 var wg sync.WaitGroup wg.Add(1) - go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop) }() + go func() { defer wg.Done(); swapInto(path, outside, fifo, &stop, &swaps) }() t.Cleanup(func() { stop.Store(true); wg.Wait() }) + return &swaps } -func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { +func swapInto(path, outside string, fifo bool, stop *atomic.Bool, swaps *atomic.Int64) { tmp := path + ".swap" // Writers stay open until the swapper exits, so a reader parked on a // FIFO stays parked (closing one would hand the reader EOF and hide the @@ -115,33 +119,46 @@ func swapInto(path, outside string, fifo bool, stop *atomic.Bool) { }() for i := 0; !stop.Load(); i++ { _ = os.Remove(tmp) + var made error switch { case i%2 == 0: _ = os.WriteFile(tmp, []byte("# r\n\nzqinside "+"zqracenote"), 0o644) case fifo: - if err := syscall.Mkfifo(tmp, 0o644); err == nil { + if made = syscall.Mkfifo(tmp, 0o644); made == nil { if w, err := os.OpenFile(tmp, os.O_RDWR, 0); err == nil { writers = append(writers, w) } } default: - _ = os.Symlink(outside, tmp) + made = os.Symlink(outside, tmp) + } + if err := os.Rename(tmp, path); err == nil && made == nil && i%2 == 1 { + swaps.Add(1) } - _ = os.Rename(tmp, path) } } func TestQueryLeaksNothingUnderASymlinkSwapRace(t *testing.T) { brain, facts, outside := searchBrain(t) - startSwap(t, filepath.Join(facts, "r.md"), outside, false) - leaks, iters := 0, 0 + swaps := startSwap(t, filepath.Join(facts, "r.md"), outside, false) + leaks, iters, insideReads := 0, 0, 0 for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); iters++ { - rs, _ := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) + rs, err := Query(brain, QueryOptions{Query: sentinel157, Limit: 10}) + require.NoError(t, err) leaks += leaked(rs) + // Positive control (hyperguild#177): a Query that always errors + // returns nothing and so never leaks. + rs, err = Query(brain, QueryOptions{Query: "zqracenote", Limit: 10}) + require.NoError(t, err) + if len(rs) > 0 { + insideReads++ + } } if leaks > 0 { t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters) } + require.Positive(t, swaps.Load(), "no symlink was swapped in: the race did not run") + require.Positive(t, insideReads, "no Query returned the inside note") } func TestQueryDoesNotHangUnderAFIFOSwapRace(t *testing.T) {