From 7eafdb1e9a353d4bd9a3e368a0400e8451a39180 Mon Sep 17 00:00:00 2001 From: Caleb Piekstra Date: Wed, 30 Sep 2026 18:12:15 -0400 Subject: [PATCH] fix(shared): give each API client its own HTTP transport client.New used http.DefaultTransport. httptest.Server.Close calls CloseIdleConnections on http.DefaultTransport, so any parallel test that shut down its server could close another test's connection. net/http returns a connection to the idle pool before it hands a bodiless response (such as a 204) to the waiting request; closing it in that window fails the request with "HTTP/1.x transport connection broken: http: CloseIdleConnections called". That is the intermittent failure in the jtk delete, remote link, field, field context and sprint tests. Each client now uses a clone of http.DefaultTransport, so other code closing the default pool's idle connections cannot affect it. The delete tests now print the per-issue errors from stderr when runDelete fails, since its returned error only says "already reported". --- shared/client/client.go | 16 +++++++++++++- shared/client/client_test.go | 13 +++++++++++ tools/jtk/internal/cmd/issues/delete_test.go | 23 ++++++++++++++------ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/shared/client/client.go b/shared/client/client.go index 084e81f6..c871b7cd 100644 --- a/shared/client/client.go +++ b/shared/client/client.go @@ -62,13 +62,27 @@ func New(baseURL, email, apiToken string, opts *Options) *Client { BaseURL: baseURL, AuthHeader: authHeader, HTTPClient: &http.Client{ - Timeout: timeout, + Timeout: timeout, + Transport: newTransport(), }, Verbose: verbose, VerboseOut: verboseOut, } } +// newTransport returns a connection pool owned by one Client. Sharing +// http.DefaultTransport lets unrelated code that calls its +// CloseIdleConnections (httptest.Server.Close does, for example) close a +// connection that net/http has already returned to the idle pool while the +// response for a bodiless request (such as a 204) is still being handed to +// the caller, which fails that request with "CloseIdleConnections called". +func newTransport() http.RoundTripper { + if t, ok := http.DefaultTransport.(*http.Transport); ok { + return t.Clone() + } + return http.DefaultTransport +} + // Do executes an HTTP request with the given method, path, and optional body. // // The path can be either relative to the BaseURL (e.g., "/rest/api/3/issue") diff --git a/shared/client/client_test.go b/shared/client/client_test.go index 4dc604e7..6ebfac1f 100644 --- a/shared/client/client_test.go +++ b/shared/client/client_test.go @@ -65,6 +65,19 @@ func TestNew(t *testing.T) { t.Error("VerboseOut not set correctly") } }) + + t.Run("owns its transport", func(t *testing.T) { + t.Parallel() + a := New("https://example.atlassian.net", "user@example.com", "token", nil) + b := New("https://example.atlassian.net", "user@example.com", "token", nil) + + if a.HTTPClient.Transport == nil || a.HTTPClient.Transport == http.DefaultTransport { + t.Error("Transport should not be http.DefaultTransport") + } + if a.HTTPClient.Transport == b.HTTPClient.Transport { + t.Error("each Client should have its own Transport") + } + }) } func TestClient_Do(t *testing.T) { diff --git a/tools/jtk/internal/cmd/issues/delete_test.go b/tools/jtk/internal/cmd/issues/delete_test.go index 7b85d229..055051ad 100644 --- a/tools/jtk/internal/cmd/issues/delete_test.go +++ b/tools/jtk/internal/cmd/issues/delete_test.go @@ -15,6 +15,15 @@ import ( "github.com/open-cli-collective/jira-ticket-cli/internal/cmd/root" ) +// requireDeleted fails with the per-issue errors runDelete wrote to stderr, +// since its returned error only carries the failure count. +func requireDeleted(t *testing.T, err error, stderr *bytes.Buffer) { + t.Helper() + if err != nil { + t.Fatalf("unexpected error: %v\nstderr:\n%s", err, stderr.String()) + } +} + func TestRunDelete_SingleIssue(t *testing.T) { t.Parallel() @@ -32,7 +41,7 @@ func TestRunDelete_SingleIssue(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-123"}, true) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deleted PROJ-123\n") testutil.Equal(t, stderr.String(), "") } @@ -54,7 +63,7 @@ func TestRunDelete_MultipleIssues(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-1", "PROJ-2", "PROJ-3"}, true) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deleted PROJ-1\nDeleted PROJ-2\nDeleted PROJ-3\n") testutil.Equal(t, stderr.String(), "") } @@ -146,7 +155,7 @@ func TestRunDelete_NonInteractive_WithForce_Proceeds(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-123"}, true) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deleted PROJ-123\n") } @@ -165,7 +174,7 @@ func TestRunDelete_PromptDeclined(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-123"}, false) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deletion cancelled.\n") testutil.Contains(t, stderr.String(), "permanently delete issue PROJ-123") } @@ -185,7 +194,7 @@ func TestRunDelete_BatchPromptDeclined(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-1", "PROJ-2", "PROJ-3"}, false) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Contains(t, stderr.String(), "3 issues") testutil.Equal(t, stdout.String(), "Deletion cancelled.\n") } @@ -210,7 +219,7 @@ func TestRunDelete_PromptAccepted(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-123"}, false) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deleted PROJ-123\n") } @@ -234,7 +243,7 @@ func TestRunDelete_BatchPromptAccepted(t *testing.T) { opts.SetAPIClient(client) err = runDelete(context.Background(), opts, []string{"PROJ-1", "PROJ-2"}, false) - testutil.RequireNoError(t, err) + requireDeleted(t, err, &stderr) testutil.Equal(t, stdout.String(), "Deleted PROJ-1\nDeleted PROJ-2\n") }