Skip to content

test(spanner): consolidate admin system tests to reduce DDL and database churn - #18693

Merged
olavloite merged 3 commits into
mainfrom
spanner-simplify-db-admin-tests
Oct 26, 2026
Merged

olavloite merged 3 commits into
mainfrom
spanner-simplify-db-admin-tests

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Consolidate administrative system tests in test_database_api.py to eliminate redundant database creations, long-running operations (LROs), and schema churn:

  1. Consolidate Create & Update Tests:
    • Combine PITR database creation and subsequent DDL update into a single test (test_create_and_update_database_pitr), asserting both client properties and Information Schema catalog values.
    • Combine default leader creation and subsequent update into a single multi-region database test (test_create_and_update_database_with_default_leader).
    • Merge test_update_ddl_w_operation_id into test_create_database_and_update_ddl.
  2. Consolidate Database Roles & Permissions:
    • Merge test_create_role_grant_access_success and test_list_database_role_success into test_create_role_grant_access_and_list_roles.
    • Strip the 20-table schema down to only the single required table (contacts).
    • Eliminate redundant teardown DDL (revoking permissions and dropping roles) immediately prior to database deletion.
    • Fix a lazy evaluation bug where the parent role query was never iterated and therefore never dispatched over the wire.
  3. Remove Pure Server-Side DDL Parser Tests:
    • Drop test_create_database_pitr_invalid_retention_period and test_update_ddl_w_pitr_invalid, which tested Spanner backend syntax rejection of '0d' rather than client validation logic.
  4. Fix Multi-Region Database Timeout:
    • Replace the hardcoded 30-second timeout on multi-region database creation with DBAPI_OPERATION_TIMEOUT (240s) to prevent CI flakes under load.

Savings:

  • Eliminates 5 full database creations and drops (including 1 multi-region DB).
  • Eliminates ~8 long-running DDL operations.

@olavloite
olavloite requested a review from a team as a code owner October 14, 2026 10:50

@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 refactors and consolidates several system tests in test_database_api.py for Google Cloud Spanner. It merges separate creation and update tests for databases, PITR, default leaders, and database roles into combined, more efficient test cases, while removing redundant test definitions. A review comment correctly points out that using us-central1 as the new default leader in the multi-region test will fail because it is typically a witness replica, and suggests using us-east1 instead.

Comment thread packages/google-cloud-spanner/tests/system/test_database_api.py Outdated
…ase churn

Consolidate administrative system tests in `test_database_api.py` to eliminate
redundant database creations, long-running operations (LROs), and schema churn:
1. Consolidate Create & Update Tests:
   - Combine PITR database creation and subsequent DDL update into a single
     test (`test_create_and_update_database_pitr`), asserting both client
     properties and Information Schema catalog values.
   - Combine default leader creation and subsequent update into a single
     multi-region database test (`test_create_and_update_database_with_default_leader`).
   - Merge `test_update_ddl_w_operation_id` into `test_create_database_and_update_ddl`.
2. Consolidate Database Roles & Permissions:
   - Merge `test_create_role_grant_access_success` and `test_list_database_role_success`
     into `test_create_role_grant_access_and_list_roles`.
   - Strip the 20-table schema down to only the single required table (`contacts`).
   - Eliminate redundant teardown DDL (revoking permissions and dropping roles)
     immediately prior to database deletion.
   - Fix a lazy evaluation bug where the parent role query was never iterated
     and therefore never dispatched over the wire.
3. Remove Pure Server-Side DDL Parser Tests:
   - Drop `test_create_database_pitr_invalid_retention_period` and
     `test_update_ddl_w_pitr_invalid`, which tested Spanner backend syntax
     rejection of '0d' rather than client validation logic.
4. Fix Multi-Region Database Timeout:
   - Replace the hardcoded 30-second timeout on multi-region database creation
     with `DBAPI_OPERATION_TIMEOUT` (240s) to prevent CI flakes under load.

Savings:
- Eliminates 5 full database creations and drops (including 1 multi-region DB).
- Eliminates ~8 long-running DDL operations.
@olavloite
olavloite force-pushed the spanner-simplify-db-admin-tests branch from 200570c to d36a154 Compare October 14, 2026 11:55
@olavloite

Copy link
Copy Markdown
Contributor Author

/gemini review

@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 refactors and consolidates several system tests in test_database_api.py by combining database creation and DDL update tests (such as PITR and default leader updates) into single, unified test cases. It also removes redundant test cases and simplifies role creation and listing tests. The feedback suggests ensuring that database option queries do not silently pass if they return empty results by converting the query results to a list and asserting that exactly one row is returned.

Comment thread packages/google-cloud-spanner/tests/system/test_database_api.py Outdated
Comment thread packages/google-cloud-spanner/tests/system/test_database_api.py Outdated
@parthea parthea self-assigned this Oct 16, 2026

@parthea parthea 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.

LGTM, Please could you also apply the suggestions from gemini-code-assist

@parthea parthea assigned olavloite and unassigned parthea Oct 17, 2026
@olavloite
olavloite merged commit 6d1b0fd into main Oct 26, 2026
55 of 56 checks passed
@olavloite
olavloite deleted the spanner-simplify-db-admin-tests branch October 26, 2026 18:03
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