From ba7e2dabf821b67982dcfadec30dc4e08887d6c9 Mon Sep 17 00:00:00 2001 From: Shreyansh Jain Date: Mon, 18 May 2026 15:16:06 +0530 Subject: [PATCH] gcs: fix flaky test on bridge teardown race TestGcsCreateProcess occasionally panicked with Fail in goroutine after Test... completed because the fake guest (simpleGcs) goroutine outlived the test and called t.Error from there. Two layered fixes in guestconnection_test.go: 1. simpleGcsLoop now treats io.ErrUnexpectedEOF as a benign teardown signal alongside io.EOF and io.ErrClosedPipe. The bridge's brdg.Close() closes the underlying io.Pipe synchronously, so simpleGcs can legitimately observe a truncated frame mid-body when the test tears down. Production behavior is unchanged: a real bridge still surfaces ErrUnexpectedEOF as a fatal protocol error. 2. connectGcs now joins the simpleGcs goroutine via t.Cleanup so any future t.Error from background work lands inside the test scope as a normal failure instead of panicking the runtime. Reproduced at ~2/30 iterations pre-fix; 100/100 clean post-fix. Signed-off-by: Shreyansh Jain --- internal/gcs/guestconnection_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/internal/gcs/guestconnection_test.go b/internal/gcs/guestconnection_test.go index d8bc04aeea..a82e3851ca 100644 --- a/internal/gcs/guestconnection_test.go +++ b/internal/gcs/guestconnection_test.go @@ -51,7 +51,10 @@ func simpleGcsLoop(t *testing.T, rw io.ReadWriter) error { for { id, typ, b, err := readMessage(rw) if err != nil { - if errors.Is(err, io.EOF) || errors.Is(err, io.ErrClosedPipe) { + // EOF, ErrClosedPipe, and ErrUnexpectedEOF can all surface when + // the test's bridge closes the pipe mid-message during teardown. + // Treat them all as a clean shutdown of the fake guest. + if errors.Is(err, io.EOF) || errors.Is(err, io.ErrClosedPipe) || errors.Is(err, io.ErrUnexpectedEOF) { err = nil } return err @@ -154,7 +157,15 @@ func connectGcs(ctx context.Context, t *testing.T) *GuestConnection { c.Close() }() } - go simpleGcs(t, c) + // Join the fake-guest goroutine before the test returns so that any + // t.Error it makes during teardown lands inside the test scope, instead + // of panicking the runtime ("Fail in goroutine after Test... completed"). + done := make(chan struct{}) + go func() { + defer close(done) + simpleGcs(t, c) + }() + t.Cleanup(func() { <-done }) gcc := &GuestConnectionConfig{ Conn: s, Log: logrus.NewEntry(logrus.StandardLogger()),