refactor(daemon): move test-only helpers out of production code
- commit
- cad1d13f7184b7476854abf775c30c6df0d09812
- author
- brice · · 2026-10-08 20:07 UTC
- parent
- 00fabb2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- tag: v0.8.0
6 files · +48 -45
M internal/usecase/daemon/attachment_output.go +0 -18
| @@ -546,15 +546,6 @@ func marshalOutputState(data []byte, epoch, base, next, echoAck, viewRevision ui | |||
| 546 | 546 | return output, nil | |
| 547 | 547 | } | |
| 548 | 548 | ||
| 549 | - | // sideEffect builds output without advancing the state stream. When an | |
| 550 | - | // attachment is present, callers must hold its sendMu; attachment-bound sends | |
| 551 | - | // use sideEffectLocked after validating their connection incarnation. | |
| 552 | - | func (s *attachmentOutput) sideEffect(data []byte, echoAck uint64) (protocol.Output, error) { | |
| 553 | - | s.lockView() | |
| 554 | - | defer s.unlockView() | |
| 555 | - | return s.sideEffectLocked(data, echoAck) | |
| 556 | - | } | |
| 557 | - | ||
| 558 | 549 | // sideEffectLocked requires the attachment view lock for both output creation | |
| 559 | 550 | // and connection emission. Keeping that lock across the send prevents a view | |
| 560 | 551 | // publication from overtaking output fenced just before it. | |
| @@ -690,12 +681,3 @@ func (s *attachmentOutput) committedState() uint64 { | |||
| 690 | 681 | defer s.unlockView() | |
| 691 | 682 | return s.next | |
| 692 | 683 | } | |
| 693 | - | ||
| 694 | - | func (s *attachmentOutput) outstanding() uint64 { | |
| 695 | - | if s == nil { | |
| 696 | - | return 0 | |
| 697 | - | } | |
| 698 | - | s.lockView() | |
| 699 | - | defer s.unlockView() | |
| 700 | - | return s.next - s.acked | |
| 701 | - | } | |
| @@ -546,15 +546,6 @@ func marshalOutputState(data []byte, epoch, base, next, echoAck, viewRevision ui | |||||
| 546 | return output, nil | 546 | return output, nil | ||
| 547 | } | 547 | } | ||
| 548 | 548 | ||||
| 549 | - | // sideEffect builds output without advancing the state stream. When an | |||
| 550 | - | // attachment is present, callers must hold its sendMu; attachment-bound sends | |||
| 551 | - | // use sideEffectLocked after validating their connection incarnation. | |||
| 552 | - | func (s *attachmentOutput) sideEffect(data []byte, echoAck uint64) (protocol.Output, error) { | |||
| 553 | - | s.lockView() | |||
| 554 | - | defer s.unlockView() | |||
| 555 | - | return s.sideEffectLocked(data, echoAck) | |||
| 556 | - | } | |||
| 557 | - | ||||
| 558 | // sideEffectLocked requires the attachment view lock for both output creation | 549 | // sideEffectLocked requires the attachment view lock for both output creation | ||
| 559 | // and connection emission. Keeping that lock across the send prevents a view | 550 | // and connection emission. Keeping that lock across the send prevents a view | ||
| 560 | // publication from overtaking output fenced just before it. | 551 | // publication from overtaking output fenced just before it. | ||
| @@ -690,12 +681,3 @@ func (s *attachmentOutput) committedState() uint64 { | |||||
| 690 | defer s.unlockView() | 681 | defer s.unlockView() | ||
| 691 | return s.next | 682 | return s.next | ||
| 692 | } | 683 | } | ||
| 693 | - | ||||
| 694 | - | func (s *attachmentOutput) outstanding() uint64 { | |||
| 695 | - | if s == nil { | |||
| 696 | - | return 0 | |||
| 697 | - | } | |||
| 698 | - | s.lockView() | |||
| 699 | - | defer s.unlockView() | |||
| 700 | - | return s.next - s.acked | |||
| 701 | - | } | |||
M internal/usecase/daemon/client.go +0 -7
| @@ -308,13 +308,6 @@ func (ac *attachedClient) getClientTheme() themeui.Theme { | |||
| 308 | 308 | return ac.clientTheme | |
| 309 | 309 | } | |
| 310 | 310 | ||
| 311 | - | // setThemeForTest publishes a complete applied snapshot for tests that do | |
| 312 | - | // not exercise daemon configuration. Production theme changes use | |
| 313 | - | // applyHostThemeLocked, which supplies the active policy. | |
| 314 | - | func (ac *attachedClient) setThemeForTest(t themeui.Theme) { | |
| 315 | - | ac.setAppliedTheme(appliedTheme{Raw: t, Resolved: themeui.Resolve(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | |
| 316 | - | } | |
| 317 | - | ||
| 318 | 311 | func (ac *attachedClient) setAppliedTheme(next appliedTheme) { | |
| 319 | 312 | ac.themeMu.Lock() | |
| 320 | 313 | next.Generation = ac.appliedTheme.Generation + 1 | |
| @@ -308,13 +308,6 @@ func (ac *attachedClient) getClientTheme() themeui.Theme { | |||||
| 308 | return ac.clientTheme | 308 | return ac.clientTheme | ||
| 309 | } | 309 | } | ||
| 310 | 310 | ||||
| 311 | - | // setThemeForTest publishes a complete applied snapshot for tests that do | |||
| 312 | - | // not exercise daemon configuration. Production theme changes use | |||
| 313 | - | // applyHostThemeLocked, which supplies the active policy. | |||
| 314 | - | func (ac *attachedClient) setThemeForTest(t themeui.Theme) { | |||
| 315 | - | ac.setAppliedTheme(appliedTheme{Raw: t, Resolved: themeui.Resolve(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | |||
| 316 | - | } | |||
| 317 | - | ||||
| 318 | func (ac *attachedClient) setAppliedTheme(next appliedTheme) { | 311 | func (ac *attachedClient) setAppliedTheme(next appliedTheme) { | ||
| 319 | ac.themeMu.Lock() | 312 | ac.themeMu.Lock() | ||
| 320 | next.Generation = ac.appliedTheme.Generation + 1 | 313 | next.Generation = ac.appliedTheme.Generation + 1 | ||
M internal/usecase/daemon/floating_spare_test.go +1 -1
| @@ -264,7 +264,7 @@ func TestFloatingSpareExitBeforeClaimDropsIt(t *testing.T) { | |||
| 264 | 264 | spare := sess.floatingSparePane() | |
| 265 | 265 | require.NotNil(t, spare) | |
| 266 | 266 | require.NoError(t, factory.snapshot()[0].pty.Close(), "the spare's shell exits") | |
| 267 | - | require.Eventually(t, func() bool { return sess.floatingSparePane() == nil }, time.Second, time.Millisecond, | |
| 267 | + | require.Eventually(t, func() bool { return sess.floatingSparePane() == nil }, 5*time.Second, time.Millisecond, | |
| 268 | 268 | "an exited unclaimed spare must be dropped so the next claim launches a fresh shell") | |
| 269 | 269 | ||
| 270 | 270 | sess.stopInMemoryLifecycle() | |
| @@ -264,7 +264,7 @@ func TestFloatingSpareExitBeforeClaimDropsIt(t *testing.T) { | |||||
| 264 | spare := sess.floatingSparePane() | 264 | spare := sess.floatingSparePane() | ||
| 265 | require.NotNil(t, spare) | 265 | require.NotNil(t, spare) | ||
| 266 | require.NoError(t, factory.snapshot()[0].pty.Close(), "the spare's shell exits") | 266 | require.NoError(t, factory.snapshot()[0].pty.Close(), "the spare's shell exits") | ||
| 267 | - | require.Eventually(t, func() bool { return sess.floatingSparePane() == nil }, time.Second, time.Millisecond, | 267 | + | require.Eventually(t, func() bool { return sess.floatingSparePane() == nil }, 5*time.Second, time.Millisecond, |
| 268 | "an exited unclaimed spare must be dropped so the next claim launches a fresh shell") | 268 | "an exited unclaimed spare must be dropped so the next claim launches a fresh shell") | ||
| 269 | 269 | ||||
| 270 | sess.stopInMemoryLifecycle() | 270 | sess.stopInMemoryLifecycle() | ||
A internal/usecase/daemon/production_seams_helpers_test.go +46 -0
| @@ -0,0 +1,46 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "github.com/bnema/vev/internal/domain" | |
| 5 | + | "github.com/bnema/vev/internal/protocol" | |
| 6 | + | themeui "github.com/bnema/vev/internal/usecase/theme" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | // ptyReader runs one pane reader for tests that register it with sessWg.Add | |
| 10 | + | // first. Production launches readPanePTY through sessWg.Go. | |
| 11 | + | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |
| 12 | + | defer d.sessWg.Done() | |
| 13 | + | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |
| 14 | + | publishPaneOwner(p, sess, tb, 0) | |
| 15 | + | } | |
| 16 | + | d.readPanePTY(p) | |
| 17 | + | } | |
| 18 | + | ||
| 19 | + | // processPTYData feeds bytes through the pane's VT parsing path. | |
| 20 | + | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |
| 21 | + | d.processPanePTYData(p, data, bufferDuringApply) | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | // setThemeForTest publishes a complete applied snapshot for tests that do not | |
| 25 | + | // exercise daemon configuration. | |
| 26 | + | func (ac *attachedClient) setThemeForTest(t themeui.Theme) { | |
| 27 | + | ac.setAppliedTheme(appliedTheme{Raw: t, Resolved: themeui.Resolve(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | // sideEffect builds output without advancing the state stream, under the view | |
| 31 | + | // lock. | |
| 32 | + | func (s *attachmentOutput) sideEffect(data []byte, echoAck uint64) (protocol.Output, error) { | |
| 33 | + | s.lockView() | |
| 34 | + | defer s.unlockView() | |
| 35 | + | return s.sideEffectLocked(data, echoAck) | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // outstanding reports state frames sent but not yet acknowledged. | |
| 39 | + | func (s *attachmentOutput) outstanding() uint64 { | |
| 40 | + | if s == nil { | |
| 41 | + | return 0 | |
| 42 | + | } | |
| 43 | + | s.lockView() | |
| 44 | + | defer s.unlockView() | |
| 45 | + | return s.next - s.acked | |
| 46 | + | } | |
| @@ -0,0 +1,46 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "github.com/bnema/vev/internal/domain" | |||
| 5 | + | "github.com/bnema/vev/internal/protocol" | |||
| 6 | + | themeui "github.com/bnema/vev/internal/usecase/theme" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | // ptyReader runs one pane reader for tests that register it with sessWg.Add | |||
| 10 | + | // first. Production launches readPanePTY through sessWg.Go. | |||
| 11 | + | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |||
| 12 | + | defer d.sessWg.Done() | |||
| 13 | + | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |||
| 14 | + | publishPaneOwner(p, sess, tb, 0) | |||
| 15 | + | } | |||
| 16 | + | d.readPanePTY(p) | |||
| 17 | + | } | |||
| 18 | + | ||||
| 19 | + | // processPTYData feeds bytes through the pane's VT parsing path. | |||
| 20 | + | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |||
| 21 | + | d.processPanePTYData(p, data, bufferDuringApply) | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | // setThemeForTest publishes a complete applied snapshot for tests that do not | |||
| 25 | + | // exercise daemon configuration. | |||
| 26 | + | func (ac *attachedClient) setThemeForTest(t themeui.Theme) { | |||
| 27 | + | ac.setAppliedTheme(appliedTheme{Raw: t, Resolved: themeui.Resolve(t, domain.ThemeAccent{Mode: domain.ThemeAccentAuto})}) | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | // sideEffect builds output without advancing the state stream, under the view | |||
| 31 | + | // lock. | |||
| 32 | + | func (s *attachmentOutput) sideEffect(data []byte, echoAck uint64) (protocol.Output, error) { | |||
| 33 | + | s.lockView() | |||
| 34 | + | defer s.unlockView() | |||
| 35 | + | return s.sideEffectLocked(data, echoAck) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // outstanding reports state frames sent but not yet acknowledged. | |||
| 39 | + | func (s *attachmentOutput) outstanding() uint64 { | |||
| 40 | + | if s == nil { | |||
| 41 | + | return 0 | |||
| 42 | + | } | |||
| 43 | + | s.lockView() | |||
| 44 | + | defer s.unlockView() | |||
| 45 | + | return s.next - s.acked | |||
| 46 | + | } | |||
M internal/usecase/daemon/render.go +0 -18
| @@ -56,18 +56,6 @@ func (d *Daemon) paneRenderable(sess *session, tb *tab, p *pane) bool { | |||
| 56 | 56 | return false | |
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | - | // ptyReader is retained as a compatibility entry point for focused tests, which | |
| 60 | - | // register the goroutine with sessWg.Add before calling it. The production | |
| 61 | - | // goroutine is launched with sessWg.Go and calls readPanePTY, whose closure owns | |
| 62 | - | // only the pane. | |
| 63 | - | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |
| 64 | - | defer d.sessWg.Done() | |
| 65 | - | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |
| 66 | - | publishPaneOwner(p, sess, tb, 0) | |
| 67 | - | } | |
| 68 | - | d.readPanePTY(p) | |
| 69 | - | } | |
| 70 | - | ||
| 71 | 59 | func (d *Daemon) readPanePTY(p *pane) { | |
| 72 | 60 | if p == nil { | |
| 73 | 61 | return | |
| @@ -115,12 +103,6 @@ type panePTYEffects struct { | |||
| 115 | 103 | syncCleanup syncTimerCleanup | |
| 116 | 104 | } | |
| 117 | 105 | ||
| 118 | - | // processPTYData retains the old call shape for resize replay and focused tests, | |
| 119 | - | // but routing is always derived from the pane owner captured under pane.mu. | |
| 120 | - | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |
| 121 | - | d.processPanePTYData(p, data, bufferDuringApply) | |
| 122 | - | } | |
| 123 | - | ||
| 124 | 106 | // processPanePTYData is the sole VT parsing path. While a resize apply owns the | |
| 125 | 107 | // pane, reader calls append exact bytes and returns immediately; it never waits | |
| 126 | 108 | // for PTY.Resize or loses a short read. | |
| @@ -56,18 +56,6 @@ func (d *Daemon) paneRenderable(sess *session, tb *tab, p *pane) bool { | |||||
| 56 | return false | 56 | return false | ||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | - | // ptyReader is retained as a compatibility entry point for focused tests, which | |||
| 60 | - | // register the goroutine with sessWg.Add before calling it. The production | |||
| 61 | - | // goroutine is launched with sessWg.Go and calls readPanePTY, whose closure owns | |||
| 62 | - | // only the pane. | |||
| 63 | - | func (d *Daemon) ptyReader(sess *session, tb *tab, p *pane) { | |||
| 64 | - | defer d.sessWg.Done() | |||
| 65 | - | if p != nil && p.ownerSnapshot() == nil && sess != nil && tb != nil { | |||
| 66 | - | publishPaneOwner(p, sess, tb, 0) | |||
| 67 | - | } | |||
| 68 | - | d.readPanePTY(p) | |||
| 69 | - | } | |||
| 70 | - | ||||
| 71 | func (d *Daemon) readPanePTY(p *pane) { | 59 | func (d *Daemon) readPanePTY(p *pane) { | ||
| 72 | if p == nil { | 60 | if p == nil { | ||
| 73 | return | 61 | return | ||
| @@ -115,12 +103,6 @@ type panePTYEffects struct { | |||||
| 115 | syncCleanup syncTimerCleanup | 103 | syncCleanup syncTimerCleanup | ||
| 116 | } | 104 | } | ||
| 117 | 105 | ||||
| 118 | - | // processPTYData retains the old call shape for resize replay and focused tests, | |||
| 119 | - | // but routing is always derived from the pane owner captured under pane.mu. | |||
| 120 | - | func (d *Daemon) processPTYData(_ *session, _ *tab, p *pane, data []byte, bufferDuringApply bool) { | |||
| 121 | - | d.processPanePTYData(p, data, bufferDuringApply) | |||
| 122 | - | } | |||
| 123 | - | ||||
| 124 | // processPanePTYData is the sole VT parsing path. While a resize apply owns the | 106 | // processPanePTYData is the sole VT parsing path. While a resize apply owns the | ||
| 125 | // pane, reader calls append exact bytes and returns immediately; it never waits | 107 | // pane, reader calls append exact bytes and returns immediately; it never waits | ||
| 126 | // for PTY.Resize or loses a short read. | 108 | // for PTY.Resize or loses a short read. | ||
M internal/usecase/daemon/transactional_resize.go +1 -1
| @@ -421,7 +421,7 @@ func (d *Daemon) replayResizePending(sess *session, tb *tab, p *pane, resized bo | |||
| 421 | 421 | p.mu.Unlock() | |
| 422 | 422 | continue | |
| 423 | 423 | } | |
| 424 | - | d.processPTYData(sess, tb, p, data, false) | |
| 424 | + | d.processPanePTYData(p, data, false) | |
| 425 | 425 | } | |
| 426 | 426 | } | |
| 427 | 427 | ||
| @@ -421,7 +421,7 @@ func (d *Daemon) replayResizePending(sess *session, tb *tab, p *pane, resized bo | |||||
| 421 | p.mu.Unlock() | 421 | p.mu.Unlock() | ||
| 422 | continue | 422 | continue | ||
| 423 | } | 423 | } | ||
| 424 | - | d.processPTYData(sess, tb, p, data, false) | 424 | + | d.processPanePTYData(p, data, false) |
| 425 | } | 425 | } | ||
| 426 | } | 426 | } | ||
| 427 | 427 | ||||