Skip to content

tests(spanner): update prerelease_deps nox session to match system tests#16764

Open
parthea wants to merge 6 commits intomainfrom
parthea-patch-15
Open

tests(spanner): update prerelease_deps nox session to match system tests#16764
parthea wants to merge 6 commits intomainfrom
parthea-patch-15

Conversation

@parthea
Copy link
Copy Markdown
Contributor

@parthea parthea commented Apr 22, 2026

This PR fixes 2 failing tests which appear in #16762

In the system test presubmit Kokoro System Tests, the assertion is too strict. Protobuf objects may fail equality (==) if one has extra metadata populated by the server that the other lacks. If the server fills in a default field in one response but not another, the assertion fails even if the ID is identical.

=================================== FAILURES ===================================
_____________________________ test_list_instances ______________________________
..

    def test_list_instances(
        no_create_instance,
        spanner_client,
        existing_instances,
        shared_instance,
    ):
        instances = list(spanner_client.list_instances())
    
        for instance in instances:
>           assert instance in existing_instances or instance is shared_instance
...
tests/system/test_instance_api.py:40: AssertionError

The other failure only appears in the Kokoro Pre-release Tests. Updating the prerelease_deps nox session to match the system test session resolves the issue.

==================================== ERRORS ====================================
____________________ ERROR at setup of test_table_not_found ____________________

self = 

    def setup(self) -> None:
        runner_fixture_id = f"_{self._loop_scope}_scoped_runner"
        if runner_fixture_id not in self.fixturenames:
            self.fixturenames.append(runner_fixture_id)
>       return super().setup()
               ^^^^^^^^^^^^^^^

.nox/8f8a4a6e/lib/python3.14/site-packages/pytest_asyncio/plugin.py:458: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

fixturedef = 
request = >

    @pytest.hookimpl(wrapper=True)
    def pytest_fixture_setup(fixturedef: FixtureDef, request) -> object | None:
        asyncio_mode = _get_asyncio_mode(request.config)
        if not _is_asyncio_fixture_function(fixturedef.func):
            if asyncio_mode == Mode.STRICT:
                # Ignore async fixtures without explicit asyncio mark in strict mode
                # This applies to pytest_trio fixtures, for example
>               return (yield)
                        ^^^^^
E               pytest.PytestRemovedIn9Warning: '' requested an async fixture 'instance_configs', with no plugin or hook that handled it. This is usually an error, as pytest does not natively support it. This will turn into an error in pytest 9.
E               See: https://docs.pytest.org/en/stable/deprecations.html#sync-test-depending-on-async-fixture

.nox/8f8a4a6e/lib/python3.14/site-packages/pytest_asyncio/plugin.py:728: PytestRemovedIn9Warning

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a trailing newline to the mypy.ini configuration file. I have no feedback to provide.

@parthea parthea force-pushed the parthea-patch-15 branch 2 times, most recently from b102cea to 8df400c Compare April 22, 2026 19:44
@parthea parthea changed the title [for testing purposes] testing presubmits tests(spanner): update prerelease_deps nox session to match system tests Apr 22, 2026
@parthea parthea added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 22, 2026
@parthea parthea removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 23, 2026
@parthea parthea marked this pull request as ready for review April 23, 2026 00:15
@parthea parthea requested a review from a team as a code owner April 23, 2026 00:15
@parthea
Copy link
Copy Markdown
Contributor Author

parthea commented Apr 23, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the system test execution in noxfile.py to support separate sync and async test runs and improves test discovery. Additionally, it updates test_list_instances to compare instance names rather than objects, preventing failures due to server-side metadata differences. Feedback suggests centralizing common environment variables to reduce duplication and utilizing defined path variables instead of hardcoded strings in the test execution logic.

@@ -675,24 +681,75 @@ def prerelease_deps(session, protobuf_implementation, database_dialect):
run_system = False

if run_system:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The environment variables for the test runs are duplicated multiple times in this block. Consider defining a common test_env dictionary at the start of the if run_system: block to improve maintainability and reduce redundancy, adhering to the rule of removing duplicate lines of code.

References
  1. Remove duplicate lines of code to keep the codebase clean and avoid redundancy.

Comment thread packages/google-cloud-spanner/noxfile.py Outdated
Comment thread packages/google-cloud-spanner/noxfile.py Outdated
parthea and others added 3 commits April 22, 2026 21:09
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants