Skip to content

test(spanner): replace DB-API DDL system tests with mock server tests - #18683

Open
olavloite wants to merge 1 commit into
mainfrom
spanner-replace-dbapi-ddl-sys-tests
Open

olavloite wants to merge 1 commit into
mainfrom
spanner-replace-dbapi-ddl-sys-tests

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Replaces live-instance DDL system tests in test_dbapi.py with fast, deterministic in-memory mock server tests in test_dbapi_ddl.py.

Previously, test_dbapi.py dynamically created and dropped ephemeral tables (e.g. Singers, DdlExecute, DdlExecuteMany, JsonDetails) against live Cloud Spanner instances to verify client-side DDL buffering and flushing. Each table creation/drop triggered a long-running operation (LRO), adding ~1.5 to 2.0 minutes of latency and causing test flakiness.

Changes:

  • Add test_dbapi_ddl.py in tests/mockserver_tests/ with 9 mock server tests covering immediate autocommit DDL execution, non-autocommit DDL buffering, multi-statement DDL batches, DDL flushing before subsequent queries and DML, and client-side rejection of DDL in executemany().
  • Remove 7 redundant DDL tests from tests/system/test_dbapi.py.
  • Update test_autocommit_with_json_data in test_dbapi.py to reuse the existing all_types table with row cleanup instead of creating and dropping a dedicated table.
  • Eliminate all dynamic table churn in DB-API system tests, saving ~1.5 to 2.0 minutes in standard CI while executing the replacement mock tests in < 1 second.

@olavloite
olavloite requested a review from a team as a code owner October 13, 2026 18:04

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

Copy link
Copy Markdown
Contributor

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 migrates DB-API DDL tests from system tests to mock server tests, introducing a comprehensive suite in test_dbapi_ddl.py and removing redundant tests from test_dbapi.py. Additionally, test_autocommit_with_json_data is refactored to use an existing table. Feedback on the changes highlights a potential state leak in test_autocommit_with_json_data where self._conn.autocommit is modified but not restored, which could affect subsequent tests sharing the same connection.

Comment thread packages/google-cloud-spanner/tests/system/test_dbapi.py
Replaces live-instance DDL system tests in `test_dbapi.py` with fast,
deterministic in-memory mock server tests in `test_dbapi_ddl.py`.

Previously, `test_dbapi.py` dynamically created and dropped ephemeral
tables (e.g. `Singers`, `DdlExecute`, `DdlExecuteMany`, `JsonDetails`)
against live Cloud Spanner instances to verify client-side DDL buffering
and flushing. Each table creation/drop triggered a long-running operation
(LRO), adding ~1.5 to 2.0 minutes of latency and causing test flakiness.

Changes:
- Add `test_dbapi_ddl.py` in `tests/mockserver_tests/` with 9 mock server
  tests covering immediate autocommit DDL execution, non-autocommit DDL
  buffering, multi-statement DDL batches, DDL flushing before subsequent
  queries and DML, and client-side rejection of DDL in `executemany()`.
- Remove 7 redundant DDL tests from `tests/system/test_dbapi.py`.
- Update `test_autocommit_with_json_data` in `test_dbapi.py` to reuse the
  existing `all_types` table with row cleanup instead of creating and
  dropping a dedicated table.
- Eliminate all dynamic table churn in DB-API system tests, saving
  ~1.5 to 2.0 minutes in standard CI while executing the replacement
  mock tests in < 1 second.
@olavloite
olavloite force-pushed the spanner-replace-dbapi-ddl-sys-tests branch from 36fbef3 to 5c1c93b Compare October 14, 2026 07:33
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 14, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 14, 2026

This branch has not been deployed

No deployments
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.

2 participants