Skip to content

chore: clean up mock upstream in integration tests#190

Merged
pawbana merged 2 commits intomainfrom
pb/tests-refactor-mock-upstream
Feb 27, 2026
Merged

chore: clean up mock upstream in integration tests#190
pawbana merged 2 commits intomainfrom
pb/tests-refactor-mock-upstream

Conversation

@pawbana
Copy link
Contributor

@pawbana pawbana commented Feb 24, 2026

Adds MockUpstream test helper that mocks AI provider upstream.
Updates existing tests to use new MockUpstream helper cleaning up custom mocks per test.

Copy link
Contributor Author

pawbana commented Feb 24, 2026

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

@pawbana pawbana force-pushed the pb/tests-refactor-mock-upstream branch 10 times, most recently from 8dee463 to 6613c36 Compare February 26, 2026 19:05
@pawbana pawbana force-pushed the pb/tests-refactor-mock-upstream branch from 6613c36 to ea82069 Compare February 26, 2026 19:30
@pawbana pawbana marked this pull request as ready for review February 26, 2026 19:38
Copy link
Collaborator

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this, great improvement @pawbana!
A couple minor points, but otherwise good to go.

Comment on lines -1250 to -1254
require.Contains(t, files, fixtureRequest)
require.Contains(t, files, fixtureStreamingResponse)
require.Contains(t, files, fixtureNonStreamingResponse)
require.Contains(t, files, fixtureStreamingToolResponse)
require.Contains(t, files, fixtureNonStreamingToolResponse)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're losing the explicit checks here which are probably fine, but might make it hard to reason about when we have a nil request/response.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contains response checks are are done in MockUpstream.handle method but there is no check for Request.
I'll add Contains check into getter methods. Thanks for catching that.

Copy link
Contributor Author

pawbana commented Feb 27, 2026

Merge activity

  • Feb 27, 2:49 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Feb 27, 2:49 PM UTC: @pawbana merged this pull request with Graphite.

@pawbana pawbana merged commit f5866a0 into main Feb 27, 2026
5 of 6 checks passed
@pawbana pawbana linked an issue Mar 5, 2026 that may be closed by this pull request
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create proper testing package

2 participants