Skip to content

Commit 761c805

Browse files
authored
test(spanner): add pytest-xdist parallel execution with state isolation (#17344)
This PR adds `pytest-xdist` to parallelize unit tests and the `core_deps_from_source` nox sessions for the `google-cloud-spanner` package. **By running tests in parallel using `-n auto`, the execution time of the Spanner unit tests are reduced from ~8 minutes to 4 minutes.** State isolation and test reliability are achieved by: * **Simplifying Subtests**: We pass simple strings/names to `subTest()` instead of complex objects. This keeps subtests lightweight and prevents serialization errors. * **Cleaning Up Global Singletons**: We reset telemetry singleton states on test teardown (using pytest's idiomatic `monkeypatch` fixture). This ensures metric counters don't leak from one test into another. * **Fixing Concurrent Mock Conflicts**: We return fresh mocked iterators for concurrent calls (using `side_effect` instead of `return_value`). This prevents one thread from exhausting a mock's results before another thread can read them. * **Robust Assertion Checks**: We added a helper method (`_assert_concurrent_transaction_invariants`) that verifies the *behavior* of concurrent threads (ensuring exactly one thread starts the transaction while others wait/reuse it), rather than checking fragile call logs or counting sequential request IDs. This allowed us to safely run four concurrent tests. > [!note] > The long pole in the tent is still the `system` tests, which require about 30 minutes. It is not as simple as just adding xdist because there are other factors that limit velocity including the fact that system tests actually interact with live systems.
1 parent 25b857e commit 761c805

14 files changed

Lines changed: 163 additions & 217 deletions

File tree

packages/google-cloud-spanner/noxfile.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"pytest",
4444
"pytest-cov",
4545
"pytest-asyncio",
46+
"pytest-xdist",
4647
]
4748
MOCK_SERVER_ADDITIONAL_DEPENDENCIES = [
4849
"google-cloud-testutils",
@@ -240,6 +241,8 @@ def unit(session, protobuf_implementation):
240241
# Run py.test against the unit tests.
241242
args = [
242243
"py.test",
244+
"-n",
245+
"auto",
243246
"-s",
244247
f"--junitxml=unit_{session.python}_sponge_log.xml",
245248
"--cov=google",
@@ -754,7 +757,6 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
754757
def mypy(session):
755758
"""Run the type checker."""
756759
session.skip("Mypy is not yet supported")
757-
758760
# TODO(https://github.com/googleapis/gapic-generator-python/issues/2579):
759761
# use the latest version of mypy
760762
session.install(
@@ -832,12 +834,15 @@ def core_deps_from_source(session, protobuf_implementation):
832834
dep_paths = [str(deps_dir / dep) for dep in core_dependencies_from_source]
833835

834836
session.install(*dep_paths, "--no-deps", "--ignore-installed")
837+
session.install("pytest-xdist")
835838
print(
836839
f"Installed {', '.join(core_dependencies_from_source)} locally from {deps_dir}"
837840
)
838841

839842
session.run(
840843
"py.test",
844+
"-n",
845+
"auto",
841846
"tests/unit",
842847
env={
843848
"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": protobuf_implementation,

packages/google-cloud-spanner/setup.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,17 @@
6060
"google-cloud-monitoring >= 2.16.0",
6161
"mmh3 >= 4.1.0",
6262
]
63-
extras = {"libcst": "libcst >= 0.2.5"}
63+
extras = {
64+
"libcst": "libcst >= 0.2.5",
65+
"test": [
66+
"pytest",
67+
"mock",
68+
"asyncmock",
69+
"pytest-cov",
70+
"pytest-asyncio",
71+
"pytest-xdist",
72+
],
73+
}
6474

6575
url = "https://github.com/googleapis/google-cloud-python/tree/main/packages/google-cloud-spanner"
6676

packages/google-cloud-spanner/tests/system/_async/test_database_api.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,10 @@ async def _unit_of_work(transaction):
179179
transaction.insert_or_update(sd.TABLE, sd.COLUMNS, sd.ROW_DATA)
180180

181181
await shared_database.run_in_transaction(_unit_of_work)
182-
assert attempts == 2
182+
# Expect at least 2 attempts due to our simulated manual abort on first try.
183+
# We use >= 2 rather than == 2 because the live Spanner server can also
184+
# trigger transient abort retries depending on real-world GCP resource contention.
185+
assert attempts >= 2
183186

184187

185188
@pytest.mark.asyncio

packages/google-cloud-spanner/tests/system/conftest.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414

1515
import datetime
16+
import os
1617
import time
1718

1819
import pytest
@@ -25,6 +26,10 @@
2526

2627
from . import _helpers
2728

29+
# Disable builtin metrics for system tests by default to avoid 401 errors
30+
# from the background thread exporting to Cloud Monitoring without permissions.
31+
os.environ["SPANNER_DISABLE_BUILTIN_METRICS"] = "true"
32+
2833

2934
@pytest.fixture(scope="function")
3035
def if_create_instance():

packages/google-cloud-spanner/tests/unit/_async/test_client.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,9 @@ async def test_constructor_logs_options_disabled_by_default(self):
799799
info_logger.assert_not_called()
800800

801801
# Also test when the environment variable is not set at all
802-
with mock.patch.dict(os.environ, {}, clear=True):
802+
with mock.patch.dict(
803+
os.environ, {"SPANNER_DISABLE_BUILTIN_METRICS": "true"}, clear=True
804+
):
803805
with mock.patch.object(logger, "info") as info_logger:
804806
client = self._make_one(project=self.PROJECT, credentials=creds)
805807
self.assertIsNotNone(client)

packages/google-cloud-spanner/tests/unit/_async/test_client_extra.py

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,21 @@ async def test_sync_branches_admin_apis(self):
131131
self.assertIsNotNone(ia_api)
132132
self.assertIsNotNone(da_api)
133133

134-
def test_initialize_metrics_double_check(self):
134+
# Safety shield mocks: We intercept the OpenTelemetry metric classes at the client module namespace level
135+
# to prevent instantiating real exporter objects. This prevents spawning live background worker threads
136+
# that periodically wake up and trigger 401 credential errors inside unauthenticated unit test runs.
137+
@mock.patch("google.cloud.spanner_v1._async.client.CloudMonitoringMetricsExporter")
138+
@mock.patch("google.cloud.spanner_v1._async.client.PeriodicExportingMetricReader")
139+
@mock.patch("google.cloud.spanner_v1._async.client.MeterProvider")
140+
# Global state reset: Temporarily override the module's process-wide global boolean _metrics_monitor_initialized
141+
# to False so that the client enters the initialization logic instead of returning early.
142+
@mock.patch(
143+
"google.cloud.spanner_v1._async.client._metrics_monitor_initialized",
144+
False,
145+
)
146+
def test_initialize_metrics_double_check(
147+
self, mock_provider, mock_reader, mock_exporter
148+
):
135149
# coverage for line 143->exit
136150
from google.cloud.spanner_v1._async import client as MUT
137151

@@ -147,15 +161,17 @@ def __enter__(self):
147161
def __exit__(self, *args):
148162
return original_lock.__exit__(*args)
149163

164+
# Concurrency race condition simulator: Replace the process synchronization lock with our custom SettingLock.
165+
# When this lock enters, it toggles _metrics_monitor_initialized to True to simulate another thread
166+
# completing metrics setup while this thread was waiting for the lock.
150167
with mock.patch(
151-
"google.cloud.spanner_v1._async.client._metrics_monitor_initialized", False
168+
"google.cloud.spanner_v1._async.client._metrics_monitor_lock",
169+
SettingLock(),
152170
):
153-
with mock.patch(
154-
"google.cloud.spanner_v1._async.client._metrics_monitor_lock",
155-
SettingLock(),
156-
):
157-
MUT._initialize_metrics("project", self.credentials)
158-
self.assertTrue(MUT._metrics_monitor_initialized)
171+
# Trigger the initialization function and verify Spanner's double-checked lock safely
172+
# checks the flag again and aborts cleanly to prevent dual-registration.
173+
MUT._initialize_metrics("project", self.credentials)
174+
self.assertTrue(MUT._metrics_monitor_initialized)
159175

160176
def test_default_transaction_options_validation(self):
161177
# coverage for line 344

packages/google-cloud-spanner/tests/unit/_async/test_session.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import datetime
2+
import threading
23
from datetime import timezone
34

45
import google.api_core.gapic_v1.method
@@ -1800,11 +1801,16 @@ async def unit_of_work(txn, *args, **kw):
18001801
called_with.append((txn, args, kw))
18011802
txn.insert(TABLE_NAME, COLUMNS, VALUES)
18021803

1804+
main_thread = threading.current_thread()
1805+
_results = [1, 1.5]
1806+
18031807
# retry once w/ timeout_secs=1
1804-
def _time(_results=[1, 1.5]):
1805-
if len(_results) > 1:
1806-
return _results.pop(0)
1807-
return _results[0]
1808+
def _time():
1809+
if threading.current_thread() is main_thread:
1810+
if len(_results) > 1:
1811+
return _results.pop(0)
1812+
return _results[0]
1813+
return 1.0
18081814

18091815
with mock.patch("time.time", _time):
18101816
with mock.patch(
@@ -1877,9 +1883,14 @@ async def unit_of_work(txn, *args, **kw):
18771883
called_with.append((txn, args, kw))
18781884
txn.insert(TABLE_NAME, COLUMNS, VALUES)
18791885

1886+
main_thread = threading.current_thread()
1887+
_results = [1] * 100
1888+
18801889
# retry several times to check backoff
1881-
def _time(_results=[1] * 100):
1882-
return _results.pop(0)
1890+
def _time():
1891+
if threading.current_thread() is main_thread:
1892+
return _results.pop(0)
1893+
return 1.0
18831894

18841895
with (
18851896
mock.patch("time.time", _time),

packages/google-cloud-spanner/tests/unit/conftest.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,23 @@
1414

1515
import os
1616

17+
import pytest
18+
19+
from google.cloud.spanner_v1.metrics.spanner_metrics_tracer_factory import (
20+
SpannerMetricsTracerFactory,
21+
)
22+
1723
# Disable builtin metrics to avoid background thread noise and 401 errors in unit tests
1824
os.environ["SPANNER_DISABLE_BUILTIN_METRICS"] = "true"
25+
26+
27+
@pytest.fixture(autouse=True)
28+
def reset_metrics_singletons(monkeypatch):
29+
# Reset singletons and env var before test to avoid state pollution
30+
monkeypatch.setenv("SPANNER_DISABLE_BUILTIN_METRICS", "true")
31+
SpannerMetricsTracerFactory._metrics_tracer_factory = None
32+
SpannerMetricsTracerFactory._current_metrics_tracer_ctx.set(None)
33+
yield
34+
# Reset singletons after test to ensure no leakage
35+
SpannerMetricsTracerFactory._metrics_tracer_factory = None
36+
SpannerMetricsTracerFactory._current_metrics_tracer_ctx.set(None)

packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_partition_helper.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ def collect_protobufs(val):
192192

193193
registered_classes = set(partition_helper._PROTO_CLASS_MAP.values())
194194
for cls in discovered_protobuf_classes:
195-
with self.subTest(cls=cls):
195+
with self.subTest(cls_name=cls.__name__):
196196
self.assertIn(
197197
cls,
198198
registered_classes,

packages/google-cloud-spanner/tests/unit/test__helpers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ def test_w_numeric_precision_and_scale_valid(self):
329329
decimal.Decimal("1E-9"),
330330
]
331331
for value in cases:
332-
with self.subTest(value=value):
332+
with self.subTest(value=str(value)):
333333
value_pb = self._callFUT(value)
334334
self.assertIsInstance(value_pb, Value)
335335
self.assertEqual(value_pb.string_value, str(value))
@@ -371,7 +371,7 @@ def test_w_numeric_precision_and_scale_invalid(self):
371371
]
372372

373373
for value, err_msg in cases:
374-
with self.subTest(value=value, err_msg=err_msg):
374+
with self.subTest(value=str(value), err_msg=err_msg):
375375
self.assertRaisesRegex(
376376
ValueError,
377377
err_msg,

0 commit comments

Comments
 (0)