fix(daemon): align pane rearrangement results
- commit
- 5cc817b963c3dfdffa7b8e0f8cd75386a6852ade
- author
- brice · · 2026-07-27 14:08 UTC
- parent
- ef2479e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +136 -40
M internal/usecase/daemon/palette_test.go +1 -1
| @@ -617,7 +617,7 @@ func TestPaletteRecentCommandsNewestFirstThenRegistryOrder(t *testing.T) { | |||
| 617 | 617 | for i, cmd := range commands { | |
| 618 | 618 | codes[i] = cmd.Code | |
| 619 | 619 | } | |
| 620 | - | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "SPR", "SPL", "SPU", "SPD", "STP", "TST", "FLT", "CLP", "FPL", "FPR", "FPU", "FPD", "RSZ", "GPW", "SPW", "GPH", "SPH", "EQP", "PVT", "BSK", "JRS", "NTC", "YLN", "VIS", "RNS", "RNT", "DET"}, codes) | |
| 620 | + | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "SPR", "SPL", "SPU", "SPD", "CEL", "CER", "STP", "TST", "FLT", "CLP", "FPL", "FPR", "FPU", "FPD", "RSZ", "GPW", "SPW", "GPH", "SPH", "EQP", "PVT", "BSK", "JRS", "NTC", "YLN", "VIS", "RNS", "RNT", "DET"}, codes) | |
| 621 | 621 | } | |
| 622 | 622 | ||
| 623 | 623 | func TestPaletteRecencyCanBeUpdatedConcurrently(t *testing.T) { | |
| @@ -617,7 +617,7 @@ func TestPaletteRecentCommandsNewestFirstThenRegistryOrder(t *testing.T) { | |||||
| 617 | for i, cmd := range commands { | 617 | for i, cmd := range commands { | ||
| 618 | codes[i] = cmd.Code | 618 | codes[i] = cmd.Code | ||
| 619 | } | 619 | } | ||
| 620 | - | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "SPR", "SPL", "SPU", "SPD", "STP", "TST", "FLT", "CLP", "FPL", "FPR", "FPU", "FPD", "RSZ", "GPW", "SPW", "GPH", "SPH", "EQP", "PVT", "BSK", "JRS", "NTC", "YLN", "VIS", "RNS", "RNT", "DET"}, codes) | 620 | + | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "SPR", "SPL", "SPU", "SPD", "CEL", "CER", "STP", "TST", "FLT", "CLP", "FPL", "FPR", "FPU", "FPD", "RSZ", "GPW", "SPW", "GPH", "SPH", "EQP", "PVT", "BSK", "JRS", "NTC", "YLN", "VIS", "RNS", "RNT", "DET"}, codes) |
| 621 | } | 621 | } | ||
| 622 | 622 | ||||
| 623 | func TestPaletteRecencyCanBeUpdatedConcurrently(t *testing.T) { | 623 | func TestPaletteRecencyCanBeUpdatedConcurrently(t *testing.T) { | ||
M internal/usecase/daemon/pane_rearrange.go +2 -6
| @@ -9,10 +9,6 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | var errDaemonActionNoChange = errors.New("daemon action made no change") | |
| 11 | 11 | ||
| 12 | - | // errPaneRearrangeNoop names the pane-rearrange outcome for package-level | |
| 13 | - | // callers while adapters normalize the shared daemon no-change sentinel. | |
| 14 | - | var errPaneRearrangeNoop = errDaemonActionNoChange | |
| 15 | - | ||
| 16 | 12 | func (d *Daemon) consumeOrExpelPane(target daemonActionTarget, direction layout.Direction) error { | |
| 17 | 13 | changed, err := d.mutateTargetLayoutChanged(target, true, func(candidate *layout.Tree, area domain.Rect) (bool, error) { | |
| 18 | 14 | return candidate.ConsumeOrExpelPane(target.pane.id, direction, area) | |
| @@ -29,9 +25,9 @@ func (d *Daemon) consumeOrExpelPane(target daemonActionTarget, direction layout. | |||
| 29 | 25 | func paneRearrangeUserError(err error) error { | |
| 30 | 26 | switch { | |
| 31 | 27 | case errors.Is(err, layout.ErrUnsupportedColumnLayout): | |
| 32 | - | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane cannot be rearranged in this layout", err) | |
| 28 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane rearrangement requires a column layout", err) | |
| 33 | 29 | case errors.Is(err, layout.ErrTooSmall): | |
| 34 | - | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane cannot be rearranged at this size", err) | |
| 30 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to rearrange pane", err) | |
| 35 | 31 | default: | |
| 36 | 32 | return err | |
| 37 | 33 | } | |
| @@ -9,10 +9,6 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | var errDaemonActionNoChange = errors.New("daemon action made no change") | 10 | var errDaemonActionNoChange = errors.New("daemon action made no change") | ||
| 11 | 11 | ||||
| 12 | - | // errPaneRearrangeNoop names the pane-rearrange outcome for package-level | |||
| 13 | - | // callers while adapters normalize the shared daemon no-change sentinel. | |||
| 14 | - | var errPaneRearrangeNoop = errDaemonActionNoChange | |||
| 15 | - | ||||
| 16 | func (d *Daemon) consumeOrExpelPane(target daemonActionTarget, direction layout.Direction) error { | 12 | func (d *Daemon) consumeOrExpelPane(target daemonActionTarget, direction layout.Direction) error { | ||
| 17 | changed, err := d.mutateTargetLayoutChanged(target, true, func(candidate *layout.Tree, area domain.Rect) (bool, error) { | 13 | changed, err := d.mutateTargetLayoutChanged(target, true, func(candidate *layout.Tree, area domain.Rect) (bool, error) { | ||
| 18 | return candidate.ConsumeOrExpelPane(target.pane.id, direction, area) | 14 | return candidate.ConsumeOrExpelPane(target.pane.id, direction, area) | ||
| @@ -29,9 +25,9 @@ func (d *Daemon) consumeOrExpelPane(target daemonActionTarget, direction layout. | |||||
| 29 | func paneRearrangeUserError(err error) error { | 25 | func paneRearrangeUserError(err error) error { | ||
| 30 | switch { | 26 | switch { | ||
| 31 | case errors.Is(err, layout.ErrUnsupportedColumnLayout): | 27 | case errors.Is(err, layout.ErrUnsupportedColumnLayout): | ||
| 32 | - | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane cannot be rearranged in this layout", err) | 28 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane rearrangement requires a column layout", err) |
| 33 | case errors.Is(err, layout.ErrTooSmall): | 29 | case errors.Is(err, layout.ErrTooSmall): | ||
| 34 | - | return domain.UserWarn(domain.NoticeLayoutTooSmall, "pane cannot be rearranged at this size", err) | 30 | + | return domain.UserWarn(domain.NoticeLayoutTooSmall, "not enough space to rearrange pane", err) |
| 35 | default: | 31 | default: | ||
| 36 | return err | 32 | return err | ||
| 37 | } | 33 | } | ||
M internal/usecase/daemon/pane_rearrange_test.go +133 -33
| @@ -198,8 +198,7 @@ func TestPaneRearrangeEdgeNoopHasExactSilentInvariants(t *testing.T) { | |||
| 198 | 198 | } | |
| 199 | 199 | ||
| 200 | 200 | err := (daemonActions{d: h.daemon}).Run(request) | |
| 201 | - | require.ErrorIs(t, err, errPaneRearrangeNoop, "the daemon action must retain an exact no-change sentinel") | |
| 202 | - | require.NoError(t, (paletteExec{d: h.daemon, sess: h.session, ac: ac}).runAction(request), "palette normalizes the silent sentinel") | |
| 201 | + | require.ErrorIs(t, err, errDaemonActionNoChange, "the daemon action must retain the package no-change sentinel") | |
| 203 | 202 | ||
| 204 | 203 | require.Equal(t, before, h.snapshot(), "edge no-op changed tree, generations, snapshot state, pane identity, geometry, or VT size") | |
| 205 | 204 | require.Zero(t, h.totalResizes()) | |
| @@ -213,12 +212,13 @@ func TestPaneRearrangeEdgeNoopHasExactSilentInvariants(t *testing.T) { | |||
| 213 | 212 | ||
| 214 | 213 | func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testing.T) { | |
| 215 | 214 | tests := []struct { | |
| 216 | - | name string | |
| 217 | - | size domain.Size | |
| 218 | - | tree *layout.Tree | |
| 219 | - | pane layout.PaneID | |
| 220 | - | dir layout.Direction | |
| 221 | - | cause error | |
| 215 | + | name string | |
| 216 | + | size domain.Size | |
| 217 | + | tree *layout.Tree | |
| 218 | + | pane layout.PaneID | |
| 219 | + | dir layout.Direction | |
| 220 | + | cause error | |
| 221 | + | wantMsg string | |
| 222 | 222 | }{ | |
| 223 | 223 | { | |
| 224 | 224 | name: "unsupported layout", | |
| @@ -228,6 +228,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||
| 228 | 228 | {Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-2"), layout.NewLeaf("pane-3")}}, | |
| 229 | 229 | }}, Focus: "pane-1"}, | |
| 230 | 230 | pane: "pane-1", dir: layout.Right, cause: layout.ErrUnsupportedColumnLayout, | |
| 231 | + | wantMsg: "pane rearrangement requires a column layout", | |
| 231 | 232 | }, | |
| 232 | 233 | { | |
| 233 | 234 | name: "too small", | |
| @@ -236,6 +237,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||
| 236 | 237 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | |
| 237 | 238 | }}, Focus: "pane-1"}, | |
| 238 | 239 | pane: "pane-1", dir: layout.Right, cause: layout.ErrTooSmall, | |
| 240 | + | wantMsg: "not enough space to rearrange pane", | |
| 239 | 241 | }, | |
| 240 | 242 | } | |
| 241 | 243 | ||
| @@ -253,6 +255,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||
| 253 | 255 | require.ErrorAs(t, err, &userErr) | |
| 254 | 256 | require.Equal(t, domain.NoticeLayoutTooSmall, userErr.Code) | |
| 255 | 257 | require.Equal(t, domain.NoticeWarn, userErr.Severity) | |
| 258 | + | require.Equal(t, tt.wantMsg, userErr.Msg) | |
| 256 | 259 | require.Equal(t, before, h.snapshot(), "failed rearrange must be atomic") | |
| 257 | 260 | require.Zero(t, h.totalResizes()) | |
| 258 | 261 | h.factory.AssertNotCalled(t, "Open") | |
| @@ -291,40 +294,137 @@ func TestPaneRearrangeChangedActionPublishesOnce(t *testing.T) { | |||
| 291 | 294 | requireNoInvalidation(t, invalidations) | |
| 292 | 295 | } | |
| 293 | 296 | ||
| 294 | - | func TestDaemonActionAdaptersNormalizeOnlyPaneRearrangeNoop(t *testing.T) { | |
| 297 | + | func TestPaneRearrangeAdaptersOwnDirectionalRequestsAndNormalizeOnlyNoChange(t *testing.T) { | |
| 295 | 298 | genuine := errors.New("genuine rearrange failure") | |
| 299 | + | directions := []struct { | |
| 300 | + | name string | |
| 301 | + | direction layout.Direction | |
| 302 | + | keyAction keys.Action | |
| 303 | + | paletteRun func(paletteExec) error | |
| 304 | + | controlRun func(controlExec) error | |
| 305 | + | }{ | |
| 306 | + | { | |
| 307 | + | name: "left", direction: layout.Left, keyAction: keys.ActionConsumeOrExpelPaneLeft, | |
| 308 | + | paletteRun: func(e paletteExec) error { return e.ConsumeOrExpelPaneLeft() }, | |
| 309 | + | controlRun: func(e controlExec) error { return e.ConsumeOrExpelPaneLeft() }, | |
| 310 | + | }, | |
| 311 | + | { | |
| 312 | + | name: "right", direction: layout.Right, keyAction: keys.ActionConsumeOrExpelPaneRight, | |
| 313 | + | paletteRun: func(e paletteExec) error { return e.ConsumeOrExpelPaneRight() }, | |
| 314 | + | controlRun: func(e controlExec) error { return e.ConsumeOrExpelPaneRight() }, | |
| 315 | + | }, | |
| 316 | + | } | |
| 317 | + | outcomes := []struct { | |
| 318 | + | name string | |
| 319 | + | err error | |
| 320 | + | wantErr error | |
| 321 | + | wantNotice bool | |
| 322 | + | }{ | |
| 323 | + | {name: "silent no-change", err: errDaemonActionNoChange}, | |
| 324 | + | {name: "genuine error", err: genuine, wantErr: genuine, wantNotice: true}, | |
| 325 | + | } | |
| 326 | + | ||
| 327 | + | for _, direction := range directions { | |
| 328 | + | t.Run(direction.name, func(t *testing.T) { | |
| 329 | + | for _, outcome := range outcomes { | |
| 330 | + | t.Run(outcome.name, func(t *testing.T) { | |
| 331 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 332 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |
| 333 | + | ac := &attachedClient{} | |
| 334 | + | ac.setSession(sess) | |
| 335 | + | target := resolveDaemonActionTarget(sess) | |
| 336 | + | paletteSpy := &actionRunnerSpy{err: outcome.err} | |
| 337 | + | controlSpy := &actionRunnerSpy{err: outcome.err} | |
| 338 | + | keySpy := &actionRunnerSpy{err: outcome.err} | |
| 339 | + | ||
| 340 | + | paletteErr := direction.paletteRun(paletteExec{d: d, sess: sess, ac: ac, actions: paletteSpy}) | |
| 341 | + | controlErr := direction.controlRun(controlExec{d: d, sess: sess, target: target, actions: controlSpy}) | |
| 342 | + | daemonKeyHandler{d: d, ac: ac, actions: keySpy}.Action(direction.keyAction) | |
| 343 | + | ||
| 344 | + | if outcome.wantErr == nil { | |
| 345 | + | require.NoError(t, paletteErr) | |
| 346 | + | require.NoError(t, controlErr) | |
| 347 | + | } else { | |
| 348 | + | require.ErrorIs(t, paletteErr, outcome.wantErr) | |
| 349 | + | require.ErrorIs(t, controlErr, outcome.wantErr) | |
| 350 | + | } | |
| 351 | + | if outcome.wantNotice { | |
| 352 | + | require.Len(t, d.notices.history(), 1, "the key adapter owns reporting its genuine error") | |
| 353 | + | } else { | |
| 354 | + | require.Empty(t, d.notices.history(), "no-change is silent in the key adapter") | |
| 355 | + | } | |
| 356 | + | ||
| 357 | + | for adapter, spy := range map[string]*actionRunnerSpy{ | |
| 358 | + | "palette": paletteSpy, | |
| 359 | + | "control": controlSpy, | |
| 360 | + | "key": keySpy, | |
| 361 | + | } { | |
| 362 | + | require.Len(t, spy.requests, 1, "%s adapter request count", adapter) | |
| 363 | + | request := spy.requests[0] | |
| 364 | + | require.Equal(t, daemonActionConsumeOrExpelPane, request.kind, "%s adapter action kind", adapter) | |
| 365 | + | require.Equal(t, direction.direction, request.direction, "%s adapter direction", adapter) | |
| 366 | + | require.Same(t, target.session, request.target.session, "%s adapter target session", adapter) | |
| 367 | + | require.Same(t, target.tab, request.target.tab, "%s adapter target tab", adapter) | |
| 368 | + | require.Same(t, target.pane, request.target.pane, "%s adapter target pane", adapter) | |
| 369 | + | } | |
| 370 | + | }) | |
| 371 | + | } | |
| 372 | + | }) | |
| 373 | + | } | |
| 374 | + | } | |
| 375 | + | ||
| 376 | + | func TestPaneRearrangeNoChangeAdapterSideEffects(t *testing.T) { | |
| 296 | 377 | tests := []struct { | |
| 297 | - | name string | |
| 298 | - | err error | |
| 299 | - | wantErr error | |
| 300 | - | wantLog bool | |
| 378 | + | name string | |
| 379 | + | invoke func(*paneRearrangeHarness, *attachedClient) error | |
| 380 | + | wantCleanupInvalidation bool | |
| 301 | 381 | }{ | |
| 302 | - | {name: "silent no-op", err: errPaneRearrangeNoop}, | |
| 303 | - | {name: "genuine error", err: genuine, wantErr: genuine, wantLog: true}, | |
| 382 | + | { | |
| 383 | + | name: "palette cleans up its closed overlay", | |
| 384 | + | invoke: func(h *paneRearrangeHarness, ac *attachedClient) error { | |
| 385 | + | return (paletteExec{d: h.daemon, sess: h.session, ac: ac, redrawClosedPalette: true}).ConsumeOrExpelPaneLeft() | |
| 386 | + | }, | |
| 387 | + | wantCleanupInvalidation: true, | |
| 388 | + | }, | |
| 389 | + | { | |
| 390 | + | name: "control remains silent", | |
| 391 | + | invoke: func(h *paneRearrangeHarness, _ *attachedClient) error { | |
| 392 | + | return (controlExec{d: h.daemon, sess: h.session, target: h.target("pane-1")}).ConsumeOrExpelPaneLeft() | |
| 393 | + | }, | |
| 394 | + | }, | |
| 395 | + | { | |
| 396 | + | name: "key remains silent", | |
| 397 | + | invoke: func(h *paneRearrangeHarness, ac *attachedClient) error { | |
| 398 | + | daemonKeyHandler{d: h.daemon, ac: ac}.Action(keys.ActionConsumeOrExpelPaneLeft) | |
| 399 | + | return nil | |
| 400 | + | }, | |
| 401 | + | }, | |
| 304 | 402 | } | |
| 305 | 403 | ||
| 306 | 404 | for _, tt := range tests { | |
| 307 | 405 | t.Run(tt.name, func(t *testing.T) { | |
| 308 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 309 | - | sess := addControlSession(d, "work", "t_work", "p_work") | |
| 406 | + | h := newPaneRearrangeHarness(t, domain.Size{Cols: 80, Rows: 22}, threeColumnTree()) | |
| 310 | 407 | ac := &attachedClient{} | |
| 311 | - | ac.setSession(sess) | |
| 312 | - | target := resolveDaemonActionTarget(sess) | |
| 313 | - | request := daemonActionRequest{kind: daemonActionConsumeOrExpelPane, target: target, direction: layout.Left} | |
| 314 | - | ||
| 315 | - | paletteErr := (paletteExec{d: d, sess: sess, ac: ac, actions: &actionRunnerSpy{err: tt.err}}).runAction(request) | |
| 316 | - | controlErr := (controlExec{d: d, sess: sess, target: target, actions: &actionRunnerSpy{err: tt.err}}).runAction(request) | |
| 317 | - | require.ErrorIs(t, paletteErr, tt.wantErr) | |
| 318 | - | require.ErrorIs(t, controlErr, tt.wantErr) | |
| 319 | - | ||
| 320 | - | beforeNotices := len(d.notices.history()) | |
| 321 | - | daemonKeyHandler{d: d, ac: ac, actions: &actionRunnerSpy{err: tt.err}}.Action(keys.ActionEqualizePanes) | |
| 322 | - | afterNotices := len(d.notices.history()) | |
| 323 | - | if tt.wantLog { | |
| 324 | - | require.Equal(t, beforeNotices+1, afterNotices, "key action must report a genuine error") | |
| 325 | - | } else { | |
| 326 | - | require.Equal(t, beforeNotices, afterNotices, "key action must silently normalize rearrange no-op") | |
| 408 | + | ac.setSession(h.session) | |
| 409 | + | h.session.mu.Lock() | |
| 410 | + | h.session.client = ac | |
| 411 | + | h.session.mu.Unlock() | |
| 412 | + | invalidations := make(chan renderInvalidation, 2) | |
| 413 | + | rc := newRenderCoordinator(renderCoordinatorOptions{onInvalidate: func(inv renderInvalidation) { invalidations <- inv }}) | |
| 414 | + | rc.attach(ac) | |
| 415 | + | h.session.installRenderCoordinator(rc) | |
| 416 | + | before := h.snapshot() | |
| 417 | + | ||
| 418 | + | require.NoError(t, tt.invoke(h, ac)) | |
| 419 | + | ||
| 420 | + | require.Equal(t, before, h.snapshot(), "adapter no-change must not publish layout state") | |
| 421 | + | require.Empty(t, h.daemon.notices.history()) | |
| 422 | + | if tt.wantCleanupInvalidation { | |
| 423 | + | invalidation := awaitInvalidation(t, invalidations) | |
| 424 | + | require.Equal(t, "palette.go", invalidation.producer) | |
| 425 | + | require.True(t, invalidation.reset) | |
| 327 | 426 | } | |
| 427 | + | requireNoInvalidation(t, invalidations) | |
| 328 | 428 | }) | |
| 329 | 429 | } | |
| 330 | 430 | } | |
| @@ -198,8 +198,7 @@ func TestPaneRearrangeEdgeNoopHasExactSilentInvariants(t *testing.T) { | |||||
| 198 | } | 198 | } | ||
| 199 | 199 | ||||
| 200 | err := (daemonActions{d: h.daemon}).Run(request) | 200 | err := (daemonActions{d: h.daemon}).Run(request) | ||
| 201 | - | require.ErrorIs(t, err, errPaneRearrangeNoop, "the daemon action must retain an exact no-change sentinel") | 201 | + | require.ErrorIs(t, err, errDaemonActionNoChange, "the daemon action must retain the package no-change sentinel") |
| 202 | - | require.NoError(t, (paletteExec{d: h.daemon, sess: h.session, ac: ac}).runAction(request), "palette normalizes the silent sentinel") | |||
| 203 | 202 | ||||
| 204 | require.Equal(t, before, h.snapshot(), "edge no-op changed tree, generations, snapshot state, pane identity, geometry, or VT size") | 203 | require.Equal(t, before, h.snapshot(), "edge no-op changed tree, generations, snapshot state, pane identity, geometry, or VT size") | ||
| 205 | require.Zero(t, h.totalResizes()) | 204 | require.Zero(t, h.totalResizes()) | ||
| @@ -213,12 +212,13 @@ func TestPaneRearrangeEdgeNoopHasExactSilentInvariants(t *testing.T) { | |||||
| 213 | 212 | ||||
| 214 | func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testing.T) { | 213 | func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testing.T) { | ||
| 215 | tests := []struct { | 214 | tests := []struct { | ||
| 216 | - | name string | 215 | + | name string |
| 217 | - | size domain.Size | 216 | + | size domain.Size |
| 218 | - | tree *layout.Tree | 217 | + | tree *layout.Tree |
| 219 | - | pane layout.PaneID | 218 | + | pane layout.PaneID |
| 220 | - | dir layout.Direction | 219 | + | dir layout.Direction |
| 221 | - | cause error | 220 | + | cause error |
| 221 | + | wantMsg string | |||
| 222 | }{ | 222 | }{ | ||
| 223 | { | 223 | { | ||
| 224 | name: "unsupported layout", | 224 | name: "unsupported layout", | ||
| @@ -228,6 +228,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||||
| 228 | {Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-2"), layout.NewLeaf("pane-3")}}, | 228 | {Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-2"), layout.NewLeaf("pane-3")}}, | ||
| 229 | }}, Focus: "pane-1"}, | 229 | }}, Focus: "pane-1"}, | ||
| 230 | pane: "pane-1", dir: layout.Right, cause: layout.ErrUnsupportedColumnLayout, | 230 | pane: "pane-1", dir: layout.Right, cause: layout.ErrUnsupportedColumnLayout, | ||
| 231 | + | wantMsg: "pane rearrangement requires a column layout", | |||
| 231 | }, | 232 | }, | ||
| 232 | { | 233 | { | ||
| 233 | name: "too small", | 234 | name: "too small", | ||
| @@ -236,6 +237,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||||
| 236 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | 237 | layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2"), | ||
| 237 | }}, Focus: "pane-1"}, | 238 | }}, Focus: "pane-1"}, | ||
| 238 | pane: "pane-1", dir: layout.Right, cause: layout.ErrTooSmall, | 239 | pane: "pane-1", dir: layout.Right, cause: layout.ErrTooSmall, | ||
| 240 | + | wantMsg: "not enough space to rearrange pane", | |||
| 239 | }, | 241 | }, | ||
| 240 | } | 242 | } | ||
| 241 | 243 | ||||
| @@ -253,6 +255,7 @@ func TestPaneRearrangeMapsUnsupportedAndTooSmallToUserNoticesAtomically(t *testi | |||||
| 253 | require.ErrorAs(t, err, &userErr) | 255 | require.ErrorAs(t, err, &userErr) | ||
| 254 | require.Equal(t, domain.NoticeLayoutTooSmall, userErr.Code) | 256 | require.Equal(t, domain.NoticeLayoutTooSmall, userErr.Code) | ||
| 255 | require.Equal(t, domain.NoticeWarn, userErr.Severity) | 257 | require.Equal(t, domain.NoticeWarn, userErr.Severity) | ||
| 258 | + | require.Equal(t, tt.wantMsg, userErr.Msg) | |||
| 256 | require.Equal(t, before, h.snapshot(), "failed rearrange must be atomic") | 259 | require.Equal(t, before, h.snapshot(), "failed rearrange must be atomic") | ||
| 257 | require.Zero(t, h.totalResizes()) | 260 | require.Zero(t, h.totalResizes()) | ||
| 258 | h.factory.AssertNotCalled(t, "Open") | 261 | h.factory.AssertNotCalled(t, "Open") | ||
| @@ -291,40 +294,137 @@ func TestPaneRearrangeChangedActionPublishesOnce(t *testing.T) { | |||||
| 291 | requireNoInvalidation(t, invalidations) | 294 | requireNoInvalidation(t, invalidations) | ||
| 292 | } | 295 | } | ||
| 293 | 296 | ||||
| 294 | - | func TestDaemonActionAdaptersNormalizeOnlyPaneRearrangeNoop(t *testing.T) { | 297 | + | func TestPaneRearrangeAdaptersOwnDirectionalRequestsAndNormalizeOnlyNoChange(t *testing.T) { |
| 295 | genuine := errors.New("genuine rearrange failure") | 298 | genuine := errors.New("genuine rearrange failure") | ||
| 299 | + | directions := []struct { | |||
| 300 | + | name string | |||
| 301 | + | direction layout.Direction | |||
| 302 | + | keyAction keys.Action | |||
| 303 | + | paletteRun func(paletteExec) error | |||
| 304 | + | controlRun func(controlExec) error | |||
| 305 | + | }{ | |||
| 306 | + | { | |||
| 307 | + | name: "left", direction: layout.Left, keyAction: keys.ActionConsumeOrExpelPaneLeft, | |||
| 308 | + | paletteRun: func(e paletteExec) error { return e.ConsumeOrExpelPaneLeft() }, | |||
| 309 | + | controlRun: func(e controlExec) error { return e.ConsumeOrExpelPaneLeft() }, | |||
| 310 | + | }, | |||
| 311 | + | { | |||
| 312 | + | name: "right", direction: layout.Right, keyAction: keys.ActionConsumeOrExpelPaneRight, | |||
| 313 | + | paletteRun: func(e paletteExec) error { return e.ConsumeOrExpelPaneRight() }, | |||
| 314 | + | controlRun: func(e controlExec) error { return e.ConsumeOrExpelPaneRight() }, | |||
| 315 | + | }, | |||
| 316 | + | } | |||
| 317 | + | outcomes := []struct { | |||
| 318 | + | name string | |||
| 319 | + | err error | |||
| 320 | + | wantErr error | |||
| 321 | + | wantNotice bool | |||
| 322 | + | }{ | |||
| 323 | + | {name: "silent no-change", err: errDaemonActionNoChange}, | |||
| 324 | + | {name: "genuine error", err: genuine, wantErr: genuine, wantNotice: true}, | |||
| 325 | + | } | |||
| 326 | + | ||||
| 327 | + | for _, direction := range directions { | |||
| 328 | + | t.Run(direction.name, func(t *testing.T) { | |||
| 329 | + | for _, outcome := range outcomes { | |||
| 330 | + | t.Run(outcome.name, func(t *testing.T) { | |||
| 331 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 332 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |||
| 333 | + | ac := &attachedClient{} | |||
| 334 | + | ac.setSession(sess) | |||
| 335 | + | target := resolveDaemonActionTarget(sess) | |||
| 336 | + | paletteSpy := &actionRunnerSpy{err: outcome.err} | |||
| 337 | + | controlSpy := &actionRunnerSpy{err: outcome.err} | |||
| 338 | + | keySpy := &actionRunnerSpy{err: outcome.err} | |||
| 339 | + | ||||
| 340 | + | paletteErr := direction.paletteRun(paletteExec{d: d, sess: sess, ac: ac, actions: paletteSpy}) | |||
| 341 | + | controlErr := direction.controlRun(controlExec{d: d, sess: sess, target: target, actions: controlSpy}) | |||
| 342 | + | daemonKeyHandler{d: d, ac: ac, actions: keySpy}.Action(direction.keyAction) | |||
| 343 | + | ||||
| 344 | + | if outcome.wantErr == nil { | |||
| 345 | + | require.NoError(t, paletteErr) | |||
| 346 | + | require.NoError(t, controlErr) | |||
| 347 | + | } else { | |||
| 348 | + | require.ErrorIs(t, paletteErr, outcome.wantErr) | |||
| 349 | + | require.ErrorIs(t, controlErr, outcome.wantErr) | |||
| 350 | + | } | |||
| 351 | + | if outcome.wantNotice { | |||
| 352 | + | require.Len(t, d.notices.history(), 1, "the key adapter owns reporting its genuine error") | |||
| 353 | + | } else { | |||
| 354 | + | require.Empty(t, d.notices.history(), "no-change is silent in the key adapter") | |||
| 355 | + | } | |||
| 356 | + | ||||
| 357 | + | for adapter, spy := range map[string]*actionRunnerSpy{ | |||
| 358 | + | "palette": paletteSpy, | |||
| 359 | + | "control": controlSpy, | |||
| 360 | + | "key": keySpy, | |||
| 361 | + | } { | |||
| 362 | + | require.Len(t, spy.requests, 1, "%s adapter request count", adapter) | |||
| 363 | + | request := spy.requests[0] | |||
| 364 | + | require.Equal(t, daemonActionConsumeOrExpelPane, request.kind, "%s adapter action kind", adapter) | |||
| 365 | + | require.Equal(t, direction.direction, request.direction, "%s adapter direction", adapter) | |||
| 366 | + | require.Same(t, target.session, request.target.session, "%s adapter target session", adapter) | |||
| 367 | + | require.Same(t, target.tab, request.target.tab, "%s adapter target tab", adapter) | |||
| 368 | + | require.Same(t, target.pane, request.target.pane, "%s adapter target pane", adapter) | |||
| 369 | + | } | |||
| 370 | + | }) | |||
| 371 | + | } | |||
| 372 | + | }) | |||
| 373 | + | } | |||
| 374 | + | } | |||
| 375 | + | ||||
| 376 | + | func TestPaneRearrangeNoChangeAdapterSideEffects(t *testing.T) { | |||
| 296 | tests := []struct { | 377 | tests := []struct { | ||
| 297 | - | name string | 378 | + | name string |
| 298 | - | err error | 379 | + | invoke func(*paneRearrangeHarness, *attachedClient) error |
| 299 | - | wantErr error | 380 | + | wantCleanupInvalidation bool |
| 300 | - | wantLog bool | |||
| 301 | }{ | 381 | }{ | ||
| 302 | - | {name: "silent no-op", err: errPaneRearrangeNoop}, | 382 | + | { |
| 303 | - | {name: "genuine error", err: genuine, wantErr: genuine, wantLog: true}, | 383 | + | name: "palette cleans up its closed overlay", |
| 384 | + | invoke: func(h *paneRearrangeHarness, ac *attachedClient) error { | |||
| 385 | + | return (paletteExec{d: h.daemon, sess: h.session, ac: ac, redrawClosedPalette: true}).ConsumeOrExpelPaneLeft() | |||
| 386 | + | }, | |||
| 387 | + | wantCleanupInvalidation: true, | |||
| 388 | + | }, | |||
| 389 | + | { | |||
| 390 | + | name: "control remains silent", | |||
| 391 | + | invoke: func(h *paneRearrangeHarness, _ *attachedClient) error { | |||
| 392 | + | return (controlExec{d: h.daemon, sess: h.session, target: h.target("pane-1")}).ConsumeOrExpelPaneLeft() | |||
| 393 | + | }, | |||
| 394 | + | }, | |||
| 395 | + | { | |||
| 396 | + | name: "key remains silent", | |||
| 397 | + | invoke: func(h *paneRearrangeHarness, ac *attachedClient) error { | |||
| 398 | + | daemonKeyHandler{d: h.daemon, ac: ac}.Action(keys.ActionConsumeOrExpelPaneLeft) | |||
| 399 | + | return nil | |||
| 400 | + | }, | |||
| 401 | + | }, | |||
| 304 | } | 402 | } | ||
| 305 | 403 | ||||
| 306 | for _, tt := range tests { | 404 | for _, tt := range tests { | ||
| 307 | t.Run(tt.name, func(t *testing.T) { | 405 | t.Run(tt.name, func(t *testing.T) { | ||
| 308 | - | d := newTestDaemon(t, nil, stubClock{}) | 406 | + | h := newPaneRearrangeHarness(t, domain.Size{Cols: 80, Rows: 22}, threeColumnTree()) |
| 309 | - | sess := addControlSession(d, "work", "t_work", "p_work") | |||
| 310 | ac := &attachedClient{} | 407 | ac := &attachedClient{} | ||
| 311 | - | ac.setSession(sess) | 408 | + | ac.setSession(h.session) |
| 312 | - | target := resolveDaemonActionTarget(sess) | 409 | + | h.session.mu.Lock() |
| 313 | - | request := daemonActionRequest{kind: daemonActionConsumeOrExpelPane, target: target, direction: layout.Left} | 410 | + | h.session.client = ac |
| 314 | - | 411 | + | h.session.mu.Unlock() | |
| 315 | - | paletteErr := (paletteExec{d: d, sess: sess, ac: ac, actions: &actionRunnerSpy{err: tt.err}}).runAction(request) | 412 | + | invalidations := make(chan renderInvalidation, 2) |
| 316 | - | controlErr := (controlExec{d: d, sess: sess, target: target, actions: &actionRunnerSpy{err: tt.err}}).runAction(request) | 413 | + | rc := newRenderCoordinator(renderCoordinatorOptions{onInvalidate: func(inv renderInvalidation) { invalidations <- inv }}) |
| 317 | - | require.ErrorIs(t, paletteErr, tt.wantErr) | 414 | + | rc.attach(ac) |
| 318 | - | require.ErrorIs(t, controlErr, tt.wantErr) | 415 | + | h.session.installRenderCoordinator(rc) |
| 319 | - | 416 | + | before := h.snapshot() | |
| 320 | - | beforeNotices := len(d.notices.history()) | 417 | + | |
| 321 | - | daemonKeyHandler{d: d, ac: ac, actions: &actionRunnerSpy{err: tt.err}}.Action(keys.ActionEqualizePanes) | 418 | + | require.NoError(t, tt.invoke(h, ac)) |
| 322 | - | afterNotices := len(d.notices.history()) | 419 | + | |
| 323 | - | if tt.wantLog { | 420 | + | require.Equal(t, before, h.snapshot(), "adapter no-change must not publish layout state") |
| 324 | - | require.Equal(t, beforeNotices+1, afterNotices, "key action must report a genuine error") | 421 | + | require.Empty(t, h.daemon.notices.history()) |
| 325 | - | } else { | 422 | + | if tt.wantCleanupInvalidation { |
| 326 | - | require.Equal(t, beforeNotices, afterNotices, "key action must silently normalize rearrange no-op") | 423 | + | invalidation := awaitInvalidation(t, invalidations) |
| 424 | + | require.Equal(t, "palette.go", invalidation.producer) | |||
| 425 | + | require.True(t, invalidation.reset) | |||
| 327 | } | 426 | } | ||
| 427 | + | requireNoInvalidation(t, invalidations) | |||
| 328 | }) | 428 | }) | ||
| 329 | } | 429 | } | ||
| 330 | } | 430 | } | ||