amadan

carlos / platform Public

tests: every console test joins its server's detached work (fix round 4, item 2b)

The shared console fixture (newConsoleFixtureKeyed) calls Server.Shutdown
with a 15 s bound in its cleanup, registered after NewServer so it runs
before the store and the listener close; the e2e job world does the same
before every restart and at its end. A detached index cleanup can no
longer outlive the test that started it. (The bridge witness's NewServer
serves no request, so it starts no detached work.)

Witness: TestFixtureJoinsDetachedWork (a return whose cleanup stalls, in
a subtest; after the subtest the cleanup has returned). RED: "the test
ended with its detached index cleanup still running".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude d91ad106b5eef2af12b68e1c4785abe3277509c5 parent 1355a2d
3 files changed, +67 −0
  • internal/console/offlinereturn_test.go +40 −0
  • internal/console/server_test.go +10 −0
  • internal/e2e/selfserve/job_world_test.go +17 −0
diff --git a/internal/console/offlinereturn_test.go b/internal/console/offlinereturn_test.go
index 419cb8abe..950d1d2d9 100644
--- a/internal/console/offlinereturn_test.go
+++ b/internal/console/offlinereturn_test.go
@@ -9,6 +9,7 @@ import (
"sort"
"strings"
"sync"
+ "sync/atomic"
"testing"
"time"
@@ -836,3 +837,42 @@ func TestReturnIndexCleanupPanicIsContained(t *testing.T) {
t.Fatalf("the entry the sweep owns is gone: %v", err)
}
}
+
+// exitStore stalls every read of key until its context is done, and
+// records when such a read returns.
+type exitStore struct {
+ release.Store
+ key string
+ exited atomic.Bool
+}
+
+func (e *exitStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if key == e.key {
+ <-ctx.Done()
+ e.exited.Store(true)
+ return nil, "", ctx.Err()
+ }
+ return e.Store.Get(ctx, key)
+}
+
+// TestFixtureJoinsDetachedWork (fix round 4, item 2b): the shared console
+// fixture shuts its server down in its cleanup, so a test's detached work
+// (the return door's index cleanup) is joined before the test ends and
+// never outlives it.
+func TestFixtureJoinsDetachedWork(t *testing.T) {
+ defer func(d time.Duration) { offlineReturnCleanupTimeout = d }(offlineReturnCleanupTimeout)
+ offlineReturnCleanupTimeout = time.Second
+ var es *exitStore
+ t.Run("a return whose cleanup stalls", func(t *testing.T) {
+ w := newReturnWorld(t, nil)
+ w.report("uk-box", "uk-box", "", w.request(), time.Now(), w.ready())
+ es = &exitStore{Store: w.fx.rel, key: moves.ReturnIndexKey("uk", offlineHost, returnFwd)}
+ w.fx.srv.cfg.Releases = es
+ if code, out := w.req(http.MethodPost, "/stage", `{"region":"uk","return_of":"`+returnFwd+`"}`); code != http.StatusAccepted {
+ t.Fatalf("stage = %d %s", code, out)
+ }
+ })
+ if !es.exited.Load() {
+ t.Fatal("the test ended with its detached index cleanup still running")
+ }
+}
diff --git a/internal/console/server_test.go b/internal/console/server_test.go
index 9a0af8938..95f4008ce 100644
--- a/internal/console/server_test.go
+++ b/internal/console/server_test.go
@@ -78,6 +78,16 @@ func newConsoleFixtureKeyed(t *testing.T, withKey bool) *consoleFixture {
if err != nil {
t.Fatal(err)
}
+ // Every test joins the work its server started detached (the return
+ // door's index cleanup) before its store and listener close: cleanups
+ // run last-registered first.
+ t.Cleanup(func() {
+ ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+ defer cancel()
+ if err := srv.Shutdown(ctx); err != nil {
+ t.Errorf("the server's detached work outlived the test: %v", err)
+ }
+ })
// Fixture route hosts don't resolve; keep the not-reporting verdict quick.
srv.instances.timeout = 100 * time.Millisecond
// paul owns the built-in ops account (created by NewServer's backfill),
diff --git a/internal/e2e/selfserve/job_world_test.go b/internal/e2e/selfserve/job_world_test.go
index cd8374bbf..96ee871f7 100644
--- a/internal/e2e/selfserve/job_world_test.go
+++ b/internal/e2e/selfserve/job_world_test.go
@@ -317,6 +317,7 @@ func newJobWorld(t *testing.T) *jobWorld {
t.Cleanup(func() {
console.TestHooks.Probe(nil)
console.TestHooks.Fault(nil)
+ w.shutdown()
if w.cst != nil {
w.cst.Close()
}
@@ -355,12 +356,28 @@ func (w *jobWorld) open() {
// again, a new console process under the same name.
func (w *jobWorld) restart() {
w.t.Helper()
+ w.shutdown()
if err := w.cst.Close(); err != nil {
w.t.Fatal(err)
}
w.open()
}
+// shutdown joins the work the console started detached (the return
+// door's index cleanup), as a console that stops would, before its store
+// closes.
+func (w *jobWorld) shutdown() {
+ w.t.Helper()
+ if w.srv == nil {
+ return
+ }
+ ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+ defer cancel()
+ if err := w.srv.Shutdown(ctx); err != nil {
+ w.t.Errorf("the console's detached work outlived it: %v", err)
+ }
+}
+
// fault is the console's crash points: the armed one crashes the tick.
func (w *jobWorld) fault(point string) {
w.hits[point]++