test(daemon): assert pane cancellation follows shutdown preservation reservation
- commit
- d2612758ef553a42bb50211e0272a3e7c9ce580f
- author
- brice · · 2026-10-03 15:56 UTC
- parent
- 291cc75
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- feat/host-transport-flag
3 files · +37 -28
M internal/app/daemon_integration_test.go +1 -0
| @@ -426,6 +426,7 @@ func requireListedAfterRestart(t *testing.T, sessions []protocol.SessionInfo, na | |||
| 426 | 426 | require.Contains(t, []protocol.SessionState{protocol.SessionDown, protocol.SessionUp}, info.State, | |
| 427 | 427 | "session %q must be stopped or restored, not broken", info.Name) | |
| 428 | 428 | require.False(t, info.Ephemeral, "session %q must be a durable named session", info.Name) | |
| 429 | + | require.False(t, info.Attached, "session %q has no client after restart", info.Name) | |
| 429 | 430 | } | |
| 430 | 431 | require.Equal(t, names, got) | |
| 431 | 432 | } | |
| @@ -426,6 +426,7 @@ func requireListedAfterRestart(t *testing.T, sessions []protocol.SessionInfo, na | |||||
| 426 | require.Contains(t, []protocol.SessionState{protocol.SessionDown, protocol.SessionUp}, info.State, | 426 | require.Contains(t, []protocol.SessionState{protocol.SessionDown, protocol.SessionUp}, info.State, | ||
| 427 | "session %q must be stopped or restored, not broken", info.Name) | 427 | "session %q must be stopped or restored, not broken", info.Name) | ||
| 428 | require.False(t, info.Ephemeral, "session %q must be a durable named session", info.Name) | 428 | require.False(t, info.Ephemeral, "session %q must be a durable named session", info.Name) | ||
| 429 | + | require.False(t, info.Attached, "session %q has no client after restart", info.Name) | |||
| 429 | } | 430 | } | ||
| 430 | require.Equal(t, names, got) | 431 | require.Equal(t, names, got) | ||
| 431 | } | 432 | } | ||
M internal/usecase/daemon/daemon.go +3 -2
| @@ -909,8 +909,9 @@ func (d *Daemon) Serve(ctx context.Context, l ports.ServerListener) error { | |||
| 909 | 909 | ||
| 910 | 910 | // shutdownAll is the explicit daemon-stop operation (wire KillDaemon). It is | |
| 911 | 911 | // the only caller that irreversibly ends the daemon: it closes move admission, | |
| 912 | - | // marks closing, cancels the daemon-wide pane process context, preserves every | |
| 913 | - | // live session as stopped durable authority, and closes done so Serve returns. | |
| 912 | + | // marks closing, reserves shutdown preservation for every live session, then | |
| 913 | + | // cancels the daemon-wide pane process context, preserves every live session as | |
| 914 | + | // stopped durable authority, and closes done so Serve returns. | |
| 914 | 915 | // KillAll never reaches this path. Setting closing under the same lock as the | |
| 915 | 916 | // snapshot guarantees no session can be inserted after the snapshot: route | |
| 916 | 917 | // rejects once closing is set, and both run under d.mu. killSession (which | |
| @@ -909,8 +909,9 @@ func (d *Daemon) Serve(ctx context.Context, l ports.ServerListener) error { | |||||
| 909 | 909 | ||||
| 910 | // shutdownAll is the explicit daemon-stop operation (wire KillDaemon). It is | 910 | // shutdownAll is the explicit daemon-stop operation (wire KillDaemon). It is | ||
| 911 | // the only caller that irreversibly ends the daemon: it closes move admission, | 911 | // the only caller that irreversibly ends the daemon: it closes move admission, | ||
| 912 | - | // marks closing, cancels the daemon-wide pane process context, preserves every | 912 | + | // marks closing, reserves shutdown preservation for every live session, then |
| 913 | - | // live session as stopped durable authority, and closes done so Serve returns. | 913 | + | // cancels the daemon-wide pane process context, preserves every live session as |
| 914 | + | // stopped durable authority, and closes done so Serve returns. | |||
| 914 | // KillAll never reaches this path. Setting closing under the same lock as the | 915 | // KillAll never reaches this path. Setting closing under the same lock as the | ||
| 915 | // snapshot guarantees no session can be inserted after the snapshot: route | 916 | // snapshot guarantees no session can be inserted after the snapshot: route | ||
| 916 | // rejects once closing is set, and both run under d.mu. killSession (which | 917 | // rejects once closing is set, and both run under d.mu. killSession (which | ||
M internal/usecase/daemon/move_lifecycle_test.go +33 -26
| @@ -1,11 +1,14 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "fmt" | |
| 4 | 5 | "testing" | |
| 5 | 6 | "time" | |
| 6 | 7 | ||
| 7 | - | "github.com/bnema/vev/internal/protocol" | |
| 8 | 8 | "github.com/stretchr/testify/require" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/vev/internal/domain" | |
| 11 | + | "github.com/bnema/vev/internal/protocol" | |
| 9 | 12 | ) | |
| 10 | 13 | ||
| 11 | 14 | func awaitMoveTeardownState(t *testing.T, sess *session, reservations, waiters uint) { | |
| @@ -278,43 +281,47 @@ func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) { | |||
| 278 | 281 | // daemon-wide pane lifetime closes every PTY; a session whose reader sees that | |
| 279 | 282 | // EOF before its shutdown-preservation policy is reserved takes the natural | |
| 280 | 283 | // final-shell-exit path and purges its durable record instead of preserving it. | |
| 281 | - | // The drain step must therefore leave pane processes running; only the full | |
| 282 | - | // shutdown cancels them, after reserving preservation for every live session. | |
| 284 | + | // The test records each session's reservation at the exact moment the pane | |
| 285 | + | // lifetime is cancelled, so it fails if cancellation ever precedes it. | |
| 283 | 286 | func TestShutdownReservesPreservationBeforeCancellingPaneProcesses(t *testing.T) { | |
| 284 | 287 | tests := []struct { | |
| 285 | - | name string | |
| 286 | - | run func(d *Daemon) | |
| 287 | - | wantCancelled bool | |
| 288 | - | wantReserved bool | |
| 288 | + | name string | |
| 289 | + | sessions []string | |
| 289 | 290 | }{ | |
| 290 | - | { | |
| 291 | - | name: "draining move admission leaves pane processes running", | |
| 292 | - | run: func(d *Daemon) { d.closeMoveLifecycles() }, | |
| 293 | - | }, | |
| 294 | - | { | |
| 295 | - | name: "full shutdown reserves preservation and cancels pane processes", | |
| 296 | - | run: func(d *Daemon) { d.shutdownAll(protocol.ReasonServerShutdown) }, | |
| 297 | - | wantCancelled: true, | |
| 298 | - | wantReserved: true, | |
| 299 | - | }, | |
| 291 | + | {name: "one named session", sessions: []string{"work"}}, | |
| 292 | + | {name: "several named sessions", sessions: []string{"alpha", "beta", "gamma"}}, | |
| 300 | 293 | } | |
| 301 | 294 | ||
| 302 | 295 | for _, tt := range tests { | |
| 303 | 296 | t.Run(tt.name, func(t *testing.T) { | |
| 304 | 297 | d := newTestDaemon(t, nil, stubClock{}) | |
| 305 | - | sess := &session{sessionCore: sessionCore{id: "sess-0", name: "work"}} | |
| 298 | + | sessions := make([]*session, 0, len(tt.sessions)) | |
| 306 | 299 | d.mu.Lock() | |
| 307 | - | require.True(t, d.registerSessionLocked(sess)) | |
| 300 | + | for i, name := range tt.sessions { | |
| 301 | + | sess := &session{sessionCore: sessionCore{id: domain.SessionID(fmt.Sprintf("sess-%d", i)), name: name}} | |
| 302 | + | require.True(t, d.registerSessionLocked(sess)) | |
| 303 | + | sessions = append(sessions, sess) | |
| 304 | + | } | |
| 308 | 305 | d.mu.Unlock() | |
| 309 | 306 | ||
| 310 | - | tt.run(d) | |
| 307 | + | var reservedAtCancel []bool | |
| 308 | + | cancel := d.paneProcessCancel | |
| 309 | + | d.paneProcessCancel = func() { | |
| 310 | + | for _, sess := range sessions { | |
| 311 | + | sess.teardownMu.Lock() | |
| 312 | + | reservedAtCancel = append(reservedAtCancel, sess.shutdownTeardownRequested) | |
| 313 | + | sess.teardownMu.Unlock() | |
| 314 | + | } | |
| 315 | + | cancel() | |
| 316 | + | } | |
| 317 | + | ||
| 318 | + | d.shutdownAll(protocol.ReasonServerShutdown) | |
| 311 | 319 | ||
| 312 | - | cancelled := d.paneProcessCtx.Err() != nil | |
| 313 | - | sess.teardownMu.Lock() | |
| 314 | - | reserved := sess.shutdownTeardownRequested | |
| 315 | - | sess.teardownMu.Unlock() | |
| 316 | - | require.Equal(t, tt.wantCancelled, cancelled, "pane process cancellation") | |
| 317 | - | require.Equal(t, tt.wantReserved, reserved, "shutdown preservation reservation") | |
| 320 | + | require.Error(t, d.paneProcessCtx.Err(), "shutdown must cancel pane processes") | |
| 321 | + | require.NotEmpty(t, reservedAtCancel, "pane process cancellation was not observed") | |
| 322 | + | for i, reserved := range reservedAtCancel { | |
| 323 | + | require.True(t, reserved, "session %q was not reserved for preservation before its PTY was cancelled", tt.sessions[i%len(tt.sessions)]) | |
| 324 | + | } | |
| 318 | 325 | }) | |
| 319 | 326 | } | |
| 320 | 327 | } | |
| @@ -1,11 +1,14 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "fmt" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | "time" | 6 | "time" | ||
| 6 | 7 | ||||
| 7 | - | "github.com/bnema/vev/internal/protocol" | |||
| 8 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 9 | + | ||||
| 10 | + | "github.com/bnema/vev/internal/domain" | |||
| 11 | + | "github.com/bnema/vev/internal/protocol" | |||
| 9 | ) | 12 | ) | ||
| 10 | 13 | ||||
| 11 | func awaitMoveTeardownState(t *testing.T, sess *session, reservations, waiters uint) { | 14 | func awaitMoveTeardownState(t *testing.T, sess *session, reservations, waiters uint) { | ||
| @@ -278,43 +281,47 @@ func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) { | |||||
| 278 | // daemon-wide pane lifetime closes every PTY; a session whose reader sees that | 281 | // daemon-wide pane lifetime closes every PTY; a session whose reader sees that | ||
| 279 | // EOF before its shutdown-preservation policy is reserved takes the natural | 282 | // EOF before its shutdown-preservation policy is reserved takes the natural | ||
| 280 | // final-shell-exit path and purges its durable record instead of preserving it. | 283 | // final-shell-exit path and purges its durable record instead of preserving it. | ||
| 281 | - | // The drain step must therefore leave pane processes running; only the full | 284 | + | // The test records each session's reservation at the exact moment the pane |
| 282 | - | // shutdown cancels them, after reserving preservation for every live session. | 285 | + | // lifetime is cancelled, so it fails if cancellation ever precedes it. |
| 283 | func TestShutdownReservesPreservationBeforeCancellingPaneProcesses(t *testing.T) { | 286 | func TestShutdownReservesPreservationBeforeCancellingPaneProcesses(t *testing.T) { | ||
| 284 | tests := []struct { | 287 | tests := []struct { | ||
| 285 | - | name string | 288 | + | name string |
| 286 | - | run func(d *Daemon) | 289 | + | sessions []string |
| 287 | - | wantCancelled bool | |||
| 288 | - | wantReserved bool | |||
| 289 | }{ | 290 | }{ | ||
| 290 | - | { | 291 | + | {name: "one named session", sessions: []string{"work"}}, |
| 291 | - | name: "draining move admission leaves pane processes running", | 292 | + | {name: "several named sessions", sessions: []string{"alpha", "beta", "gamma"}}, |
| 292 | - | run: func(d *Daemon) { d.closeMoveLifecycles() }, | |||
| 293 | - | }, | |||
| 294 | - | { | |||
| 295 | - | name: "full shutdown reserves preservation and cancels pane processes", | |||
| 296 | - | run: func(d *Daemon) { d.shutdownAll(protocol.ReasonServerShutdown) }, | |||
| 297 | - | wantCancelled: true, | |||
| 298 | - | wantReserved: true, | |||
| 299 | - | }, | |||
| 300 | } | 293 | } | ||
| 301 | 294 | ||||
| 302 | for _, tt := range tests { | 295 | for _, tt := range tests { | ||
| 303 | t.Run(tt.name, func(t *testing.T) { | 296 | t.Run(tt.name, func(t *testing.T) { | ||
| 304 | d := newTestDaemon(t, nil, stubClock{}) | 297 | d := newTestDaemon(t, nil, stubClock{}) | ||
| 305 | - | sess := &session{sessionCore: sessionCore{id: "sess-0", name: "work"}} | 298 | + | sessions := make([]*session, 0, len(tt.sessions)) |
| 306 | d.mu.Lock() | 299 | d.mu.Lock() | ||
| 307 | - | require.True(t, d.registerSessionLocked(sess)) | 300 | + | for i, name := range tt.sessions { |
| 301 | + | sess := &session{sessionCore: sessionCore{id: domain.SessionID(fmt.Sprintf("sess-%d", i)), name: name}} | |||
| 302 | + | require.True(t, d.registerSessionLocked(sess)) | |||
| 303 | + | sessions = append(sessions, sess) | |||
| 304 | + | } | |||
| 308 | d.mu.Unlock() | 305 | d.mu.Unlock() | ||
| 309 | 306 | ||||
| 310 | - | tt.run(d) | 307 | + | var reservedAtCancel []bool |
| 308 | + | cancel := d.paneProcessCancel | |||
| 309 | + | d.paneProcessCancel = func() { | |||
| 310 | + | for _, sess := range sessions { | |||
| 311 | + | sess.teardownMu.Lock() | |||
| 312 | + | reservedAtCancel = append(reservedAtCancel, sess.shutdownTeardownRequested) | |||
| 313 | + | sess.teardownMu.Unlock() | |||
| 314 | + | } | |||
| 315 | + | cancel() | |||
| 316 | + | } | |||
| 317 | + | ||||
| 318 | + | d.shutdownAll(protocol.ReasonServerShutdown) | |||
| 311 | 319 | ||||
| 312 | - | cancelled := d.paneProcessCtx.Err() != nil | 320 | + | require.Error(t, d.paneProcessCtx.Err(), "shutdown must cancel pane processes") |
| 313 | - | sess.teardownMu.Lock() | 321 | + | require.NotEmpty(t, reservedAtCancel, "pane process cancellation was not observed") |
| 314 | - | reserved := sess.shutdownTeardownRequested | 322 | + | for i, reserved := range reservedAtCancel { |
| 315 | - | sess.teardownMu.Unlock() | 323 | + | require.True(t, reserved, "session %q was not reserved for preservation before its PTY was cancelled", tt.sessions[i%len(tt.sessions)]) |
| 316 | - | require.Equal(t, tt.wantCancelled, cancelled, "pane process cancellation") | 324 | + | } |
| 317 | - | require.Equal(t, tt.wantReserved, reserved, "shutdown preservation reservation") | |||
| 318 | }) | 325 | }) | ||
| 319 | } | 326 | } | ||
| 320 | } | 327 | } | ||