refactor: create internal/integrationtest package infrastructure by pawbana · Pull Request #206 · coder/aibridge

pawbana · 2026-03-06T13:01:04Z

Refactor integration tests into internal/integrationtest package

This PR consolidates all integration tests into a dedicated internal/integrationtest package.

Move test infrastructure to internal/integrationtest

Move upstream.go and mockmcp.go from internal/testutil to internal/integrationtest
Create setupbridge.go with newBridgeTestServer and functional options pattern
Create helpers.go with shared configuration builders and utilities

Migrate all integration tests

Move integration test files from root to internal/integrationtest/:

bridge_integration_test.go → bridge_test.go
metrics_integration_test.go → metrics_test.go
trace_integration_test.go → trace_test.go
responses_integration_test.go → responses_test.go
circuit_breaker_integration_test.go → circuit_breaker_test.go
apidump_integration_test.go → apidump_test.go

Replace boilerplate with functional options

Convert all aibridge.NewRequestBridge call sites to use newBridgeTestServer with options:

Replace inline bridge setup patterns with withProvider(), withMetrics(), withMCP(), etc.
Eliminate configureFunc closures and repetitive server setup code
Standardize actor context setup with withActor()

Consolidate test utilities

Remove duplicate helper functions across test files
Standardize request creation with bridgeTestServer.makeRequest()
Use consistent constants (apiKey, defaultActorID, path constants)
Centralize provider configuration builders

pawbana · 2026-03-06T13:01:19Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

- Move upstream.go and mockmcp.go from internal/testutil to internal/integrationtest - Create bridge.go with NewBridgeTestServer, NewLogger, DefaultActorID, DefaultTracer - Create requests.go with shared request/config helpers (promoted from test files) - internal/testutil retains only lightweight mocks (MockRecorder, MockProvider)

…egrationtest/ - Move metrics_integration_test.go → internal/integrationtest/metrics_test.go - Move trace_integration_test.go → internal/integrationtest/trace_test.go - Change package from aibridge_test to integrationtest_test - Replace all newTestSrv calls with integrationtest.NewBridgeTestServer - Delete the newTestSrv helper function entirely - Update all imports: testutil symbols → integrationtest where moved - Replace local helpers (apiKey, userID, openaiCfg, etc.) with exported versions - All former newTestSrv sites use WithWrappedRecorder() to preserve behavior - Remove duplicate testBedrockCfg (already in bridge_test.go)

… tests to internal/integrationtest/ Move the following files from the repo root: - responses_integration_test.go → internal/integrationtest/responses_test.go - circuit_breaker_integration_test.go → internal/integrationtest/circuit_breaker_test.go - apidump_integration_test.go → internal/integrationtest/apidump_test.go Changes in all files: - Package: aibridge_test → integrationtest_test - Replace newTestSrv/NewRequestBridge with integrationtest.NewBridgeTestServer - Replace testutil.* moved symbols with integrationtest.* - Replace local helper refs (openaiCfg, apiKey, userID, etc.) with integrationtest.OpenAICfg, integrationtest.APIKey, integrationtest.DefaultActorID - Delete local createOpenAIResponsesReq (now in requests.go) - Keep local helpers specific to each file

…lerplate with NewBridgeTestServer - Convert all 18 remaining aibridge.NewRequestBridge call sites to use NewBridgeTestServer with functional options - Replace all configureFunc closures (4 different signatures, ~12 inline closures) with providerFn field + NewBridgeTestServer in test body - Convert setupInjectedToolTest to use NewBridgeTestServer internally - Fix stale recorderClient references to ts.Recorder - Remove unused imports (slog, aibcontext, testutil from responses_test) - Delete bridge_integration_test.go from root Net result: -424 lines of boilerplate, root directory down to 6 files. All tests pass (go test ./... -count=1).

Change package from integrationtest_test to integrationtest, remove the self-import, and strip all integrationtest. prefixes from identifiers.

Switch all _test.go files from package integrationtest_test to package integrationtest. Since nothing outside this directory imports the package, all exported symbols are unexported: - Types: BridgeTestServer → bridgeTestServer, MockMCP → mockMCP, MockUpstream → mockUpstream, UpstreamResponse → upstreamResponse, etc. - Functions: NewBridgeTestServer → newBridgeTestServer, CreateAnthropicMessagesReq → createAnthropicMessagesReq, etc. - Options: WithMetrics → withMetrics, WithTracer → withTracer, etc. - Constants: APIKey → apiKey, DefaultActorID → defaultActorID, etc. - Methods: GetCallsByTool → getCallsByTool, ReceivedRequests → receivedRequests, etc. This eliminates the integrationtest. prefix from all call sites and removes the artificial export surface. All tests pass (go test ./... -count=1).

…p.Response return type makeRequest now performs the HTTP request internally and returns *http.Response directly. Remove the intermediate http.Client creation, client.Do(req), and require.NoError(t, err) calls at all 5 call sites.

makeRequest now returns *http.Response directly instead of *http.Request. Updated all 6 call sites: - Removed intermediate http.DefaultClient.Do / client.Do calls - Removed require.NoError for the HTTP request error - Collapsed header mutation site to pass http.Header as extra arg

