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

[olavloite]

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.

[gemini-code-assist]

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.

[olavloite]

/gemini review

[gemini-code-assist]

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.

[gemini-code-assist]

If the query returns no rows, the for result in results: loop will not execute, and the assertion assert result[0] == new_retention_period will be silently skipped. To prevent a potential silent test pass, convert the results to a list and assert that exactly one row is returned.

Suggested change

	with temp_db.snapshot() as snapshot:
	results = snapshot.execute_sql(
	"SELECT OPTION_VALUE AS version_retention_period "
	"FROM INFORMATION_SCHEMA.DATABASE_OPTIONS "
	"WHERE SCHEMA_NAME = '' "
	"AND OPTION_NAME = 'version_retention_period'"
	)
	for result in results:
	assert result[0] == new_retention_period
	with temp_db.snapshot() as snapshot:
	results = list(
	snapshot.execute_sql(
	"SELECT OPTION_VALUE AS version_retention_period "
	"FROM INFORMATION_SCHEMA.DATABASE_OPTIONS "
	"WHERE SCHEMA_NAME = '' "
	"AND OPTION_NAME = 'version_retention_period'"
	)
	)
	assert len(results) == 1
	assert results[0][0] == new_retention_period

[gemini-code-assist]

If the query returns no rows, the for result in results: loop will not execute, and the assertion assert result[0] == new_default_leader will be silently skipped. To prevent a potential silent test pass, convert the results to a list and assert that exactly one row is returned.

Suggested change

	with temp_db.snapshot() as snapshot:
	results = snapshot.execute_sql(
	"SELECT OPTION_VALUE AS default_leader "
	"FROM INFORMATION_SCHEMA.DATABASE_OPTIONS "
	"WHERE SCHEMA_NAME = '' AND OPTION_NAME = 'default_leader'"
	)
	for result in results:
	assert result[0] == new_default_leader
	with temp_db.snapshot() as snapshot:
	results = list(
	snapshot.execute_sql(
	"SELECT OPTION_VALUE AS default_leader "
	"FROM INFORMATION_SCHEMA.DATABASE_OPTIONS "
	"WHERE SCHEMA_NAME = '' AND OPTION_NAME = 'default_leader'"
	)
	)
	assert len(results) == 1
	assert results[0][0] == new_default_leader