testutils: don't deadlock on Recorder overflow; restore cwd via t.Cleanup

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"

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)
+	}
+}

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 {