makeRequest now returns *http.Response directly (performs HTTP request internally). Updated all 14 call sites: - Changed req := to resp := - Removed http.DefaultClient.Do(req) and require.NoError(t, err) lines - Removed client := &http.Client{} and client.Do(req) patterns - Converted header mutation case to pass http.Header as extra arg

…etions to use newBridgeTestServer + test some names cleanup

dannykopping

Love the more concise tests! Thanks for doing this.

I would've appreciated if you'd stacked these changes. It's a huge PR with not only deduplication but also logical changes, and this makes it very difficult to review effectively. I'm concerned that some things will slip through, but in general it looks good.

dannykopping · 2026-03-09T15:31:13Z

internal/integrationtest/setupbridge.go

+	for _, h := range header {
+		for k, vals := range h {
+			for _, v := range vals {
+				req.Header.Set(k, v)


This should be Header.Add right? Otherwise the last one always wins?

dannykopping · 2026-03-09T15:32:28Z

internal/integrationtest/bridge_test.go

 			require.Len(t, received, 1)
 			require.Equal(t, tc.expectedUpstreamPath, received[0].Path)
-			require.Contains(t, received[0].Header.Get(provider.AuthHeader()), apiKey)
+			require.Contains(t, received[0].Header.Get(tc.authHeader), apiKey)


You're hardcoding the auth header now instead of using provider.AuthHeader; why?

After refactor provider variable is no longer here.
This change is done because of it but in general hardcoding test inputs / expected outputs, in my experience was desired.

I'd prefer we didn't define this upstream requirement in multiple places.

But, I can see why you did it and change it in the provider will cause the tests to fail, so it's not a big deal.

…derName from TestFallthrough

pawbana force-pushed the pb/testing-cleanup-new-request-bridge branch from f45d4d2 to e95970f Compare March 6, 2026 13:04

pawbana marked this pull request as ready for review March 6, 2026 15:15

pawbana requested review from dannykopping and ssncferreira March 6, 2026 15:41

pawbana added 25 commits March 6, 2026 16:41

refactor: convert bridge_test.go from external to internal test package

35ba107

Change package from integrationtest_test to integrationtest, remove the self-import, and strip all integrationtest. prefixes from identifiers.

refactor: split bridge_test.go into per-interceptor test files

975e3e3

fixup: apply unexport renames to remaining helper files

fb43a44

remove providerFunc

4315e2f

merge tests back into bridge_test.go

ae65a55

bridge.go -> setupbridge.go + helpers.go

fc1839d

remove withWrappedRecorder

bad3ef4

remove actorID arg from setupInjectedToolTest

3ccddc6

use bridgeTestServer.newRequest

c2b580c

bridgeServer.makeRequest + tracerSetup + TestSessionIDTracking cleanup

32de2aa

cleanup

6d96370

reorder tests in bridge_test.go to original order

24ec8fc

remove not needed withLogger

079884b

fix comments

7f70b94

rename

4870b9d

formatting fix

0eba5b8

reorder to previous

3d5bd76

pawbana added 3 commits March 6, 2026 16:41

recorder cleanup

70fea88

MockRecorder Total*Tokens

80994d6

add Body.Close() to bridgeServer.makeRequest + some test names cleanup

e235208

pawbana force-pushed the pb/testing-cleanup-new-request-bridge branch from fd1cc42 to e235208 Compare March 6, 2026 16:42

pawbana added 2 commits March 6, 2026 17:12

changed TestResponsesParallelToolsOverwritten and TestOpenAIChatCompl…

fcf0f05

…etions to use newBridgeTestServer + test some names cleanup

fmt fix

69ed4d7

dannykopping approved these changes Mar 9, 2026

View reviewed changes

pawbana mentioned this pull request Mar 9, 2026

feat: add client label to InterceptionCount, PromptCount and TokenUseCount metrics #209

Open

review 1: fixed setting headers in makeRequest, removed unneded provi…

b24c6fc

…derName from TestFallthrough

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: create internal/integrationtest package infrastructure#206

refactor: create internal/integrationtest package infrastructure#206
pawbana wants to merge 31 commits intomainfrom
pb/testing-cleanup-new-request-bridge

pawbana commented Mar 6, 2026 •

edited

Loading

Uh oh!

pawbana commented Mar 6, 2026 •

edited

Loading

Uh oh!

dannykopping left a comment

Uh oh!

dannykopping Mar 9, 2026

Uh oh!

dannykopping Mar 9, 2026

Uh oh!

pawbana Mar 9, 2026

Uh oh!

dannykopping Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pawbana commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactor integration tests into internal/integrationtest package

Move test infrastructure to internal/integrationtest

Migrate all integration tests

Replace boilerplate with functional options

Consolidate test utilities

Uh oh!

pawbana commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dannykopping left a comment

Choose a reason for hiding this comment

Uh oh!

dannykopping Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

dannykopping Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

pawbana Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

dannykopping Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pawbana commented Mar 6, 2026 •

edited

Loading

pawbana commented Mar 6, 2026 •

edited

Loading