diff --git a/testutils/faketimer.go b/testutils/faketimer.go index eeca3ce..8500d29 100644 --- a/testutils/faketimer.go +++ b/testutils/faketimer.go @@ -1,8 +1,8 @@ package testutils import ( - "github.com/wirenboard/wbgong" "github.com/stretchr/testify/require" + "github.com/wirenboard/wbgong" "log" "sync" "testing" diff --git a/testutils/robustness_test.go b/testutils/robustness_test.go new file mode 100644 index 0000000..23bf052 --- /dev/null +++ b/testutils/robustness_test.go @@ -0,0 +1,62 @@ +package testutils + +import ( + "os" + "strconv" + "testing" + "time" +) + +// A full record buffer must not block the producer (previously the driver +// goroutine deadlocked mid-transaction when a test generated more records +// than it Verified). Overflow drops with a log marker instead. +func TestRecorderOverflowDoesNotBlock(t *testing.T) { + rec := NewRecorder(t) + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 1500; i++ { + rec.Rec("item %d", i) + } + }() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatal("Rec blocked on a full buffer") + } + // the buffered 1000 records are still there for Verify + for i := 0; i < 1000; i++ { + rec.Verify("item " + strconv.Itoa(i)) + } + rec.VerifyEmpty() +} + +// SetupTempDir chdirs into the temp dir; if a test fails hard (FailNow or +// panic) its explicit cleanup never runs, and before this fix the whole +// process stayed chdir'd into a removed directory, silently breaking +// cwd-relative logic in every later test. t.Cleanup must restore it. +func TestSetupTempDirRestoresCwdWithoutExplicitCleanup(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + t.Run("leaky", func(t *testing.T) { + dir, _ := SetupTempDir(t) + if dir == "" { + t.Fatal("no temp dir") + } + // deliberately no cleanup call - simulates FailNow/panic paths + }) + t.Run("explicit-then-auto", func(t *testing.T) { + // explicit cleanup followed by the t.Cleanup re-run must be safe + _, cleanup := SetupTempDir(t) + cleanup() + }) + after, err := os.Getwd() + if err != nil { + t.Fatalf("cwd stranded in a removed directory: %v", err) + } + if after != wd { + t.Fatalf("cwd not restored: %q != %q", after, wd) + } +} diff --git a/testutils/testutils.go b/testutils/testutils.go index 501f75a..245503d 100644 --- a/testutils/testutils.go +++ b/testutils/testutils.go @@ -7,6 +7,7 @@ import ( "regexp" "sort" "sync" + "sync/atomic" "testing" "time" @@ -136,6 +137,7 @@ type Recorder struct { *Fixture ch chan string emptyWaitTime time.Duration + closed atomic.Bool // set when the owning test ends } func NewRecorder(t *testing.T) *Recorder { @@ -144,13 +146,33 @@ func NewRecorder(t *testing.T) *Recorder { ch: make(chan string, 1000), emptyWaitTime: REC_EMPTY_WAIT_TIME_MS * time.Millisecond, } + // producers (driver goroutines, timers) may outlive the test; calling + // t.Log after the test ends panics the whole binary, so silence the + // recorder on all test exit paths + t.Cleanup(func() { rec.closed.Store(true) }) return rec } func (rec *Recorder) Rec(format string, args ...any) { + if rec.closed.Load() { + return // the owning test is over; logging now would panic the binary + } item := fmt.Sprintf(format, args...) rec.t.Log("REC: ", item) - rec.ch <- item + select { + case rec.ch <- item: + default: + // A full buffer previously blocked the producer forever - typically + // the driver goroutine mid-transaction - deadlocking any test that + // generates more records than it Verifies (seen with a rule script + // defining a device with dozens of controls). Dropping the NEWEST + // record is diagnosable: the log above still has the item, and a + // later Verify of it fails with "timeout" right after this marker. + // Note for tests that drain via SkipTill: a sync marker published + // after >1000 undrained records is exactly what gets dropped - + // drain earlier or assert less traffic. + rec.t.Log("REC OVERFLOW (dropped): ", item) + } } func (rec *Recorder) SetEmptyWaitTime(duration time.Duration) { @@ -374,10 +396,15 @@ func SetupTempDir(t *testing.T) (path string, cleanup func()) { } os.Chdir(dir) - return dir, func() { + cleanup = func() { os.RemoveAll(dir) os.Chdir(wd) } + // t.Cleanup runs even when the test fails via FailNow or panics, so the + // process never stays chdir'd into a removed directory; the returned + // cleanup stays for explicit calls (both orders are safe). + t.Cleanup(cleanup) + return dir, cleanup } type Suite struct {