Merged
Make CLI transport timeout tests deterministic #171
urandonAdd on 27 days ago
Make CLI transport timeout tests deterministic #171
Merged
Merged fromcodex/issue-187-deterministic-cli-timeoutintomain
Total: 1 files changed+93-6
| @@ -8,9 +8,11 @@ import ( | |||
| 8 | "errors" | 8 | "errors" |
| 9 | "fmt" | 9 | "fmt" |
| 10 | "io" | 10 | "io" |
| 11 | + "net/http" | ||
| 11 | "os" | 12 | "os" |
| 12 | "path/filepath" | 13 | "path/filepath" |
| 13 | "strings" | 14 | "strings" |
| 15 | + "sync/atomic" | ||
| 14 | "testing" | 16 | "testing" |
| 15 | "time" | 17 | "time" |
| 16 | 18 | ||
| @@ -626,7 +628,6 @@ func TestCLIStartupPlanSelectsLiveProvider(t *testing.T) { | |||
| 626 | {name: "SCN-CLI-ERROR-OUTPUT-MALFORMED-JSON", mode: testnet.GitCodeAPIFailureMalformedJSON, want: string(diagnostics.CodeSchemaDecode)}, | 628 | {name: "SCN-CLI-ERROR-OUTPUT-MALFORMED-JSON", mode: testnet.GitCodeAPIFailureMalformedJSON, want: string(diagnostics.CodeSchemaDecode)}, |
| 627 | {name: "SCN-CLI-ERROR-OUTPUT-SCHEMA-MISMATCH", mode: testnet.GitCodeAPIFailureSchemaMismatch, want: string(diagnostics.CodeSchemaDecode)}, | 629 | {name: "SCN-CLI-ERROR-OUTPUT-SCHEMA-MISMATCH", mode: testnet.GitCodeAPIFailureSchemaMismatch, want: string(diagnostics.CodeSchemaDecode)}, |
| 628 | {name: "SCN-CLI-ERROR-OUTPUT-PARTIAL-RESPONSE", mode: testnet.GitCodeAPIFailurePartial, want: string(diagnostics.CodeSchemaDecode)}, | 630 | {name: "SCN-CLI-ERROR-OUTPUT-PARTIAL-RESPONSE", mode: testnet.GitCodeAPIFailurePartial, want: string(diagnostics.CodeSchemaDecode)}, |
| 629 | - {name: "SCN-CLI-ERROR-OUTPUT-TIMEOUT", mode: testnet.GitCodeAPIFailureTimeout, want: string(diagnostics.CodeLiveTransportFailure)}, | ||
| 630 | {name: "SCN-CLI-ERROR-OUTPUT-500", mode: testnet.GitCodeAPIFailure500, want: string(diagnostics.CodeLiveTransportFailure)}, | 631 | {name: "SCN-CLI-ERROR-OUTPUT-500", mode: testnet.GitCodeAPIFailure500, want: string(diagnostics.CodeLiveTransportFailure)}, |
| 631 | } { | 632 | } { |
| 632 | t.Run(tt.name, func(t *testing.T) { | 633 | t.Run(tt.name, func(t *testing.T) { |
| @@ -635,11 +636,6 @@ func TestCLIStartupPlanSelectsLiveProvider(t *testing.T) { | |||
| 635 | cachePath := filepath.Join(t.TempDir(), "cache.db") | 636 | cachePath := filepath.Join(t.TempDir(), "cache.db") |
| 636 | src := newTestSource(t) | 637 | src := newTestSource(t) |
| 637 | src.env[config.EnvToken] = "test-token" | 638 | src.env[config.EnvToken] = "test-token" |
| 638 | - if tt.mode == testnet.GitCodeAPIFailureTimeout { | ||
| 639 | - configPath := filepath.Join(t.TempDir(), "config.json") | ||
| 640 | - src.env[config.EnvConfigPath] = configPath | ||
| 641 | - src.files[configPath] = []byte(`{"default_timeout":"1ms","max_retries":0}`) | ||
| 642 | - } | ||
| 643 | addRepoForStartupTest(t, cachePath, server.BaseURL()) | 639 | addRepoForStartupTest(t, cachePath, server.BaseURL()) |
| 644 | 640 | ||
| 645 | var stdout, stderr bytes.Buffer | 641 | var stdout, stderr bytes.Buffer |
| @@ -657,6 +653,41 @@ func TestCLIStartupPlanSelectsLiveProvider(t *testing.T) { | |||
| 657 | }) | 653 | }) |
| 658 | } | 654 | } |
| 659 | 655 | ||
| 656 | + for _, cancelled := range []bool{false, true} { | ||
| 657 | + name := "SCN-CLI-ERROR-OUTPUT-TIMEOUT" | ||
| 658 | + if cancelled { | ||
| 659 | + name = "SCN-CLI-ERROR-OUTPUT-TRANSPORT-CANCELLED" | ||
| 660 | + } | ||
| 661 | + t.Run(name, func(t *testing.T) { | ||
| 662 | + var attempts atomic.Int32 | ||
| 663 | + // Inject only at Client.Do's transport boundary. Startup must reach | ||
| 664 | + // HTTP with a live context; no whole-command deadline or sleep races. | ||
| 665 | + withStartupTransport(t, startupRoundTripper(func(req *http.Request) (*http.Response, error) { | ||
| 666 | + attempts.Add(1) | ||
| 667 | + if req.Context().Err() != nil || req.Method != http.MethodGet || req.URL.Host != "example.invalid" || req.URL.Path != "/api/v5/repos/owner-a/repo-a/issues" { | ||
| 668 | + t.Error("transport entered with cancelled context or unexpected request") | ||
| 669 | + } | ||
| 670 | + if cancelled { | ||
| 671 | + return nil, context.Canceled | ||
| 672 | + } | ||
| 673 | + return nil, context.DeadlineExceeded | ||
| 674 | + })) | ||
| 675 | + src := newTestSource(t) | ||
| 676 | + src.env[config.EnvToken] = "test-token" | ||
| 677 | + configPath := filepath.Join(t.TempDir(), "config.json") | ||
| 678 | + src.env[config.EnvConfigPath] = configPath | ||
| 679 | + src.files[configPath] = []byte(`{"max_retries":0}`) | ||
| 680 | + cachePath := filepath.Join(t.TempDir(), "cache.db") | ||
| 681 | + addRepoForStartupTest(t, cachePath, "https://example.invalid") | ||
| 682 | + var stdout, stderr bytes.Buffer | ||
| 683 | + code := run([]string{"sync", "--live", "--cache-path", cachePath, "--repo", "fixture-a", "--issues"}, strings.NewReader(""), &stdout, &stderr, src) | ||
| 684 | + out := stdout.String() + stderr.String() | ||
| 685 | + if code == 0 || attempts.Load() != 1 || !strings.Contains(out, "failure_class: live_transport_failure") || !strings.Contains(out, "http_attempted: true") { | ||
| 686 | + t.Fatalf("exit=%d attempts=%d: transport failure with one HTTP attempt required", code, attempts.Load()) | ||
| 687 | + } | ||
| 688 | + }) | ||
| 689 | + } | ||
| 690 | + | ||
| 660 | t.Run("SCN-MOCKAPI-OFFLINE-SYNC-NO-HTTP", func(t *testing.T) { | 691 | t.Run("SCN-MOCKAPI-OFFLINE-SYNC-NO-HTTP", func(t *testing.T) { |
| 661 | server := testnet.NewGitCodeAPIServer(t) | 692 | server := testnet.NewGitCodeAPIServer(t) |
| 662 | 693 | ||
| @@ -943,6 +974,62 @@ func TestCLIStartupPlanSelectsLiveProvider(t *testing.T) { | |||
| 943 | }) | 974 | }) |
| 944 | } | 975 | } |
| 945 | 976 | ||
| 977 | +type startupRoundTripper func(*http.Request) (*http.Response, error) | ||
| 978 | + | ||
| 979 | +func (f startupRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| 980 | + return f(req) | ||
| 981 | +} | ||
| 982 | + | ||
| 983 | +// Entrypoint tests are serial (they also replace cliRoute). Never use this | ||
| 984 | +// process-wide test seam in a parallel test; restore before the next subtest. | ||
| 985 | +func withStartupTransport(t *testing.T, transport http.RoundTripper) { | ||
| 986 | + t.Helper() | ||
| 987 | + old := http.DefaultTransport | ||
| 988 | + http.DefaultTransport = transport | ||
| 989 | + t.Cleanup(func() { http.DefaultTransport = old }) | ||
| 990 | +} | ||
| 991 | + | ||
| 992 | +func TestCLIStartupPreHTTPCancellation(t *testing.T) { | ||
| 993 | + for _, deadline := range []bool{false, true} { | ||
| 994 | + name := "cancelled" | ||
| 995 | + if deadline { | ||
| 996 | + name = "deadline_expired" | ||
| 997 | + } | ||
| 998 | + t.Run(name, func(t *testing.T) { | ||
| 999 | + src := newTestSource(t) | ||
| 1000 | + src.env[config.EnvToken] = "test-token" | ||
| 1001 | + configPath := filepath.Join(t.TempDir(), "config.json") | ||
| 1002 | + src.env[config.EnvConfigPath] = configPath | ||
| 1003 | + src.files[configPath] = []byte(`{"max_retries":0}`) | ||
| 1004 | + cachePath := filepath.Join(t.TempDir(), "cache.db") | ||
| 1005 | + addRepoForStartupTest(t, cachePath, "https://example.invalid") | ||
| 1006 | + var attempts atomic.Int32 | ||
| 1007 | + withStartupTransport(t, startupRoundTripper(func(*http.Request) (*http.Response, error) { | ||
| 1008 | + attempts.Add(1) | ||
| 1009 | + return nil, errors.New("unexpected HTTP after startup cancellation") | ||
| 1010 | + })) | ||
| 1011 | + old := cliRoute | ||
| 1012 | + t.Cleanup(func() { cliRoute = old }) | ||
| 1013 | + cliRoute = func(ctx context.Context, args []string, stdout, stderr io.Writer, deps StartupDeps) int { | ||
| 1014 | + // Fail the CLI startup read, after the entrypoint's own config | ||
| 1015 | + // load. This is a startup failure, not a provider cancellation. | ||
| 1016 | + if deadline { | ||
| 1017 | + src.readErr[configPath] = context.DeadlineExceeded | ||
| 1018 | + } else { | ||
| 1019 | + src.readErr[configPath] = context.Canceled | ||
| 1020 | + } | ||
| 1021 | + return old(ctx, args, stdout, stderr, deps) | ||
| 1022 | + } | ||
| 1023 | + var stdout, stderr bytes.Buffer | ||
| 1024 | + code := run([]string{"sync", "--live", "--cache-path", cachePath, "--repo", "fixture-a", "--issues"}, strings.NewReader(""), &stdout, &stderr, src) | ||
| 1025 | + out := stdout.String() + stderr.String() | ||
| 1026 | + if code == 0 || attempts.Load() != 0 || !strings.Contains(out, "failure_class: configuration_error") || !strings.Contains(out, "http_attempted: false") { | ||
| 1027 | + t.Fatalf("exit=%d attempts=%d: pre-HTTP configuration failure required", code, attempts.Load()) | ||
| 1028 | + } | ||
| 1029 | + }) | ||
| 1030 | + } | ||
| 1031 | +} | ||
| 1032 | + | ||
| 946 | func addRepoForStartupTest(t *testing.T, cachePath, baseURL string) { | 1033 | func addRepoForStartupTest(t *testing.T, cachePath, baseURL string) { |
| 947 | t.Helper() | 1034 | t.Helper() |
| 948 | addRepoForStartupTestWithScopes(t, cachePath, baseURL, "issues,wiki") | 1035 | addRepoForStartupTestWithScopes(t, cachePath, baseURL, "issues,wiki") |