Skip to content

test(runtime): eliminate server-startup port guessing + surface Run's error (follow-up to #529) #530

Description

@initializ-mk

Problem

Runner-startup tests guess a port via findFreePort (bind :0, close, return the port), then poll it. Two failure modes stem from guessing:

  1. Wrong-port poll (fixed in test(runtime): fix flaky server-readiness wait (port auto-increment race) #529): the freed port can be stolen before the runner binds, so server.Start auto-increments to port+1..+9 and the test polled the wrong port → opaque "server did not start within 5s". test(runtime): fix flaky server-readiness wait (port auto-increment race) #529 mitigated this by scanning the auto-increment window in waitForServer, but the scan has no server-identity check (under heavy parallel overlap it could match a neighbor's /healthz).
  2. Ephemeral-port exhaustion (rarer): running the whole package in a tight loop (10+ back-to-back startups) exhausts ephemeral ports so Run's bind fails outright. Not seen in single-pass CI, deliberately scoped out of test(runtime): fix flaky server-readiness wait (port auto-increment race) #529.

Compounding both: 6 of the 8 startup sites do go func() { _ = runner.Run(ctx) }(), swallowing Run's error — so a bind/startup failure is invisible and surfaces only as the readiness timeout (this is why the #529 flake "looked like a generic timeout" until the error was temporarily surfaced during diagnosis). Two files already use the good pattern (errCh <- runner.Run(ctx) / runErrCh <-).

Durable fix

  1. Stop guessing the port — have the runner accept a pre-bound net.Listener (or expose its resolved port after Start), so tests bind once and use exactly that address. Eliminates both the wrong-port race and the scan's identity ambiguity, and removes the exhaustion-from-guessing.
  2. Surface Run's error in every startup test — send it to a channel (template already in runner_test.go / tracing_runner_test.go) and have the readiness wait select on it, so a startup failure fails fast with the real cause instead of the 20s ceiling.

Scope

Test-harness only; no production behavior change (unless option 1 adds a small, opt-in WithListener/resolved-port accessor to the runner). Follow-up to #529 (which shipped the interim scan fix).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions