Test: Migrate the Eventarc tests off unittest.IsolatedAsyncioTestCase to pytest style - #244
Open
AmaadMartin wants to merge 4 commits into
Open
Test: Migrate the Eventarc tests off unittest.IsolatedAsyncioTestCase to pytest style#244AmaadMartin wants to merge 4 commits into
AmaadMartin wants to merge 4 commits into
Conversation
added 4 commits
August 11, 2026 22:58
…syncio TestEventarcClient inherited from unittest.IsolatedAsyncioTestCase, so unittest awaited its coroutines and pytest-asyncio never saw them. Drop the base class, mark the five async tests explicitly, and replace the 25 self.assert* calls with bare asserts.
…asyncio TestEventarcToolset inherited from unittest.IsolatedAsyncioTestCase, so unittest awaited its coroutines and pytest-asyncio never saw them. Drop the base class, mark the two async tests explicitly, and replace the 10 self.assert* calls with bare asserts.
…bTest loops TestMessageTool inherited from unittest.IsolatedAsyncioTestCase, so unittest awaited its coroutines and pytest-asyncio never saw them: markers, fixtures and parametrize were all inert inside the class. Drop the base class and mark the 31 async tests explicitly. Replace setUp/tearDown with an autouse fixture that patches through context managers. The old tearDown called mock.patch.stopall(), which drains unittest.mock's process-global registry and so could stop patches this class never started; the fixture unwinds only its own two patchers. Expand the two self.subTest loops into parametrized cases, which raises the collected count for this file from 31 to 42.
test_config.py was the last unittest module in the directory, so the package advertised two test styles at once. Convert it to a plain pytest class with bare asserts and pytest.raises. Its two node IDs are unchanged. Also drop the unused os import from test_client.py, in an import block this change already rewrites.
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A
Problem: Three Eventarc test classes inherited from
unittest.IsolatedAsyncioTestCase.unittestawaits the coroutines of such a class itself, so pytest-asyncio never sees them. That makes@pytest.mark.asyncio,pytest.fixtureandpytest.mark.parametrizeinert inside those classes, and it hides the async intent from the pytest tooling the rest of the suite uses. The directory also disagreed with its own neighbourtest_domain_specific_publish.py, which is already pytest style. A fourth module,test_config.py, was a plainunittest.TestCase, so the package advertised two styles at once.Solution: I converted the three classes to plain pytest classes.
setUp/tearDownintest_message_tool.pybecomes an autouse fixture that patches through context managers. The oldtearDowncalledmock.patch.stopall(), which drainsunittest.mock's process-global registry and can therefore stop patches this class never started; the fixture unwinds only its own two patchers. I also convertedtest_config.py, which makes the whole package pytest style, and I dropped a deadosimport fromtest_client.pyin an import block this change already rewrites. This is a test-only change:src/google/adk/integrations/eventarc/**is untouched.Collected-test count: 59 -> 70. The two
self.subTestloops intest_message_tool.pyeach collected as one item. Their parametrized successors collect one item per case, which adds 8 invalid-input cases and 5 timestamp cases. I diffed--collect-onlybefore and after: exactly 2 node IDs are removed and exactly 13 are added, and every added ID is an expansion of a formersubTestcase. No test is deleted, renamed, or weakened, and every class and method name is unchanged, so existing node IDs keep resolving.Collision check. I listed the 100 open pull requests on this fork and inspected the adjacent ones. #179, #180 and #146 (async marker hygiene) touch no Eventarc file. #170 does touch
test_message_tool.py: it rewrites the samesetUpto useaddCleanup, and it adds aTestMessageToolPatchHygiene(unittest.TestCase)guard. It does not migrate the class, so it is an overlap and not a duplicate. This change supersedes it, because the autouse fixture removes themock.patch.stopall()hazard that #170 guards against. I branched frommainrather than stacking, so the diff stays a pure migration. If #170 lands first, resolve the conflict by keeping this fixture and deleting the guard class: it cannot be instantiated onceTestMessageToolis no longer aTestCase.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
Commands run on the pushed commit, from the repository root:
The baseline on
mainwas 59 passed with 13 subtests passed. The strict run matters most: in strict mode pytest-asyncio does not await an async test that carries no marker. All 38 async tests pass there, so every marker is real. The run reports 0 skipped and noPytestUnhandledCoroutineWarning. The-n autorun matches the xdist setting that CI uses.Formatters are a no-op afterwards:
Proof that the converted tests can fail. Coverage is not proof, so I broke the production source once per group and confirmed the tests catch it. I reverted each mutation immediately, and
git diff main -- src/is empty.src/"status": "OK"instead of"ERROR"invalid_inputscases:AssertionError: assert 'OK' == 'ERROR'custom_attr["time"]pinned to a fixed timestamptime_valid_rfc3339casescustom_attr["time"]pinned to the first case's timestamptest_client.py; the sync test passes_get_credential_idreturns a constanttest_get_credential_id, which holds 25 converted assertsassert any(...)warning teststest_message_tool.py; the 12 survivors are the error-path testsproject_iddefault changed fromNonetest_config.py::test_valid_configproject_idwidened to accept any typetest_config.py::test_invalid_config:DID NOT RAISE ValidationErrorThe third row is the one that justifies a deletion. The old loop called
self.mock_eventarc_v1.reset_mock()on each iteration, and I removed it. Pinning the source to case 1's timestamp makes case 1 pass and cases 2 to 5 fail, which proves the mock does not leak between cases and the reset was dead code once the loop was gone.Manual End-to-End (E2E) Tests:
Not applicable. No runtime code path changes, so there is nothing to exercise against a live service. The commands above are the deliverable.
Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.
CI note
Every test job passes: Unit Tests on Python 3.10 to 3.14, A2A v0.3 Tests on Python 3.10 to 3.14, and Mypy Check on Python 3.10 to 3.13.
Pre-commit Linterfails for an unrelated reason. Theupdate-constraintshook regeneratesconstraints-3.*.txtwith a newer--exclude-newersnapshot date, so the checkout is dirty and the gate rejects it. The failure names only those generated files, and this branch touches no file outsidetests/unittests/integrations/eventarc/. The same job fails the same way on sibling pull requests that share no code with this one, for example #241 and #243. I did not fix it, because it is out of scope here.