chore(firestore): optimize system tests runtime by ohmayr · Pull Request #17418 · googleapis/google-cloud-python

ohmayr · 2026-06-10T23:49:21Z

Optimizes Firestore system tests execution time to 13 minutes (down from ~1 hour) under live GCP environments.

Parallelism: Added pytest-xdist using all cores (--dist load for maximum load balancing).
Isolation: Appended os.getpid() to test resource IDs to prevent parallel worker collisions.
Fixture Re-use: Promoted query setup fixtures to module scope to reduce database setups by ~97%.
Fast Polling: Reduced hardcoded delays in watch tests from 1.0s to 0.2s with early-exit polling.

gemini-code-assist

Code Review

This pull request introduces parallel execution for system tests using pytest-xdist and updates several fixtures in both synchronous and asynchronous system tests to be module-scoped. The review feedback highlights critical issues with these changes: first, changing async fixtures to module scope in test_system_async.py will cause a ScopeMismatch error unless a module-scoped event_loop is defined; second, a similar ScopeMismatch error may occur in test_system.py if the database fixture is function-scoped; and finally, to preserve the caching benefits of these module-scoped fixtures under pytest-xdist, the test distribution mode should be configured with --dist=loadfile instead of the default behavior.

gemini-code-assist · 2026-06-10T23:50:55Z



-@pytest_asyncio.fixture
+@pytest_asyncio.fixture(scope="module")


In pytest-asyncio (especially with the pinned version 0.21.2 in noxfile.py), the default event_loop fixture is function-scoped. Defining an async fixture with scope="module" (like query_docs, collection, and async_query) will raise a ScopeMismatch error during test collection or execution.

To resolve this, you must define a module-scoped event_loop fixture in this file or in conftest.py to override the default one.

@pytest.fixture(scope="module") def event_loop(): loop = asyncio.get_event_loop_policy().new_event_loop() yield loop loop.close() @pytest_asyncio.fixture(scope="module")

gemini-code-assist · 2026-06-10T23:50:55Z

+            "-n",
+            "auto",


When running system tests in parallel with pytest-xdist (-n auto), the default distribution mode is --dist=load, which distributes individual tests to any available worker. This can cause tests from the same module to run on different workers, meaning module-scoped fixtures (like query_docs which was changed to scope="module" in this PR) will be executed multiple times (once per worker), defeating the caching benefit.

To ensure that all tests within a single module run on the same worker and preserve the efficiency of module-scoped fixtures, consider using --dist=loadfile.

Suggested change

"-n",

"auto",

"-n",

"auto",

"--dist",

"loadfile",

gemini-code-assist · 2026-06-10T23:50:55Z

+            "-n",
+            "auto",


When running system tests in parallel with pytest-xdist (-n auto), the default distribution mode is --dist=load, which distributes individual tests to any available worker. This can cause tests from the same module to run on different workers, meaning module-scoped fixtures (like query_docs which was changed to scope="module" in this PR) will be executed multiple times (once per worker), defeating the caching benefit.

To ensure that all tests within a single module run on the same worker and preserve the efficiency of module-scoped fixtures, consider using --dist=loadfile.

Suggested change

"-n",

"auto",

"-n",

"auto",

"--dist",

"loadfile",

gemini-code-assist · 2026-06-10T23:50:55Z

+@pytest.fixture(scope="module")
 def query_docs(client, database):


If the database fixture is function-scoped, defining query_docs with scope="module" will raise a ScopeMismatch error. To prevent this, the database fixture must also be configured with scope="module" or higher, or query_docs should remain function-scoped.

daniel-sanche

Looks like I'm late with my comments, but I'll add them here anyway. Nothing major

daniel-sanche · 2026-06-11T20:49:51Z

    count: 1
+  # TODO(https://github.com/googleapis/google-cloud-python/issues/17429):
+  # Temporary post-processing rule to add pytest-xdist dependency.
+  # Remove this once gapic-generator includes pytest-xdist by default.


