test(notifications): address final review findings
- commit
- eb9f4eb82efd2c41c515b941ac759cb75975593f
- author
- brice · · 2026-07-21 11:31 UTC
- parent
- 156e0f0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +107 -55
M internal/ports/wire_test.go +1 -1
| @@ -690,7 +690,7 @@ func TestMsgTypeConstantsDistinct(t *testing.T) { | |||
| 690 | 690 | ReasonReplaced: "ReasonReplaced", | |
| 691 | 691 | } | |
| 692 | 692 | if len(vals) != 4 { | |
| 693 | - | t.Fatalf("expected 3 distinct detached reasons, got %d", len(vals)) | |
| 693 | + | t.Fatalf("expected 4 distinct detached reasons, got %d", len(vals)) | |
| 694 | 694 | } | |
| 695 | 695 | }) | |
| 696 | 696 | } | |
| @@ -690,7 +690,7 @@ func TestMsgTypeConstantsDistinct(t *testing.T) { | |||||
| 690 | ReasonReplaced: "ReasonReplaced", | 690 | ReasonReplaced: "ReasonReplaced", | ||
| 691 | } | 691 | } | ||
| 692 | if len(vals) != 4 { | 692 | if len(vals) != 4 { | ||
| 693 | - | t.Fatalf("expected 3 distinct detached reasons, got %d", len(vals)) | 693 | + | t.Fatalf("expected 4 distinct detached reasons, got %d", len(vals)) |
| 694 | } | 694 | } | ||
| 695 | }) | 695 | }) | ||
| 696 | } | 696 | } | ||
M internal/usecase/client/reconnect_toast_test.go +18 -30
| @@ -141,34 +141,33 @@ func newReconnectToastRecordingTransport(recvs ...reconnectToastRecv) *reconnect | |||
| 141 | 141 | } | |
| 142 | 142 | ||
| 143 | 143 | type reconnectToastLinkTransport struct { | |
| 144 | - | mu sync.Mutex | |
| 145 | - | recvCh chan reconnectToastRecv | |
| 146 | - | sends []ports.Frame | |
| 147 | - | events chan ports.LinkEvent | |
| 148 | - | state ports.LinkState | |
| 144 | + | recvCh chan reconnectToastRecv | |
| 145 | + | clientNotices chan ports.ClientNotice | |
| 146 | + | events chan ports.LinkEvent | |
| 147 | + | state ports.LinkState | |
| 149 | 148 | } | |
| 150 | 149 | ||
| 151 | 150 | func newReconnectToastLinkTransport() *reconnectToastLinkTransport { | |
| 152 | 151 | return &reconnectToastLinkTransport{ | |
| 153 | - | recvCh: make(chan reconnectToastRecv, 4), | |
| 154 | - | events: make(chan ports.LinkEvent, 4), | |
| 155 | - | state: ports.LinkStateConnected, | |
| 152 | + | recvCh: make(chan reconnectToastRecv, 4), | |
| 153 | + | clientNotices: make(chan ports.ClientNotice, 2), | |
| 154 | + | events: make(chan ports.LinkEvent, 4), | |
| 155 | + | state: ports.LinkStateConnected, | |
| 156 | 156 | } | |
| 157 | 157 | } | |
| 158 | 158 | ||
| 159 | 159 | func (t *reconnectToastLinkTransport) Send(f ports.Frame) error { | |
| 160 | - | t.mu.Lock() | |
| 161 | - | t.sends = append(t.sends, f) | |
| 162 | - | t.mu.Unlock() | |
| 160 | + | if f.Type != ports.MsgClientNotice { | |
| 161 | + | return nil | |
| 162 | + | } | |
| 163 | + | notice, err := ports.UnmarshalClientNotice(f.Payload) | |
| 164 | + | if err != nil { | |
| 165 | + | return err | |
| 166 | + | } | |
| 167 | + | t.clientNotices <- notice | |
| 163 | 168 | return nil | |
| 164 | 169 | } | |
| 165 | 170 | ||
| 166 | - | func (t *reconnectToastLinkTransport) sentFrames() []ports.Frame { | |
| 167 | - | t.mu.Lock() | |
| 168 | - | defer t.mu.Unlock() | |
| 169 | - | return append([]ports.Frame(nil), t.sends...) | |
| 170 | - | } | |
| 171 | - | ||
| 172 | 171 | func (t *reconnectToastLinkTransport) Recv() (ports.Frame, error) { | |
| 173 | 172 | recv := <-t.recvCh | |
| 174 | 173 | return recv.frame, recv.err | |
| @@ -305,19 +304,8 @@ func TestReconnectLinkEventsNotifyDaemonWithoutLocalTerminalWrites(t *testing.T) | |||
| 305 | 304 | ||
| 306 | 305 | tr.events <- ports.LinkEvent{State: ports.LinkStateDegraded} | |
| 307 | 306 | tr.events <- ports.LinkEvent{State: ports.LinkStateConnected} | |
| 308 | - | require.Eventually(t, func() bool { | |
| 309 | - | var actions []uint8 | |
| 310 | - | for _, frame := range tr.sentFrames() { | |
| 311 | - | if frame.Type != ports.MsgClientNotice { | |
| 312 | - | continue | |
| 313 | - | } | |
| 314 | - | notice, err := ports.UnmarshalClientNotice(frame.Payload) | |
| 315 | - | if err == nil { | |
| 316 | - | actions = append(actions, notice.Action) | |
| 317 | - | } | |
| 318 | - | } | |
| 319 | - | return len(actions) == 2 && actions[0] == ports.ClientNoticeLinkDegraded && actions[1] == ports.ClientNoticeLinkConnected | |
| 320 | - | }, time.Second, time.Millisecond) | |
| 307 | + | require.Equal(t, ports.ClientNoticeLinkDegraded, (<-tr.clientNotices).Action) | |
| 308 | + | require.Equal(t, ports.ClientNoticeLinkConnected, (<-tr.clientNotices).Action) | |
| 321 | 309 | select { | |
| 322 | 310 | case bytes := <-out.completed: | |
| 323 | 311 | t.Fatalf("link notices wrote client-local terminal bytes: %q", bytes) | |
| @@ -141,34 +141,33 @@ func newReconnectToastRecordingTransport(recvs ...reconnectToastRecv) *reconnect | |||||
| 141 | } | 141 | } | ||
| 142 | 142 | ||||
| 143 | type reconnectToastLinkTransport struct { | 143 | type reconnectToastLinkTransport struct { | ||
| 144 | - | mu sync.Mutex | 144 | + | recvCh chan reconnectToastRecv |
| 145 | - | recvCh chan reconnectToastRecv | 145 | + | clientNotices chan ports.ClientNotice |
| 146 | - | sends []ports.Frame | 146 | + | events chan ports.LinkEvent |
| 147 | - | events chan ports.LinkEvent | 147 | + | state ports.LinkState |
| 148 | - | state ports.LinkState | |||
| 149 | } | 148 | } | ||
| 150 | 149 | ||||
| 151 | func newReconnectToastLinkTransport() *reconnectToastLinkTransport { | 150 | func newReconnectToastLinkTransport() *reconnectToastLinkTransport { | ||
| 152 | return &reconnectToastLinkTransport{ | 151 | return &reconnectToastLinkTransport{ | ||
| 153 | - | recvCh: make(chan reconnectToastRecv, 4), | 152 | + | recvCh: make(chan reconnectToastRecv, 4), |
| 154 | - | events: make(chan ports.LinkEvent, 4), | 153 | + | clientNotices: make(chan ports.ClientNotice, 2), |
| 155 | - | state: ports.LinkStateConnected, | 154 | + | events: make(chan ports.LinkEvent, 4), |
| 155 | + | state: ports.LinkStateConnected, | |||
| 156 | } | 156 | } | ||
| 157 | } | 157 | } | ||
| 158 | 158 | ||||
| 159 | func (t *reconnectToastLinkTransport) Send(f ports.Frame) error { | 159 | func (t *reconnectToastLinkTransport) Send(f ports.Frame) error { | ||
| 160 | - | t.mu.Lock() | 160 | + | if f.Type != ports.MsgClientNotice { |
| 161 | - | t.sends = append(t.sends, f) | 161 | + | return nil |
| 162 | - | t.mu.Unlock() | 162 | + | } |
| 163 | + | notice, err := ports.UnmarshalClientNotice(f.Payload) | |||
| 164 | + | if err != nil { | |||
| 165 | + | return err | |||
| 166 | + | } | |||
| 167 | + | t.clientNotices <- notice | |||
| 163 | return nil | 168 | return nil | ||
| 164 | } | 169 | } | ||
| 165 | 170 | ||||
| 166 | - | func (t *reconnectToastLinkTransport) sentFrames() []ports.Frame { | |||
| 167 | - | t.mu.Lock() | |||
| 168 | - | defer t.mu.Unlock() | |||
| 169 | - | return append([]ports.Frame(nil), t.sends...) | |||
| 170 | - | } | |||
| 171 | - | ||||
| 172 | func (t *reconnectToastLinkTransport) Recv() (ports.Frame, error) { | 171 | func (t *reconnectToastLinkTransport) Recv() (ports.Frame, error) { | ||
| 173 | recv := <-t.recvCh | 172 | recv := <-t.recvCh | ||
| 174 | return recv.frame, recv.err | 173 | return recv.frame, recv.err | ||
| @@ -305,19 +304,8 @@ func TestReconnectLinkEventsNotifyDaemonWithoutLocalTerminalWrites(t *testing.T) | |||||
| 305 | 304 | ||||
| 306 | tr.events <- ports.LinkEvent{State: ports.LinkStateDegraded} | 305 | tr.events <- ports.LinkEvent{State: ports.LinkStateDegraded} | ||
| 307 | tr.events <- ports.LinkEvent{State: ports.LinkStateConnected} | 306 | tr.events <- ports.LinkEvent{State: ports.LinkStateConnected} | ||
| 308 | - | require.Eventually(t, func() bool { | 307 | + | require.Equal(t, ports.ClientNoticeLinkDegraded, (<-tr.clientNotices).Action) |
| 309 | - | var actions []uint8 | 308 | + | require.Equal(t, ports.ClientNoticeLinkConnected, (<-tr.clientNotices).Action) |
| 310 | - | for _, frame := range tr.sentFrames() { | |||
| 311 | - | if frame.Type != ports.MsgClientNotice { | |||
| 312 | - | continue | |||
| 313 | - | } | |||
| 314 | - | notice, err := ports.UnmarshalClientNotice(frame.Payload) | |||
| 315 | - | if err == nil { | |||
| 316 | - | actions = append(actions, notice.Action) | |||
| 317 | - | } | |||
| 318 | - | } | |||
| 319 | - | return len(actions) == 2 && actions[0] == ports.ClientNoticeLinkDegraded && actions[1] == ports.ClientNoticeLinkConnected | |||
| 320 | - | }, time.Second, time.Millisecond) | |||
| 321 | select { | 309 | select { | ||
| 322 | case bytes := <-out.completed: | 310 | case bytes := <-out.completed: | ||
| 323 | t.Fatalf("link notices wrote client-local terminal bytes: %q", bytes) | 311 | t.Fatalf("link notices wrote client-local terminal bytes: %q", bytes) | ||
M internal/usecase/daemon/config.go +9 -3
| @@ -77,13 +77,19 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||
| 77 | 77 | d.repaintAllAttachedClients() | |
| 78 | 78 | ||
| 79 | 79 | if len(allWarnings) > 0 { | |
| 80 | - | first := allWarnings[0] | |
| 81 | 80 | d.notify(nil, domain.NoticeWarn, domain.NoticeConfigReload, | |
| 82 | - | fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(allWarnings), first.Line, first.Msg), | |
| 83 | - | configWarningsError(allWarnings)) | |
| 81 | + | configReloadNoticeMessage(allWarnings), configWarningsError(allWarnings)) | |
| 84 | 82 | } | |
| 85 | 83 | } | |
| 86 | 84 | ||
| 85 | + | func configReloadNoticeMessage(warnings []domain.Warning) string { | |
| 86 | + | first := warnings[0] | |
| 87 | + | if first.Line > 0 { | |
| 88 | + | return fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(warnings), first.Line, first.Msg) | |
| 89 | + | } | |
| 90 | + | return fmt.Sprintf("config reloaded with %d warning(s): %s", len(warnings), first.Msg) | |
| 91 | + | } | |
| 92 | + | ||
| 87 | 93 | // configWarningsError joins every config warning into a single error whose | |
| 88 | 94 | // message lists them all, so noticeDetails (which renders a cause's own | |
| 89 | 95 | // Error() string) surfaces every warning line in one notice's Details. | |
| @@ -77,13 +77,19 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||||
| 77 | d.repaintAllAttachedClients() | 77 | d.repaintAllAttachedClients() | ||
| 78 | 78 | ||||
| 79 | if len(allWarnings) > 0 { | 79 | if len(allWarnings) > 0 { | ||
| 80 | - | first := allWarnings[0] | |||
| 81 | d.notify(nil, domain.NoticeWarn, domain.NoticeConfigReload, | 80 | d.notify(nil, domain.NoticeWarn, domain.NoticeConfigReload, | ||
| 82 | - | fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(allWarnings), first.Line, first.Msg), | 81 | + | configReloadNoticeMessage(allWarnings), configWarningsError(allWarnings)) |
| 83 | - | configWarningsError(allWarnings)) | |||
| 84 | } | 82 | } | ||
| 85 | } | 83 | } | ||
| 86 | 84 | ||||
| 85 | + | func configReloadNoticeMessage(warnings []domain.Warning) string { | |||
| 86 | + | first := warnings[0] | |||
| 87 | + | if first.Line > 0 { | |||
| 88 | + | return fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(warnings), first.Line, first.Msg) | |||
| 89 | + | } | |||
| 90 | + | return fmt.Sprintf("config reloaded with %d warning(s): %s", len(warnings), first.Msg) | |||
| 91 | + | } | |||
| 92 | + | ||||
| 87 | // configWarningsError joins every config warning into a single error whose | 93 | // configWarningsError joins every config warning into a single error whose | ||
| 88 | // message lists them all, so noticeDetails (which renders a cause's own | 94 | // message lists them all, so noticeDetails (which renders a cause's own | ||
| 89 | // Error() string) surfaces every warning line in one notice's Details. | 95 | // Error() string) surfaces every warning line in one notice's Details. | ||
M internal/usecase/daemon/config_test.go +25 -0
| @@ -560,6 +560,30 @@ func TestApplyConfigInvalidDefaultsWithoutPanic(t *testing.T) { | |||
| 560 | 560 | require.False(t, ok) | |
| 561 | 561 | } | |
| 562 | 562 | ||
| 563 | + | func TestConfigReloadNoticeMessageIncludesLineOnlyWhenKnown(t *testing.T) { | |
| 564 | + | tests := []struct { | |
| 565 | + | name string | |
| 566 | + | warnings []domain.Warning | |
| 567 | + | want string | |
| 568 | + | }{ | |
| 569 | + | { | |
| 570 | + | name: "no source line", | |
| 571 | + | warnings: []domain.Warning{{Msg: "unknown action"}}, | |
| 572 | + | want: "config reloaded with 1 warning(s): unknown action", | |
| 573 | + | }, | |
| 574 | + | { | |
| 575 | + | name: "source line", | |
| 576 | + | warnings: []domain.Warning{{Line: 12, Msg: "unknown action"}}, | |
| 577 | + | want: "config reloaded with 1 warning(s): line 12: unknown action", | |
| 578 | + | }, | |
| 579 | + | } | |
| 580 | + | for _, tt := range tests { | |
| 581 | + | t.Run(tt.name, func(t *testing.T) { | |
| 582 | + | require.Equal(t, tt.want, configReloadNoticeMessage(tt.warnings)) | |
| 583 | + | }) | |
| 584 | + | } | |
| 585 | + | } | |
| 586 | + | ||
| 563 | 587 | func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | |
| 564 | 588 | d := newTestDaemon(t, nil, stubClock{}) | |
| 565 | 589 | d.ApplyConfig(domain.Config{ | |
| @@ -586,6 +610,7 @@ func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | |||
| 586 | 610 | require.Equal(t, domain.NoticeWarn, n.Severity) | |
| 587 | 611 | require.Equal(t, domain.SessionID(""), n.SessionID, "config reload notice must be global") | |
| 588 | 612 | require.Contains(t, n.Message, "config reloaded with 3 warning(s):") | |
| 613 | + | require.NotContains(t, n.Message, "line 0:") | |
| 589 | 614 | require.Contains(t, n.Details, `unknown action "unknown-action"`) | |
| 590 | 615 | require.Contains(t, n.Details, `unknown command code slug "missing-command"`) | |
| 591 | 616 | require.Contains(t, n.Details, `invalid command code for "new-tab"`) | |
| @@ -560,6 +560,30 @@ func TestApplyConfigInvalidDefaultsWithoutPanic(t *testing.T) { | |||||
| 560 | require.False(t, ok) | 560 | require.False(t, ok) | ||
| 561 | } | 561 | } | ||
| 562 | 562 | ||||
| 563 | + | func TestConfigReloadNoticeMessageIncludesLineOnlyWhenKnown(t *testing.T) { | |||
| 564 | + | tests := []struct { | |||
| 565 | + | name string | |||
| 566 | + | warnings []domain.Warning | |||
| 567 | + | want string | |||
| 568 | + | }{ | |||
| 569 | + | { | |||
| 570 | + | name: "no source line", | |||
| 571 | + | warnings: []domain.Warning{{Msg: "unknown action"}}, | |||
| 572 | + | want: "config reloaded with 1 warning(s): unknown action", | |||
| 573 | + | }, | |||
| 574 | + | { | |||
| 575 | + | name: "source line", | |||
| 576 | + | warnings: []domain.Warning{{Line: 12, Msg: "unknown action"}}, | |||
| 577 | + | want: "config reloaded with 1 warning(s): line 12: unknown action", | |||
| 578 | + | }, | |||
| 579 | + | } | |||
| 580 | + | for _, tt := range tests { | |||
| 581 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 582 | + | require.Equal(t, tt.want, configReloadNoticeMessage(tt.warnings)) | |||
| 583 | + | }) | |||
| 584 | + | } | |||
| 585 | + | } | |||
| 586 | + | ||||
| 563 | func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | 587 | func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | ||
| 564 | d := newTestDaemon(t, nil, stubClock{}) | 588 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 565 | d.ApplyConfig(domain.Config{ | 589 | d.ApplyConfig(domain.Config{ | ||
| @@ -586,6 +610,7 @@ func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | |||||
| 586 | require.Equal(t, domain.NoticeWarn, n.Severity) | 610 | require.Equal(t, domain.NoticeWarn, n.Severity) | ||
| 587 | require.Equal(t, domain.SessionID(""), n.SessionID, "config reload notice must be global") | 611 | require.Equal(t, domain.SessionID(""), n.SessionID, "config reload notice must be global") | ||
| 588 | require.Contains(t, n.Message, "config reloaded with 3 warning(s):") | 612 | require.Contains(t, n.Message, "config reloaded with 3 warning(s):") | ||
| 613 | + | require.NotContains(t, n.Message, "line 0:") | |||
| 589 | require.Contains(t, n.Details, `unknown action "unknown-action"`) | 614 | require.Contains(t, n.Details, `unknown action "unknown-action"`) | ||
| 590 | require.Contains(t, n.Details, `unknown command code slug "missing-command"`) | 615 | require.Contains(t, n.Details, `unknown command code slug "missing-command"`) | ||
| 591 | require.Contains(t, n.Details, `invalid command code for "new-tab"`) | 616 | require.Contains(t, n.Details, `invalid command code for "new-tab"`) | ||
M internal/usecase/daemon/notice_persist_test.go +4 -2
| @@ -51,7 +51,8 @@ func TestKillSessionServerShutdownSaturatedAppendsNotice(t *testing.T) { | |||
| 51 | 51 | } | |
| 52 | 52 | ||
| 53 | 53 | target := newSnapshotTestSession(t, "target", false, "/work") | |
| 54 | - | targetPTY := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | |
| 54 | + | targetPTY, ok := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | |
| 55 | + | require.True(t, ok, "target pane PTY must be a MockPTY") | |
| 55 | 56 | targetPTY.EXPECT().Close().Return(nil).Maybe() | |
| 56 | 57 | d.mu.Lock() | |
| 57 | 58 | d.sessions[target.id] = target | |
| @@ -92,7 +93,8 @@ func TestKillSessionServerShutdownNotSaturatedSkipsAppend(t *testing.T) { | |||
| 92 | 93 | d.noticeStore = notices | |
| 93 | 94 | ||
| 94 | 95 | target := newSnapshotTestSession(t, "target", false, "/work") | |
| 95 | - | targetPTY := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | |
| 96 | + | targetPTY, ok := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | |
| 97 | + | require.True(t, ok, "target pane PTY must be a MockPTY") | |
| 96 | 98 | targetPTY.EXPECT().Close().Return(nil).Maybe() | |
| 97 | 99 | d.mu.Lock() | |
| 98 | 100 | d.sessions[target.id] = target | |
| @@ -51,7 +51,8 @@ func TestKillSessionServerShutdownSaturatedAppendsNotice(t *testing.T) { | |||||
| 51 | } | 51 | } | ||
| 52 | 52 | ||||
| 53 | target := newSnapshotTestSession(t, "target", false, "/work") | 53 | target := newSnapshotTestSession(t, "target", false, "/work") | ||
| 54 | - | targetPTY := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | 54 | + | targetPTY, ok := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) |
| 55 | + | require.True(t, ok, "target pane PTY must be a MockPTY") | |||
| 55 | targetPTY.EXPECT().Close().Return(nil).Maybe() | 56 | targetPTY.EXPECT().Close().Return(nil).Maybe() | ||
| 56 | d.mu.Lock() | 57 | d.mu.Lock() | ||
| 57 | d.sessions[target.id] = target | 58 | d.sessions[target.id] = target | ||
| @@ -92,7 +93,8 @@ func TestKillSessionServerShutdownNotSaturatedSkipsAppend(t *testing.T) { | |||||
| 92 | d.noticeStore = notices | 93 | d.noticeStore = notices | ||
| 93 | 94 | ||||
| 94 | target := newSnapshotTestSession(t, "target", false, "/work") | 95 | target := newSnapshotTestSession(t, "target", false, "/work") | ||
| 95 | - | targetPTY := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) | 96 | + | targetPTY, ok := target.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY) |
| 97 | + | require.True(t, ok, "target pane PTY must be a MockPTY") | |||
| 96 | targetPTY.EXPECT().Close().Return(nil).Maybe() | 98 | targetPTY.EXPECT().Close().Return(nil).Maybe() | ||
| 97 | d.mu.Lock() | 99 | d.mu.Lock() | ||
| 98 | d.sessions[target.id] = target | 100 | d.sessions[target.id] = target | ||
M internal/usecase/daemon/notice_render_test.go +1 -1
| @@ -92,7 +92,7 @@ func TestPaintComposesNoticeToastTopRightAndExpiresOnTTL(t *testing.T) { | |||
| 92 | 92 | t.Fatalf("composed frame missing toast message top-right:\n%s", strings.Join(rows, "\n")) | |
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | - | clk.advance(8 * time.Second) // error-severity TTL, per noticeTTL | |
| 95 | + | clk.advance(noticeTTL(domain.NoticeError)) | |
| 96 | 96 | awaitToastCount(t, ac, 0) | |
| 97 | 97 | select { | |
| 98 | 98 | case f := <-sends: | |
| @@ -92,7 +92,7 @@ func TestPaintComposesNoticeToastTopRightAndExpiresOnTTL(t *testing.T) { | |||||
| 92 | t.Fatalf("composed frame missing toast message top-right:\n%s", strings.Join(rows, "\n")) | 92 | t.Fatalf("composed frame missing toast message top-right:\n%s", strings.Join(rows, "\n")) | ||
| 93 | } | 93 | } | ||
| 94 | 94 | ||||
| 95 | - | clk.advance(8 * time.Second) // error-severity TTL, per noticeTTL | 95 | + | clk.advance(noticeTTL(domain.NoticeError)) |
| 96 | awaitToastCount(t, ac, 0) | 96 | awaitToastCount(t, ac, 0) | ||
| 97 | select { | 97 | select { | ||
| 98 | case f := <-sends: | 98 | case f := <-sends: | ||
M internal/usecase/daemon/notify_test.go +2 -1
| @@ -42,7 +42,8 @@ func TestClientNoticeMapsFixedActionsAndDismissesOnlyConnectionToast(t *testing. | |||
| 42 | 42 | ||
| 43 | 43 | func TestMalformedClientNoticeIsIgnored(t *testing.T) { | |
| 44 | 44 | d, _, ac, _ := newNoticeFixture(t, ¬iceClock{}) | |
| 45 | - | tr := ac.tr.(*portsmocks.MockTransport) | |
| 45 | + | tr, ok := ac.tr.(*portsmocks.MockTransport) | |
| 46 | + | require.True(t, ok, "attached client transport must be a MockTransport") | |
| 46 | 47 | frames := []ports.Frame{ | |
| 47 | 48 | {Type: ports.MsgClientNotice, Payload: []byte{0xff}}, | |
| 48 | 49 | {Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}, | |
| @@ -42,7 +42,8 @@ func TestClientNoticeMapsFixedActionsAndDismissesOnlyConnectionToast(t *testing. | |||||
| 42 | 42 | ||||
| 43 | func TestMalformedClientNoticeIsIgnored(t *testing.T) { | 43 | func TestMalformedClientNoticeIsIgnored(t *testing.T) { | ||
| 44 | d, _, ac, _ := newNoticeFixture(t, ¬iceClock{}) | 44 | d, _, ac, _ := newNoticeFixture(t, ¬iceClock{}) | ||
| 45 | - | tr := ac.tr.(*portsmocks.MockTransport) | 45 | + | tr, ok := ac.tr.(*portsmocks.MockTransport) |
| 46 | + | require.True(t, ok, "attached client transport must be a MockTransport") | |||
| 46 | frames := []ports.Frame{ | 47 | frames := []ports.Frame{ | ||
| 47 | {Type: ports.MsgClientNotice, Payload: []byte{0xff}}, | 48 | {Type: ports.MsgClientNotice, Payload: []byte{0xff}}, | ||
| 48 | {Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}, | 49 | {Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}, | ||
M internal/usecase/daemon/prompt_test.go +41 -16
| @@ -110,22 +110,47 @@ func TestPromptSubmitSessionSpawnFailureReportsOneSafeNotice(t *testing.T) { | |||
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | 112 | func TestPromptSubmitValidationErrorStaysInlineOnly(t *testing.T) { | |
| 113 | - | p, release := newBlockingPTY(t) | |
| 114 | - | defer release() | |
| 115 | - | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 116 | - | ||
| 117 | - | d.enterPrompt(sess, ac, " Create session ", "", func(name string) error { | |
| 118 | - | return d.createSessionAndSwitch(sess, ac, name) | |
| 119 | - | }) | |
| 120 | - | awaitFrame(t, sends, ports.MsgOutput) | |
| 121 | - | d.handlePromptInput(ac, []byte("invalid name\r")) | |
| 122 | - | repaint := awaitFrame(t, sends, ports.MsgOutput) | |
| 123 | - | output, err := ports.UnmarshalOutput(repaint.Payload) | |
| 124 | - | require.NoError(t, err) | |
| 125 | - | require.Contains(t, string(output.Data), domain.ErrInvalidSessionName.Error()) | |
| 126 | - | ||
| 127 | - | require.True(t, ac.overlays.promptActive()) | |
| 128 | - | require.Empty(t, d.notices.history()) | |
| 113 | + | tests := []struct { | |
| 114 | + | name string | |
| 115 | + | input []byte | |
| 116 | + | wantErr error | |
| 117 | + | setup func(*Daemon) | |
| 118 | + | }{ | |
| 119 | + | {name: "invalid name", input: []byte("invalid name\r"), wantErr: domain.ErrInvalidSessionName}, | |
| 120 | + | {name: "required name", input: []byte("\r"), wantErr: errSessionNameRequired}, | |
| 121 | + | { | |
| 122 | + | name: "name in use", | |
| 123 | + | input: []byte("taken\r"), | |
| 124 | + | wantErr: errSessionNameInUse, | |
| 125 | + | setup: func(d *Daemon) { | |
| 126 | + | d.sessions["taken"] = &session{id: "taken", name: "taken"} | |
| 127 | + | }, | |
| 128 | + | }, | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | for _, tt := range tests { | |
| 132 | + | t.Run(tt.name, func(t *testing.T) { | |
| 133 | + | p, release := newBlockingPTY(t) | |
| 134 | + | defer release() | |
| 135 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 136 | + | if tt.setup != nil { | |
| 137 | + | tt.setup(d) | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | d.enterPrompt(sess, ac, " Create session ", "", func(name string) error { | |
| 141 | + | return d.createSessionAndSwitch(sess, ac, name) | |
| 142 | + | }) | |
| 143 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 144 | + | d.handlePromptInput(ac, tt.input) | |
| 145 | + | repaint := awaitFrame(t, sends, ports.MsgOutput) | |
| 146 | + | output, err := ports.UnmarshalOutput(repaint.Payload) | |
| 147 | + | require.NoError(t, err) | |
| 148 | + | require.Contains(t, string(output.Data), tt.wantErr.Error()) | |
| 149 | + | ||
| 150 | + | require.True(t, ac.overlays.promptActive()) | |
| 151 | + | require.Empty(t, d.notices.history()) | |
| 152 | + | }) | |
| 153 | + | } | |
| 129 | 154 | } | |
| 130 | 155 | ||
| 131 | 156 | func TestPromptEscapeCancelsWithoutRename(t *testing.T) { | |
| @@ -110,22 +110,47 @@ func TestPromptSubmitSessionSpawnFailureReportsOneSafeNotice(t *testing.T) { | |||||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | func TestPromptSubmitValidationErrorStaysInlineOnly(t *testing.T) { | 112 | func TestPromptSubmitValidationErrorStaysInlineOnly(t *testing.T) { | ||
| 113 | - | p, release := newBlockingPTY(t) | 113 | + | tests := []struct { |
| 114 | - | defer release() | 114 | + | name string |
| 115 | - | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | 115 | + | input []byte |
| 116 | - | 116 | + | wantErr error | |
| 117 | - | d.enterPrompt(sess, ac, " Create session ", "", func(name string) error { | 117 | + | setup func(*Daemon) |
| 118 | - | return d.createSessionAndSwitch(sess, ac, name) | 118 | + | }{ |
| 119 | - | }) | 119 | + | {name: "invalid name", input: []byte("invalid name\r"), wantErr: domain.ErrInvalidSessionName}, |
| 120 | - | awaitFrame(t, sends, ports.MsgOutput) | 120 | + | {name: "required name", input: []byte("\r"), wantErr: errSessionNameRequired}, |
| 121 | - | d.handlePromptInput(ac, []byte("invalid name\r")) | 121 | + | { |
| 122 | - | repaint := awaitFrame(t, sends, ports.MsgOutput) | 122 | + | name: "name in use", |
| 123 | - | output, err := ports.UnmarshalOutput(repaint.Payload) | 123 | + | input: []byte("taken\r"), |
| 124 | - | require.NoError(t, err) | 124 | + | wantErr: errSessionNameInUse, |
| 125 | - | require.Contains(t, string(output.Data), domain.ErrInvalidSessionName.Error()) | 125 | + | setup: func(d *Daemon) { |
| 126 | - | 126 | + | d.sessions["taken"] = &session{id: "taken", name: "taken"} | |
| 127 | - | require.True(t, ac.overlays.promptActive()) | 127 | + | }, |
| 128 | - | require.Empty(t, d.notices.history()) | 128 | + | }, |
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | for _, tt := range tests { | |||
| 132 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 133 | + | p, release := newBlockingPTY(t) | |||
| 134 | + | defer release() | |||
| 135 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |||
| 136 | + | if tt.setup != nil { | |||
| 137 | + | tt.setup(d) | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | d.enterPrompt(sess, ac, " Create session ", "", func(name string) error { | |||
| 141 | + | return d.createSessionAndSwitch(sess, ac, name) | |||
| 142 | + | }) | |||
| 143 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 144 | + | d.handlePromptInput(ac, tt.input) | |||
| 145 | + | repaint := awaitFrame(t, sends, ports.MsgOutput) | |||
| 146 | + | output, err := ports.UnmarshalOutput(repaint.Payload) | |||
| 147 | + | require.NoError(t, err) | |||
| 148 | + | require.Contains(t, string(output.Data), tt.wantErr.Error()) | |||
| 149 | + | ||||
| 150 | + | require.True(t, ac.overlays.promptActive()) | |||
| 151 | + | require.Empty(t, d.notices.history()) | |||
| 152 | + | }) | |||
| 153 | + | } | |||
| 129 | } | 154 | } | ||
| 130 | 155 | ||||
| 131 | func TestPromptEscapeCancelsWithoutRename(t *testing.T) { | 156 | func TestPromptEscapeCancelsWithoutRename(t *testing.T) { | ||
M internal/usecase/daemon/snapshot.go +1 -1
| @@ -62,10 +62,10 @@ func (d *Daemon) closeRestoreDone() { | |||
| 62 | 62 | } | |
| 63 | 63 | ||
| 64 | 64 | func (d *Daemon) restoreSnapshots(ctx context.Context) { | |
| 65 | - | defer d.closeRestoreDone() | |
| 66 | 65 | if d == nil { | |
| 67 | 66 | return | |
| 68 | 67 | } | |
| 68 | + | defer d.closeRestoreDone() | |
| 69 | 69 | if ns := d.noticeStore; ns != nil { | |
| 70 | 70 | claimed, err := ns.Claim() | |
| 71 | 71 | if err != nil { | |
| @@ -62,10 +62,10 @@ func (d *Daemon) closeRestoreDone() { | |||||
| 62 | } | 62 | } | ||
| 63 | 63 | ||||
| 64 | func (d *Daemon) restoreSnapshots(ctx context.Context) { | 64 | func (d *Daemon) restoreSnapshots(ctx context.Context) { | ||
| 65 | - | defer d.closeRestoreDone() | |||
| 66 | if d == nil { | 65 | if d == nil { | ||
| 67 | return | 66 | return | ||
| 68 | } | 67 | } | ||
| 68 | + | defer d.closeRestoreDone() | |||
| 69 | if ns := d.noticeStore; ns != nil { | 69 | if ns := d.noticeStore; ns != nil { | ||
| 70 | claimed, err := ns.Claim() | 70 | claimed, err := ns.Claim() | ||
| 71 | if err != nil { | 71 | if err != nil { | ||
M internal/usecase/daemon/snapshot_restore_test.go +5 -0
| @@ -59,6 +59,11 @@ func TestRestorePaneTerminalRejectsMissingOrMalformedCanonicalBlobs(t *testing.T | |||
| 59 | 59 | } | |
| 60 | 60 | } | |
| 61 | 61 | ||
| 62 | + | func TestRestoreSnapshotsNilDaemonDoesNotPanic(t *testing.T) { | |
| 63 | + | var d *Daemon | |
| 64 | + | require.NotPanics(t, func() { d.restoreSnapshots(t.Context()) }) | |
| 65 | + | } | |
| 66 | + | ||
| 62 | 67 | func TestRestoreSessionRejectsInvalidActiveTab(t *testing.T) { | |
| 63 | 68 | factory := &restorePTYFactory{} | |
| 64 | 69 | d := newTestDaemon(t, factory, stubClock{}) | |
| @@ -59,6 +59,11 @@ func TestRestorePaneTerminalRejectsMissingOrMalformedCanonicalBlobs(t *testing.T | |||||
| 59 | } | 59 | } | ||
| 60 | } | 60 | } | ||
| 61 | 61 | ||||
| 62 | + | func TestRestoreSnapshotsNilDaemonDoesNotPanic(t *testing.T) { | |||
| 63 | + | var d *Daemon | |||
| 64 | + | require.NotPanics(t, func() { d.restoreSnapshots(t.Context()) }) | |||
| 65 | + | } | |||
| 66 | + | ||||
| 62 | func TestRestoreSessionRejectsInvalidActiveTab(t *testing.T) { | 67 | func TestRestoreSessionRejectsInvalidActiveTab(t *testing.T) { | ||
| 63 | factory := &restorePTYFactory{} | 68 | factory := &restorePTYFactory{} | ||
| 64 | d := newTestDaemon(t, factory, stubClock{}) | 69 | d := newTestDaemon(t, factory, stubClock{}) | ||