Skip to content

Test: Convert the slack and vmaas tests from unittest.IsolatedAsyncioTestCase to pytest style - #245

Open
AmaadMartin wants to merge 1 commit into
mainfrom
feat/slack-vmaas-tests-pytest-style
Open

Test: Convert the slack and vmaas tests from unittest.IsolatedAsyncioTestCase to pytest style#245
AmaadMartin wants to merge 1 commit into
mainfrom
feat/slack-vmaas-tests-pytest-style

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A
  2. Or, if no issue exists, describe the change:
    Problem: Three test modules subclass unittest.IsolatedAsyncioTestCase. pytest does not apply fixtures or @pytest.mark.parametrize to a unittest.TestCase subclass, so anyone who adds either to these classes gets a silent no-op. The base class also awaits its own coroutines, so pytest-asyncio never sees these tests and the asyncio_mode setting does not cover them.

Solution: I converted the three modules to plain pytest classes. setUp becomes module-level fixtures for the mocks and module-level constants for the plain strings. Every self.assert* becomes a bare assert, rewritten through an AST pass so no assertion changes meaning. Every async test now carries an explicit @pytest.mark.asyncio as its outermost decorator.

Files:

  • tests/unittests/integrations/slack/test_slack_runner.py
  • tests/unittests/integrations/vmaas/test_sandbox_client.py
  • tests/unittests/integrations/vmaas/test_sandbox_computer.py

No production source file changes. No test is added, removed, renamed, skipped, or weakened. 100 self.assert* calls became bare asserts: 2 / 49 / 49 per file. Marker counts are 3 / 19 / 26.

Collection order changes, and this is unavoidable. unittest sorts test methods alphabetically; pytest collects them in source order. The set of node IDs is byte-identical before and after (verified with --collect-only -q | sort | diff), and every test is independent, so only the order differs.

Collision check: I listed the 100 open pull requests on this fork and read the diff of every plausibly adjacent one (#180, #179, #146, #177, #176, #170). None touches a file under tests/unittests/integrations/slack or .../vmaas. #180 switches pyproject.toml to asyncio_mode = strict; this change touches no config file and makes those three modules ready for that switch.

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.

Baseline before the change and result after the change, on both async modes:

uv run pytest tests/unittests/integrations/slack tests/unittests/integrations/vmaas -q
# before: 57 passed     after: 57 passed

uv run pytest tests/unittests/integrations/slack tests/unittests/integrations/vmaas -q -o asyncio_mode=strict
# before: 57 passed     after: 57 passed

uv run pytest ... --collect-only -q | grep :: | sort
# before and after are identical, 57 node IDs

Formatting gate on the three files: uv run pre-commit run --files <the three files> passes (isort, pyink, addlicense, compliance checks, codespell).

Mutation checks. This change adds no code, so I proved instead that the converted assertions and the new markers still fail when the code under them breaks. Each mutation was reverted afterwards.

Mutation Result
sandbox_client.py: CDP path "cdp" -> "cdpX" test_make_cdp_request FAILED (1 failed, 20 passed)
sandbox_computer.py: _screen_size (1280, 720) -> (1281, 720) test_init and test_screen_size FAILED (2 failed, 31 passed)
slack_runner.py: "_Thinking..._" -> "_Working..._" all 3 slack tests FAILED
slack_runner.py: error text "Sorry, I encountered an error" -> "Oops, a problem occurred" test_handle_message_error FAILED with AssertionError: assert 'Sorry, I encountered an error' in 'Oops, a problem occurred: Something went wrong'
Remove one @pytest.mark.asyncio (test_screen_size), run strict mode FAILED with async def functions are not natively supported.

The last row is the one that matters for the markers. Under the current asyncio_mode = auto a missing marker is invisible; strict mode is what proves each marker is real.

CI. All 14 test jobs pass: Unit Tests on Python 3.10 to 3.14, Mypy Check on 3.10 to 3.13, and A2A v0.3 Tests on 3.10 to 3.14.

Pre-commit Linter fails, and the cause is unrelated to this change. Every formatting hook passes (isort, pyink, addlicense, ruff, pyproject-fmt, ADK Compliance Checks, mdformat, codespell). The single failing hook is update-constraints, which regenerates constraints-3.10.txt through constraints-3.14.txt with today's --exclude-newer date instead of the recorded 2026-07-24. This change touches no dependency, no pyproject.toml, and no constraints file. The same job fails the same way on unrelated open pull requests #243, #241 and #238.

Manual End-to-End (E2E) Tests:
Not applicable. This change touches only test files and adds no runtime behaviour. The strict-mode run above is the meaningful manual check.

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.

The three test modules subclassed unittest.IsolatedAsyncioTestCase. pytest
does not apply fixtures or parametrize marks to a unittest.TestCase subclass,
so anyone adding either to these classes gets a silent no-op. The base class
also awaits its own coroutines, so pytest-asyncio never sees these tests and
asyncio_mode does not cover them.

Drop the base class, replace setUp with module-level fixtures and constants,
replace every self.assert* with a bare assert, and mark every async test with
an explicit @pytest.mark.asyncio. No test is added, removed, or renamed, and
no assertion changes meaning.
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.

1 participant