test(spanner): upgrade Spanner sample instance fixtures to session scope#17175
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the sample_name fixture across multiple test files and centralizes instance labeling in conftest.py by using a hardcoded "shared-samples" value. Additionally, several fixtures in conftest.py have been upgraded from module to session scope. A potential issue was identified regarding a ScopeMismatch error, as the newly session-scoped sample_instance fixture depends on other fixtures that might still be module-scoped.
| spanner_client, | ||
| cleanup_old_instances, |
There was a problem hiding this comment.
The sample_instance fixture has been upgraded to session scope, but it depends on spanner_client and cleanup_old_instances. If these fixtures are still module-scoped (which was the previous scope for most fixtures in this file), pytest will raise a ScopeMismatch error. Please ensure that spanner_client, cleanup_old_instances, and any other dependencies of session-scoped fixtures are also upgraded to session scope.
There was a problem hiding this comment.
spanner_client and cleanup_old_instances were already session scope, so this should not cause any problems.
There was a problem hiding this comment.
Won't this cause race conditions if multiple integration test runs are started in parallel? They will now use the same instance, and potentially try to use the same database name.
There was a problem hiding this comment.
I fixed it. created a new unique db name
Previously, standard and multi-region Spanner instance fixtures in conftest.py were module-scoped, meaning a brand-new Spanner instance was created and destroyed for every single test file, adding massive execution time (~2 minutes per file) in CI/CD runs. This change upgrades the instance and config fixtures to session scope. To prevent ScopeMismatch exceptions, the dependencies on module-scoped sample_name are replaced with a generic shared-samples label. This allows the test suite to reuse a single shared Spanner instance, dramatically optimizing test execution times.
a9e0f14 to
805dcd1
Compare
Previously, standard and multi-region Spanner instance fixtures in conftest.py were module-scoped, meaning a brand-new Spanner instance was created and destroyed for every single test file, adding massive execution time (~2 minutes per file) in CI/CD runs.
This change upgrades the instance and config fixtures to session scope. To prevent ScopeMismatch exceptions, the dependencies on module-scoped sample_name are replaced with a generic shared-samples label. This allows the test suite to reuse a single shared Spanner instance, dramatically optimizing test execution times.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