Skip to content

test(spanner): update mock server tests to check for inline begin#16656

Merged
parthea merged 3 commits intomainfrom
fix-spanner-mock-tests-main
Apr 15, 2026
Merged

test(spanner): update mock server tests to check for inline begin#16656
parthea merged 3 commits intomainfrom
fix-spanner-mock-tests-main

Conversation

@chalmerlowe
Copy link
Copy Markdown
Contributor

@chalmerlowe chalmerlowe commented Apr 15, 2026

This PR updates the sqlalchemy-spanner mock server tests to account for the dialect now inlining BeginTransaction requests into the first statement execution.

Changes

  • Updated expected request counts in multiple tests (typically reducing by 1).
  • Removed assertions for separate BeginTransactionRequest where applicable.
  • Added assertions to verify that the first statement request contains transaction.begin options.
  • Updated assertions to verify that subsequent requests use the transaction ID returned by the mock server.

These changes resolve the off-by-one failures in request counts across multiple mock server tests.

@chalmerlowe chalmerlowe requested a review from a team as a code owner April 15, 2026 13:48
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the create_engine method in the mock server test base to use NullPool, which disables connection pooling and ensures that a new session is created for every test. I have no feedback to provide as there were no review comments to evaluate.

@olavloite olavloite changed the title test(spanner): disable pooling in mock server tests test(spanner): update mock server tests to check for inline begin Apr 15, 2026
@parthea parthea merged commit ef62df4 into main Apr 15, 2026
30 checks passed
@parthea parthea deleted the fix-spanner-mock-tests-main branch April 15, 2026 15:15
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.

3 participants