[sakthivelmanii]

Problem

• The system session in packages/django-google-spanner/noxfile.py (tests/system) was skipped with session.skip("System tests are temporarily disabled.").
• When enabled against live Cloud Spanner, each of the 4 system test classes (TestCheckConstraint, TestDecimal, TestJsonField, TestQueries) created and dropped the database and ran full call_command("migrate") across unused django.contrib.* apps (auth, contenttypes, sites, sessions) 4 separate times, causing the 8-test suite to take ~52 minutes.

Solution

• Remove session.skip("System tests are temporarily disabled.") from def system(session): in packages/django-google-spanner/noxfile.py.
• Remove unused django.contrib.* entries from INSTALLED_APPS in tests/settings.py and generate a unique database name per test session (dj_sys_<timestamp>_<hex6>).
• Update tests/system/django_spanner/utils.py to connect to the existing shared Spanner instance on live Spanner (creating an instance only on the Emulator), clean up stale dj_sys_<timestamp>_* databases older than 1 hour, reuse the active database across all system test classes in the session, and drop the database via an atexit cleanup handler.

[gemini-code-assist]

Code Review

This pull request re-enables system tests, introduces dynamic database naming in settings, and refactors test resource setup and teardown in utils.py using an atexit cleanup handler. The review feedback highlights three key issues: first, removing the CREATE_INSTANCE guard during instance checks could leak orphaned Spanner instances when CREATE_INSTANCE is disabled; second, using the un-prefixed database name in setup_database leads to duplicate database creation alongside Django's prefixed test database; and third, generating random database names per run risks accumulating orphaned databases if tests terminate abruptly, suggesting a static naming strategy or a startup cleanup routine instead.

[gemini-code-assist]

By removing the if CREATE_INSTANCE: check and instead checking if not Config.INSTANCE.exists(), the test suite will now automatically create a Spanner instance if it does not exist, even if GOOGLE_CLOUD_TESTS_CREATE_SPANNER_INSTANCE is not set.

However, in _cleanup_test_resources(), the instance is only deleted if CREATE_INSTANCE is True:

if CREATE_INSTANCE and Config.INSTANCE is not None:
    Config.INSTANCE.delete()

This means if CREATE_INSTANCE is False and the instance did not exist, the suite will create the Spanner instance but never delete it, leading to extremely expensive orphaned Spanner instances running indefinitely.

To prevent this, restore the CREATE_INSTANCE guard so that instances are only created when explicitly requested.

Suggested change

	if not Config.INSTANCE.exists():
	if CREATE_INSTANCE and not Config.INSTANCE.exists():

References

1. Avoid eagerly creating or initializing a client or resource if a downstream process can lazily create it only when needed.

[sakthivelmanii]

Removed live Spanner instance creation and deletion from utils.py since the shared test instance already exists in CI (only creating an instance when running against the Emulator).

[gemini-code-assist]

When running tests via Django's test runner (e.g., manage.py test), Django automatically prefixes the database name with test_ (e.g., test_dj_sys_...) and updates connections['default'].settings_dict['NAME'] accordingly.

However, DATABASE_NAME in utils.py is initialized at module-import time using settings.DATABASES['default']['NAME'], which does not have the test_ prefix (e.g., dj_sys_...).

As a result:

1. setup_database() checks and creates the un-prefixed database (dj_sys_...).
2. Django's test runner creates and runs tests against the prefixed database (test_dj_sys_...).
3. This leads to two databases being created on Spanner, with the un-prefixed one being completely unused and wasted, doubling the creation/deletion overhead.

To fix this, retrieve the active database name dynamically from connections['default'].settings_dict['NAME'] inside setup_database() instead of using the module-level DATABASE_NAME constant.

Suggested change

	if Config.DATABASE is not None:
	return
	Config.DATABASE = Config.INSTANCE.database(DATABASE_NAME)
	if not Config.DATABASE.exists():
	creation = DatabaseCreation(connection)
	creation._create_test_db(verbosity=0, autoclobber=False, keepdb=True)
	# Running migrations on the db.
	call_command("migrate", interactive=False)
	try:
	created_op = Config.DATABASE.create()
	created_op.result(SPANNER_OPERATION_TIMEOUT_IN_SECONDS)
	except exceptions.AlreadyExists:
	pass
	if Config.DATABASE is not None:
	return
	active_db_name = connections['default'].settings_dict['NAME']
	Config.DATABASE = Config.INSTANCE.database(active_db_name)
	if not Config.DATABASE.exists():
	try:
	created_op = Config.DATABASE.create()
	created_op.result(SPANNER_OPERATION_TIMEOUT_IN_SECONDS)
	except exceptions.AlreadyExists:
	pass

[sakthivelmanii]

Updated setup_database() to read connections['default'].settings_dict['NAME'] dynamically and removed the redundant module-level DATABASE_NAME constant.

[gemini-code-assist]

Generating a completely random database name (dj_sys_{int(time.time())}_{uuid.uuid4().hex[:6]}) per test run can lead to orphaned databases accumulating in the Spanner instance if a test run crashes or is terminated abruptly (e.g., via SIGKILL, CI timeout, or runner cancellation), as atexit handlers do not run in those cases.

Since Spanner has a limit on the number of databases per instance (typically 100), these orphaned databases will eventually cause all subsequent test runs to fail with quota/limit errors.

Consider one of the following improvements:

1. Use a static database name (or a name based on a stable environment variable like the CI build ID/job ID) so that subsequent runs can reuse or clean up the same database.
2. Implement a startup cleanup routine in setup_instance() that lists existing databases in the Spanner instance and deletes any databases with the dj_sys_ prefix that are older than a certain threshold (e.g., 2 hours).

[sakthivelmanii]

Added _cleanup_stale_databases() in setup_instance() to drop any stale dj_sys_<timestamp>_* databases older than 1 hour.