test: fix concurrent start test assertions

[nanookclaw]

Summary

Fixes #581 by removing testing.T assertions from the worker goroutines in TestServer_StartDev_ConcurrentStarts. Each concurrent start now returns an error, and the main test goroutine collects those errors and reports them with require.NoError.

The test still starts 40 dev server instances with a fixed worker pool of 6 concurrent executions. The replacement command helper runs server start-dev with a cancellable context and captured stdout/stderr, so failures from early command exit or cleanup are preserved in the final assertion message without calling FailNow-style methods outside the test goroutine.

Verification

• gofmt -w internal/temporalcli/commands.server_test.go
• git diff --check
• go test ./internal/temporalcli -run '^TestServer_StartDev_ConcurrentStarts$' -count=1

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Pull request overview

Refactors TestServer_StartDev_ConcurrentStarts to avoid calling testing.T/require assertions from worker goroutines by having each concurrent start return an error, then asserting in the main test goroutine. This aligns with Go’s testing constraints and should reduce flakiness/misreporting in the concurrent dev-server start integration test.

Changes:

• Reworked the concurrent-start worker logic to return errors from each run and aggregate them in the main goroutine.
• Replaced the command harness usage inside worker goroutines with a small helper that executes temporalcli.Execute using captured stdout/stderr.
• Updated the concurrency level to a fixed worker pool of 6 and improved error reporting to include command output.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chaptersix]

@nanookclaw I like the approach. let's use t.Context() instead of context.Background()

[chaptersix]

let's add func (h *CommandHarness) ExecuteWithContext(ctx context.Context, args ...string) *CommandResult {

so we can keep using the harness

[chaptersix]

how about something like this:

 ticker := time.NewTicker(200 * time.Millisecond)
  defer ticker.Stop()
  timeout := time.NewTimer(3 * time.Second)
  defer timeout.Stop()

  outer:
  for {
      select {
      case res := <-resCh:
          return concurrentStartCommandResultError("got early server result", res)
      case <-timeout.C:
          break outer
      case <-ticker.C:
          var err error
          cl, err = client.Dial(client.Options{HostPort: "127.0.0.1:" + port})
          if err == nil {
              break outer
          }
          lastDialErr = err
      }
  }

[nanookclaw]

Updated in ce9c1a5 to address the review comments:

• added CommandHarness.ExecuteWithContext(ctx, args...) while preserving existing Execute behavior
• changed the concurrent start test to use t.Context() and the harness
• replaced the readiness sleep loop with a ticker/timer select
• removed the bespoke concurrent-start command helper

Local verification:

• go test ./internal/temporalcli -run '^$'
• go test ./internal/temporalcli -run '^TestServer_StartDev_ConcurrentStarts$' -count=1