0a66bf2 test(mocks): make cursor tests deterministic, harden fakes-check and mock regeneration · neferwl

M AGENTS.md +6 -2


@@ -11,14 +11,18 @@
11	11
12	12		Test doubles are always generated by Mockery v3 (`testify` template). Never handwrite them.
13	13
14		-	- No handwritten fakes, stubs or spies. This includes structs implementing an interface in a test and function fields swapped by tests (`x.ioctl = func...`). `make check` runs `fakes-check`, which rejects `type fake…`/`stub…`/`spy…`/`mock…` in tests.
	14	+	- No handwritten fakes, stubs or spies. This covers any test type standing in for a project interface, and function fields swapped by tests (`x.ioctl = func...`). `make check` runs `fakes-check`, which rejects `fake`/`stub`/`spy`/`dummy`/`mock` type names in tests.
15	15		- Every seam is an interface. When a test needs to replace a dependency, add an interface and generate its mock:
16	16		- Cross-layer dependencies go in `internal/ports`. Mocks: `internal/mocks/ports`, package `portsmocks`, type `MockX`.
17	17		- Adapter-internal seams (e.g. a kernel ioctl) stay unexported in the adapter. Mocks: `<name>_mock_test.go` in the same package.
18	18		- Add a `.mockery.yml` entry for each new interface, run `make mocks` and commit the generated files. `make mocks-check` (CI) fails if they are stale or missing.
19	19		- Use mocks through `.EXPECT()`. Record calls with `RunAndReturn` when a test inspects arguments.
20	20		- Channel ports (sealed message interfaces in `ports/messages.go`) are tested by sending messages, not with mocks.
21		-	- Real protocol clients in wayland tests (`wlturbo` proxies) are test clients, not doubles.
	21	+	- Not doubles, so allowed:
	22	+	- real protocol clients in wayland tests (`wlturbo` proxies);
	23	+	- real in-memory implementations of std interfaces (e.g. a locked `bytes.Buffer` as `io.Writer`);
	24	+	- production constructor parameters such as `NewRenderer func(w, h) (ports.Renderer, error)`, where tests return a generated mock;
	25	+	- channels passed to `app.run` to inject input or observe scenes (channel ports).
22	26
23	27		## Checks before merge
24	28

 Test doubles are always generated by Mockery v3 (`testify` template). Never handwrite them.
-- **No handwritten fakes, stubs or spies.** This includes structs implementing an interface in a test and function fields swapped by tests (`x.ioctl = func...`). `make check` runs `fakes-check`, which rejects `type fake…`/`stub…`/`spy…`/`mock…` in tests.
+- **No handwritten fakes, stubs or spies.** This covers any test type standing in for a project interface, and function fields swapped by tests (`x.ioctl = func...`). `make check` runs `fakes-check`, which rejects `fake`/`stub`/`spy`/`dummy`/`mock` type names in tests.
 - **Every seam is an interface.** When a test needs to replace a dependency, add an interface and generate its mock:
   - Cross-layer dependencies go in `internal/ports`. Mocks: `internal/mocks/ports`, package `portsmocks`, type `MockX`.
   - Adapter-internal seams (e.g. a kernel ioctl) stay unexported in the adapter. Mocks: `<name>_mock_test.go` in the same package.
 - **Add a `.mockery.yml` entry** for each new interface, run `make mocks` and commit the generated files. `make mocks-check` (CI) fails if they are stale or missing.
 - Use mocks through `.EXPECT()`. Record calls with `RunAndReturn` when a test inspects arguments.
 - Channel ports (sealed message interfaces in `ports/messages.go`) are tested by sending messages, not with mocks.
-- Real protocol clients in wayland tests (`wlturbo` proxies) are test clients, not doubles.
+- Not doubles, so allowed:
+  - real protocol clients in wayland tests (`wlturbo` proxies);
+  - real in-memory implementations of std interfaces (e.g. a locked `bytes.Buffer` as `io.Writer`);
+  - production constructor parameters such as `NewRenderer func(w, h) (ports.Renderer, error)`, where tests return a generated mock;
+  - channels passed to `app.run` to inject input or observe scenes (channel ports).
 ## Checks before merge

M Makefile +8 -2


@@ -26,16 +26,22 @@ race:
26	26		# cgo is enabled only for the race test binary.
27	27		CGO_ENABLED=1 go test -race ./...
28	28		MOCKS := internal/mocks ':(glob)internal/*/_mock_test.go'
	29	+	# Regenerate from scratch so mocks of removed interfaces disappear too.
29	30		mocks:
	31	+	rm -rf internal/mocks
	32	+	find internal -name '*_mock_test.go' -delete