I'm surprised firestore uses a generated noxfile. I thought most veneers had their own customized copies

It looks like bigtable has the entire noxfile contents in a post-processing script now? This seems like something we might want to revisit, but probably out of scope for this PR

Yes. Firestore is not completely handwritten. We override things via post-processing. There's many ways to go about this. This is more so a design change i.e. we could make it completely handwritten or have the entire file in post processing script or implement something on the generator side too.

But yes, out of scope for this work.

daniel-sanche · 2026-06-11T20:54:03Z

+  # TODO(https://github.com/googleapis/google-cloud-python/issues/17429):
+  # Temporary post-processing rule to inject `-n auto` for Firestore parallel tests.
+  # This rule should be removed once the generator template changes are released
+  # and the generator version is updated in librarian.yaml.


Is the plan to include -n auto for all system tests?

Let's have this discussion on this PR: #17430

daniel-sanche · 2026-06-11T20:57:04Z

 DOCUMENT_EXISTS = "Document already exists: "
 ENTERPRISE_MODE_ERROR = "only allowed on ENTERPRISE mode"
-UNIQUE_RESOURCE_ID = unique_resource_id("-")
+UNIQUE_RESOURCE_ID = unique_resource_id("-") + "-" + str(os.getpid())


If unique_resource_id() isn't unique enough, this logic should probably be added there, instead of appended afterwards

Good catch! We can address this as a follow up or determine why it won't work.

Addressed in #17440.

daniel-sanche · 2026-06-11T20:58:02Z

    after: |
      "pytest-asyncio==0.21.2",
    count: 2
+  # TODO(https://github.com/googleapis/google-cloud-python/issues/17429):


What's the plan for these TODOs? Can you make sure we follow through with them, instead of leaving them in the backlog?

Well! Depends if we want to work on flaky tests within this fixit and have the time for it. I'll sync on this one and if not a priority, we can close this bug.

Addressed in #17439.

wip

9066c33

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

ohmayr force-pushed the update-firestore-system-tests branch from 80d97d6 to 5ce929d Compare June 11, 2026 04:39

update unique resource id

a3b5b1d

ohmayr force-pushed the update-firestore-system-tests branch from 5ce929d to a3b5b1d Compare June 11, 2026 04:43

change sleep time

e11bb0e

ohmayr added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 11, 2026

ohmayr closed this Jun 11, 2026

ohmayr reopened this Jun 11, 2026

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 11, 2026

ohmayr changed the title ~~wip~~ perf(firestore): optimize system test suite performance from 1 hr to ~13 mins Jun 11, 2026

ohmayr changed the title ~~perf(firestore): optimize system test suite performance from 1 hr to ~13 mins~~ chore(firestore): optimize system tests runtime Jun 11, 2026

ohmayr assigned daniel-sanche Jun 11, 2026

ohmayr marked this pull request as ready for review June 11, 2026 06:08

ohmayr requested a review from a team as a code owner June 11, 2026 06:08

parthea approved these changes Jun 11, 2026

View reviewed changes

Comment thread packages/google-cloud-firestore/noxfile.py

parthea reviewed Jun 11, 2026

View reviewed changes

Comment thread packages/google-cloud-firestore/tests/system/test_system.py

address PR feedback

2e448a7

ohmayr requested a review from a team as a code owner June 11, 2026 16:20

ohmayr merged commit f006770 into main Jun 11, 2026
32 checks passed

ohmayr deleted the update-firestore-system-tests branch June 11, 2026 21:02

daniel-sanche reviewed Jun 11, 2026

View reviewed changes



		@pytest_asyncio.fixture
		@pytest_asyncio.fixture(scope="module")

-            "-n",
-            "auto",
+            "-n",
+            "auto",
+            "--dist",
+            "loadfile",

		@pytest.fixture(scope="module")
		def query_docs(client, database):

Conversation

ohmayr commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ohmayr commented Jun 10, 2026 •

edited

Loading

daniel-sanche left a comment •

edited

Loading