30	33		$(HOME)/go/bin/mockery
31	34		# Generated mocks must be committed and up to date.
32	35		mocks-check: mocks
33	36		git diff --exit-code -- $(MOCKS)
34	37		@test -z "$$(git ls-files --others --exclude-standard -- $(MOCKS))"
35	38		# Test doubles come from Mockery only: no handwritten fake/stub/spy types.
	39	+	# Matches `type fakeX`, `\ttype RendererStub[T any]` and `spyX struct` in `type (` blocks.
	40	+	# grep exit 1 (no match) passes; 0 (match) and 2 (error) fail.
	41	+	FAKES := ^\stype\s+\w(fake\|stub\|spy\|dummy\|mock)\w\|^\s\w(fake\|stub\|spy\|dummy\|mock)\w(\[[^]]*\])?\s+(struct\|interface)\b
36	42		fakes-check:
37		-	@! grep -rnE '^type +(fake\|stub\|spy\|dummy\|mock)[A-Za-z0-9_]* ' --include='_test.go' --exclude='_mock_test.go' internal cmd \
38		-	\|\| (echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1)
	43	+	@rc=0; grep -rniE '$(FAKES)' --include='_test.go' --exclude='_mock_test.go' internal cmd \|\| rc=$$?; \
	44	+	[ $$rc -eq 1 ] \|\| { [ $$rc -eq 0 ] && echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1; }
39	45		arch:
40	46		$(HOME)/go/bin/hexcheck -hexcheck.config .hexcheck.yaml -hexcheck.root . ./...
41	47		check: vet test arch fakes-check

 	# cgo is enabled only for the race test binary.
 	CGO_ENABLED=1 go test -race ./...
 MOCKS := internal/mocks ':(glob)internal/**/*_mock_test.go'
+# Regenerate from scratch so mocks of removed interfaces disappear too.
 mocks:
+	rm -rf internal/mocks
+	find internal -name '*_mock_test.go' -delete
 	$(HOME)/go/bin/mockery
 # Generated mocks must be committed and up to date.
 mocks-check: mocks
 	git diff --exit-code -- $(MOCKS)
 	@test -z "$$(git ls-files --others --exclude-standard -- $(MOCKS))"
 # Test doubles come from Mockery only: no handwritten fake/stub/spy types.
+# Matches `type fakeX`, `\ttype RendererStub[T any]` and `spyX struct` in `type (` blocks.
+# grep exit 1 (no match) passes; 0 (match) and 2 (error) fail.
+FAKES := ^\s*type\s+\w*(fake|stub|spy|dummy|mock)\w*|^\s*\w*(fake|stub|spy|dummy|mock)\w*(\[[^]]*\])?\s+(struct|interface)\b
 fakes-check:
-	@! grep -rnE '^type +(fake|stub|spy|dummy|mock)[A-Za-z0-9_]* ' --include='*_test.go' --exclude='*_mock_test.go' internal cmd \
+	@rc=0; grep -rniE '$(FAKES)' --include='*_test.go' --exclude='*_mock_test.go' internal cmd || rc=$$?; \
-		|| (echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1)
+	[ $$rc -eq 1 ] || { [ $$rc -eq 0 ] && echo 'handwritten test double: add an interface and a Mockery entry (see AGENTS.md)' >&2; exit 1; }
 arch:
 	$(HOME)/go/bin/hexcheck -hexcheck.config .hexcheck.yaml -hexcheck.root . ./...
 check: vet test arch fakes-check

M internal/adapters/drm/cursor_test.go +40 -10

@@ -1,6 +1,7 @@
1	1		package drm
2	2
3	3		import (
	4	+	"sync"
4	5		"testing"
5	6		"time"
6	7		"unsafe"

@@ -58,19 +59,47 @@ func TestCursorMovesOncePerVblank(t *testing.T) {
58	59		}
59	60
60	61		// Move must never wait for the plane, or libinput stops reading and the
61		-	// kernel drops mouse reports.
	62	+	// kernel drops mouse reports. Moves made meanwhile are coalesced.
62	63		func TestCursorMoveDoesNotBlockOnPlane(t *testing.T) {
63	64		c, plane := testCursor(t)
64		-	release := make(chan struct{})
65		-	plane.EXPECT().WaitVblank().RunAndReturn(func() error { <-release; return nil })
66		-	plane.EXPECT().Set(mock.Anything).Return(nil).Maybe()
	65	+	c.hotX, c.hotY = 3, 4
	66	+	started, release := make(chan struct{}), make(chan struct{})
	67	+	var once sync.Once
	68	+	moved := make(chan modeCursor2, 1000)
	69	+	plane.EXPECT().WaitVblank().RunAndReturn(func() error {
	70	+	once.Do(func() { close(started) })
	71	+	<-release
	72	+	return nil
	73	+	})
	74	+	plane.EXPECT().Set(mock.Anything).RunAndReturn(func(v modeCursor2) error { moved <- v; return nil })
67	75		c.start()
68		-	defer func() { close(release); c.close() }()
69		-	for i := range 1000 { // the worker is stuck in WaitVblank the whole time
70		-	c.Move(float64(i), 0)
	76	+	defer c.close()
	77	+	c.Move(0, 0)
	78	+	<-started // the worker is now held in WaitVblank
	79	+	done := make(chan struct{})
	80	+	go func() {
	81	+	for i := range 1000 {
	82	+	c.Move(float64(i), float64(2*i))
	83	+	}
	84	+	close(done)
	85	+	}()
	86	+	select {
	87	+	case <-done:
	88	+	case <-time.After(2 * time.Second):
	89	+	t.Fatal("Move blocks while the plane waits for vblank")
71	90		}
72		-	if s := c.TakeStats(); s.Moves != 1000 {
73		-	t.Fatalf("stats %+v", s)
	91	+	close(release)
	92	+	for n := 1; ; n++ {
	93	+	v := <-moved
	94	+	if v.flags != cursorMove {
	95	+	t.Fatalf("flags %#x", v.flags)
	96	+	}
	97	+	if v.x == 999-3 && v.y == 1998-4 {
	98	+	if n >= 1000 {
	99	+	t.Fatalf("%d moves applied; want coalesced", n)
	100	+	}
	101	+	return
	102	+	}
74	103		}
75	104		}
76	105

@@ -101,8 +130,9 @@ func TestCursorStats(t *testing.T) {
101	130		<-applied
102	131		c.Move(2, 2)
103	132		<-applied
	133	+	// Set signals before the worker counts the ioctl.
104	134		s := c.TakeStats()
105		-	if s.Moves != 2 \|\| s.Ioctls != 2 {
	135	+	if s.Moves != 2 \|\| s.Ioctls < 1 \|\| s.Ioctls > 2 {
106	136		t.Fatalf("stats %+v", s)
107	137		}
108	138		if s := c.TakeStats(); s.Moves != 0 {

 package drm
 import (
+	"sync"
 	"testing"
 	"time"
 	"unsafe"
 }
 // Move must never wait for the plane, or libinput stops reading and the
-// kernel drops mouse reports.
+// kernel drops mouse reports. Moves made meanwhile are coalesced.
 func TestCursorMoveDoesNotBlockOnPlane(t *testing.T) {
 	c, plane := testCursor(t)
-	release := make(chan struct{})
+	c.hotX, c.hotY = 3, 4
-	plane.EXPECT().WaitVblank().RunAndReturn(func() error { <-release; return nil })
+	started, release := make(chan struct{}), make(chan struct{})
-	plane.EXPECT().Set(mock.Anything).Return(nil).Maybe()
+	var once sync.Once
+	moved := make(chan modeCursor2, 1000)
+	plane.EXPECT().WaitVblank().RunAndReturn(func() error {
+		once.Do(func() { close(started) })
+		<-release
+		return nil
+	})
+	plane.EXPECT().Set(mock.Anything).RunAndReturn(func(v *modeCursor2) error { moved <- *v; return nil })
 	c.start()
-	defer func() { close(release); c.close() }()
+	defer c.close()
-	for i := range 1000 { // the worker is stuck in WaitVblank the whole time
+	c.Move(0, 0)
-		c.Move(float64(i), 0)
+	<-started // the worker is now held in WaitVblank
+	done := make(chan struct{})
+	go func() {
+		for i := range 1000 {
+			c.Move(float64(i), float64(2*i))
+		}
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("Move blocks while the plane waits for vblank")
 	}
-	if s := c.TakeStats(); s.Moves != 1000 {
+	close(release)
-		t.Fatalf("stats %+v", s)
+	for n := 1; ; n++ {
+		v := <-moved
+		if v.flags != cursorMove {
+			t.Fatalf("flags %#x", v.flags)
+		}
+		if v.x == 999-3 && v.y == 1998-4 {
+			if n >= 1000 {
+				t.Fatalf("%d moves applied; want coalesced", n)
+			}
+			return
+		}
 	}
 }
 	<-applied
 	c.Move(2, 2)
 	<-applied
+	// Set signals before the worker counts the ioctl.
 	s := c.TakeStats()
-	if s.Moves != 2 || s.Ioctls != 2 {
+	if s.Moves != 2 || s.Ioctls < 1 || s.Ioctls > 2 {
 		t.Fatalf("stats %+v", s)
 	}
 	if s := c.TakeStats(); s.Moves != 0 {

M internal/adapters/headless/output_test.go +3 -1


@@ -83,7 +83,9 @@ func TestRun(t *testing.T) {
83	83		}
84	84		contents <- ports.SurfaceContent{ID: 1}
85	85		waitFrames(t, f, 2)
86		-	if _, c := f.snapshot(); c[1][1].Pixels != nil {
	86	+	if _, c := f.snapshot(); len(c) < 2 {
	87	+	t.Fatal("no second frame")
	88	+	} else if _, ok := c[1][1]; ok {
87	89		t.Error("content not deleted")
88	90		}
89	91		cancel()

 	}
 	contents <- ports.SurfaceContent{ID: 1}
 	waitFrames(t, f, 2)
-	if _, c := f.snapshot(); c[1][1].Pixels != nil {
+	if _, c := f.snapshot(); len(c) < 2 {
+		t.Fatal("no second frame")
+	} else if _, ok := c[1][1]; ok {
 		t.Error("content not deleted")
 	}
 	cancel()