Skip to content

Conversation

@ttypic
Copy link
Contributor

@ttypic ttypic commented Dec 8, 2025

replace setUp/tearDown methods with pytest fixtures

Replaced asyncSetUp/asyncTearDown in test cases with @pytest.fixture for improved pytest integration. Added pytest-asyncio as a dependency.

Summary by CodeRabbit

  • Tests
    • Migrated many tests to use pytest autouse async fixtures and standardized pytest-asyncio, unifying async setup/teardown behavior.
  • Chores
    • Enabled pytest-asyncio and automatic asyncio mode in test config and dev dependencies.
    • Removed legacy unittest base-class usage in test utilities.
    • Updated CI matrix (removed Python 3.7, added Python 3.14).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 8, 2025

Walkthrough

Migrates tests from unittest-style async lifecycle to pytest-asyncio: adds pytest-asyncio plugin config, replaces many asyncSetUp/asyncTearDown with autouse async fixtures (yield-based teardown), removes unittest inheritance from base test classes, and updates CI matrix and dev dependencies.

Changes

Cohort / File(s) Summary
Top-level pytest config
conftest.py, test/conftest.py, pyproject.toml
Adds pytest_plugins = ('pytest_asyncio',) (in conftest.py and test/conftest.py), adds conditional pytest-asyncio dev-dependencies and sets tool.pytest.ini_options.asyncio_mode = "auto" in pyproject.toml.
Test app session fixture
test/ably/conftest.py
Removes manual event-loop fixture and adds an async, session-scoped autouse fixture test_app_setup() that awaits TestApp.get_test_vars() for setup and TestApp.clear_test_vars() for teardown.
Test utilities
test/ably/utils.py
Removes unittest/IsolatedAsyncioTestCase imports and drops inheritance from unittest.TestCase / IsolatedAsyncioTestCase for BaseTestCase and BaseAsyncTestCase.
Realtime tests
test/ably/realtime/*
test/ably/realtime/eventemitter_test.py, .../realtimechannel_test.py, .../realtimechannel_publish_test.py, .../realtimechannel_vcdiff_test.py, .../realtimeconnection_test.py, .../realtimeinit_test.py, .../realtimeresume_test.py
Replaced class asyncSetUp methods with @pytest.fixture(autouse=True) async setup fixtures; added/adjusted pytest/pytest_asyncio imports where needed; preserved existing setup logic.
REST tests (broad fixture migration)
test/ably/rest/*
test/ably/rest/encoders_test.py, .../restauth_test.py, .../restcapability_test.py, .../restchannelhistory_test.py, .../restchannelpublish_test.py, .../restchannels_test.py, .../restchannelstatus_test.py, .../restinit_test.py, .../restpaginatedresult_test.py, .../restpresence_test.py, .../restpush_test.py, .../restrequest_test.py, .../reststats_test.py, .../resttime_test.py, .../resttoken_test.py
Systematic replacement of asyncSetUp/asyncTearDown with autouse async fixtures (typically setup) that perform setup, yield, then teardown (closing clients, stopping mocks, cleanup). Imports updated to include pytest.
restcrypto special-case
test/ably/rest/restcrypto_test.py
Converted class-level setUpClass(cls) to an autouse instance fixture; moved resource and cipher initialization into the fixture and replaced cls.* references with self.*.
CI matrix
.github/workflows/check.yml
Updated CI matrix: removed Python 3.7 and added Python 3.14.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~45 minutes

  • Focus review on:
    • Correct placement of teardown logic after yield in each new fixture.
    • test/ably/rest/restcrypto_test.py conversions and thorough check of all cls.self. replacements.
    • Areas where tests previously relied on unittest.TestCase behavior (assert helpers, skip/xfail semantics, fixture-scoped teardown ordering).
    • test/ably/conftest.py session fixture scope/autouse implications.
    • pyproject.toml pytest-asyncio conditional dependency markers and asyncio_mode = "auto" config.

Poem

🐇 I hopped through tests both near and far,
I swapped asyncSetUp for fixtures that are.
Yields stitch teardown, loops rest and mend,
Pytest hums softly — rabbit-approved end. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main objective of this PR: removing unittest.TestCase dependencies and migrating to pytest fixtures. It accurately reflects the primary change across the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/update-pytest-config

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c9be051 and d319494.

📒 Files selected for processing (1)
  • test/conftest.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: check (3.14)
  • GitHub Check: check (3.13)
  • GitHub Check: check (3.9)
  • GitHub Check: check (3.8)
  • GitHub Check: check (3.10)
  • GitHub Check: check (3.11)
  • GitHub Check: check (3.12)
🔇 Additional comments (1)
test/conftest.py (1)

1-4: LGTM! pytest-asyncio is properly configured.

The plugin loading approach is correct and follows pytest conventions. The pytest_plugins tuple syntax is the standard way to enable pytest-asyncio across the test suite. pytest-asyncio is correctly added to dev dependencies in pyproject.toml with appropriate version constraints for Python 3.7 and 3.8+, and asyncio_mode = "auto" is properly configured in [tool.pytest.ini_options].


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot temporarily deployed to staging/pull/647/features December 8, 2025 13:26 Inactive
@ttypic ttypic force-pushed the feat/update-pytest-config branch from b73ad72 to 77430b9 Compare December 8, 2025 13:28
@github-actions github-actions bot temporarily deployed to staging/pull/647/features December 8, 2025 13:28 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
test/ably/rest/reststats_test.py (2)

26-74: Add teardown to the setup fixture.

The setup fixture is missing a yield and teardown section. The asyncTearDown method at lines 76-78 will not be called by pytest.

Apply this diff to add proper teardown:

         await self.ably.http.post('/stats', body=stats + previous_stats)
         TestRestAppStatsSetup.__stats_added = True
+        yield
+        await self.ably.close()
+        await self.ably_text.close()

76-78: Remove obsolete asyncTearDown method.

This asyncTearDown method will not be called by pytest. The teardown should be moved into the setup fixture after a yield statement (see comment on lines 26-74).

Apply this diff to remove the obsolete method:

-    async def asyncTearDown(self):
-        await self.ably.close()
-        await self.ably_text.close()
-
🧹 Nitpick comments (5)
test/ably/rest/restinit_test.py (1)

15-17: Consider adding yield for consistency with other fixtures.

This fixture lacks a yield statement, unlike other setup fixtures in this PR (e.g., restcapability_test.py, restchannelhistory_test.py). While technically correct since there's no teardown needed, adding yield at the end maintains a consistent pattern and makes it clearer this is a setup-only fixture.

     @pytest.fixture(autouse=True)
     async def setup(self):
         self.test_vars = await TestApp.get_test_vars()
+        yield
test/ably/rest/restpush_test.py (1)

24-44: Consider using scope="class" for this heavy fixture.

This fixture registers 10 devices and multiple subscriptions before each test method, which could significantly slow down the test suite. Since the tests appear to read from these shared resources without modifying them destructively, a class-scoped fixture could improve performance:

-    @pytest.fixture(autouse=True)
+    @pytest.fixture(autouse=True, scope="class")
     async def setup(self):

However, verify that tests don't have interdependencies or modify shared state in ways that would cause flakiness with shared setup.

test/ably/rest/restcrypto_test.py (1)

204-217: Consider adding scope='class' to preserve original behavior.

The original setUpClass was a classmethod that ran once per test class. Converting it to a function-scoped fixture changes it to run once per test method, which means the JSON file will be read and parsed repeatedly. Since the tests don't modify this state, consider adding scope='class' to preserve the original performance characteristics:

-    @pytest.fixture(autouse=True)
+    @pytest.fixture(autouse=True, scope='class')
     def setUpClass(self):

Note: This would require the test classes to be compatible with class-scoped fixtures (typically works fine for read-only fixture data).

test/ably/realtime/realtimechannel_vcdiff_test.py (1)

36-48: Consider adding yield for consistency.

While this fixture doesn't require teardown logic, adding a yield statement would maintain consistency with the fixture pattern used throughout the test suite and make the fixture structure more explicit:

     async def setup(self):
         self.test_vars = await TestApp.get_test_vars()
         self.valid_key_format = "api:key"
         
         # Test data equivalent to JavaScript version
         self.test_data = [
             {'foo': 'bar', 'count': 1, 'status': 'active'},
             {'foo': 'bar', 'count': 2, 'status': 'active'},
             {'foo': 'bar', 'count': 2, 'status': 'inactive'},
             {'foo': 'bar', 'count': 3, 'status': 'inactive'},
             {'foo': 'bar', 'count': 3, 'status': 'active'},
         ]
+        yield
test/ably/realtime/realtimechannel_test.py (1)

15-19: Channel tests use the standard async setup; consider de-duplicating later

The async autouse setup fixture here correctly initializes test_vars and valid_key_format for each test, matching other realtime suites. Since this same body now appears in multiple classes, you might later factor it into a shared fixture or BaseAsyncTestCase helper to avoid repetition, but that can be a follow-up.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9005b4e and b73ad72.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (27)
  • conftest.py (1 hunks)
  • pyproject.toml (2 hunks)
  • test/ably/conftest.py (1 hunks)
  • test/ably/realtime/eventemitter_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_vcdiff_test.py (2 hunks)
  • test/ably/realtime/realtimeconnection_test.py (1 hunks)
  • test/ably/realtime/realtimeinit_test.py (1 hunks)
  • test/ably/realtime/realtimeresume_test.py (2 hunks)
  • test/ably/rest/encoders_test.py (5 hunks)
  • test/ably/rest/restauth_test.py (7 hunks)
  • test/ably/rest/restcapability_test.py (1 hunks)
  • test/ably/rest/restchannelhistory_test.py (1 hunks)
  • test/ably/rest/restchannelpublish_test.py (2 hunks)
  • test/ably/rest/restchannels_test.py (1 hunks)
  • test/ably/rest/restchannelstatus_test.py (2 hunks)
  • test/ably/rest/restcrypto_test.py (2 hunks)
  • test/ably/rest/restinit_test.py (1 hunks)
  • test/ably/rest/restpaginatedresult_test.py (3 hunks)
  • test/ably/rest/restpresence_test.py (2 hunks)
  • test/ably/rest/restpush_test.py (2 hunks)
  • test/ably/rest/restrequest_test.py (2 hunks)
  • test/ably/rest/reststats_test.py (1 hunks)
  • test/ably/rest/resttime_test.py (1 hunks)
  • test/ably/rest/resttoken_test.py (2 hunks)
  • test/ably/utils.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (18)
test/ably/rest/encoders_test.py (3)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/resttime_test.py (1)
  • setup (17-20)
test/ably/rest/restrequest_test.py (5)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/conftest.py (1)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_test_vars (41-70)
  • clear_test_vars (105-115)
test/ably/rest/reststats_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restpush_test.py (3)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/realtime/realtimeresume_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/rest/restpresence_test.py (6)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restauth_test.py (5)
test/ably/rest/restchannelpublish_test.py (3)
  • setup (30-37)
  • setup (454-459)
  • get_ably_rest (533-535)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restrequest_test.py (1)
  • setup (16-27)
test/ably/rest/resttime_test.py (1)
  • setup (17-20)
test/ably/realtime/realtimechannel_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/realtime/realtimeconnection_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/realtime/eventemitter_test.py (3)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/realtime/realtimeinit_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/rest/restchannelhistory_test.py (2)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restpaginatedresult_test.py (6)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restpresence_test.py (1)
  • setup (15-22)
test/ably/rest/restchannelpublish_test.py (4)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restchannelstatus_test.py (3)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/testapp.py (2)
  • TestApp (37-115)
  • get_ably_rest (73-77)
test/ably/rest/restchannels_test.py (2)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/resttoken_test.py (5)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
🪛 Gitleaks (8.30.0)
test/ably/rest/encoders_test.py

[high] 150-150: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 354-354: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: check (3.9)
  • GitHub Check: check (3.10)
  • GitHub Check: check (3.11)
  • GitHub Check: check (3.12)
  • GitHub Check: check (3.8)
  • GitHub Check: check (3.13)
🔇 Additional comments (34)
conftest.py (1)

1-5: LGTM!

The configuration correctly enables pytest-asyncio plugin globally for the test suite.

pyproject.toml (2)

45-46: LGTM!

The pytest-asyncio dependency is correctly added with appropriate version constraints for different Python versions.


96-96: LGTM!

Setting asyncio_mode = "auto" enables automatic detection of async tests, which is appropriate for this migration.

test/ably/rest/restcapability_test.py (1)

11-16: LGTM!

The migration from asyncSetUp/asyncTearDown to pytest fixture is correctly implemented with proper setup, yield for teardown separation, and cleanup.

test/ably/rest/restauth_test.py (5)

29-31: LGTM!

The fixture correctly initializes test_vars without requiring teardown.


171-176: LGTM!

The fixture properly handles setup and teardown with yield, ensuring the ably client is closed after each test.


326-328: LGTM!

The fixture correctly initializes test_vars without requiring teardown.


485-532: LGTM!

The complex fixture correctly sets up mock API routes, starts interception, and properly cleans up resources after yield.


588-637: LGTM!

The fixture properly manages mock setup and teardown, ensuring cleanup after each test.

test/ably/rest/restpresence_test.py (2)

14-22: LGTM!

The fixture correctly handles setup and teardown with proper resource cleanup.


191-198: LGTM!

The fixture properly manages channel lifecycle and client cleanup.

test/ably/utils.py (2)

17-17: LGTM!

Removing unittest.TestCase inheritance is correct for the pytest migration, as test lifecycle is now managed by pytest fixtures.


36-36: LGTM!

Removing IsolatedAsyncioTestCase inheritance is correct, as pytest-asyncio now handles async test execution.

test/ably/rest/encoders_test.py (4)

25-29: LGTM!

The fixture correctly initializes the ably client with appropriate teardown.


147-152: LGTM!

The fixture properly sets up encryption parameters for testing. The cipher key is a test fixture value, not a real secret.


260-264: LGTM!

The fixture correctly initializes the binary protocol client with proper cleanup.


351-356: LGTM!

The fixture properly configures encryption for binary encoding tests. The cipher key is a test fixture value.

test/ably/rest/restchannelstatus_test.py (1)

13-17: LGTM!

The fixture correctly replaces the unittest-style asyncSetUp/asyncTearDown pattern. The yield properly separates setup from teardown, ensuring self.ably.close() runs after each test. This is consistent with other test files in the PR.

test/ably/rest/restchannels_test.py (1)

15-20: LGTM!

The fixture correctly implements the setup/teardown pattern, initializing test_vars and ably before yielding, then properly closing the ably client after each test. This is consistent with the pattern used across other test files in this PR.

test/ably/rest/resttime_test.py (1)

16-20: LGTM!

The fixture correctly implements the async setup/teardown pattern. The ably client is properly initialized before yielding and closed after each test completes.

test/ably/rest/restchannelhistory_test.py (1)

16-21: LGTM: Clean migration to pytest fixture.

The migration from asyncSetUp/asyncTearDown to an autouse pytest fixture is correctly implemented. The fixture properly initializes resources, yields control to the test, and ensures cleanup via await self.ably.close().

test/ably/conftest.py (1)

6-10: LGTM: Session-scoped fixture correctly implemented.

The session-scoped fixture properly manages test application lifecycle, ensuring TestApp.get_test_vars() is called once before all tests and TestApp.clear_test_vars() is called after all tests complete. Using pytest_asyncio.fixture for the session-scoped async fixture is appropriate.

test/ably/rest/restrequest_test.py (1)

15-27: LGTM: Fixture correctly handles setup with test data population.

The autouse fixture properly initializes the test client and pre-populates 20 test messages before yielding to the test. Cleanup is correctly handled after yield.

test/ably/rest/restpaginatedresult_test.py (1)

30-68: LGTM: Complex fixture setup handled correctly.

The autouse fixture properly manages mocked API setup with respx, including starting/stopping the mock and initializing paginated result instances. The teardown sequence (stop, reset, close) is correct.

test/ably/rest/restchannelpublish_test.py (2)

29-37: LGTM: Multi-client fixture correctly implemented.

The fixture properly initializes multiple Ably clients (self.ably and self.ably_with_client_id) and ensures both are closed in the teardown phase.


453-459: LGTM: Consistent multi-client fixture pattern.

This fixture follows the same correct pattern as the first class, managing two client instances with proper cleanup.

test/ably/rest/resttoken_test.py (2)

22-28: LGTM: Standard fixture pattern correctly applied.

The autouse fixture properly initializes the Ably client and test configuration, with cleanup handled after yield.


160-166: LGTM: Consistent fixture implementation.

The fixture correctly follows the established pattern for test setup and teardown.

test/ably/rest/restcrypto_test.py (1)

21-28: LGTM: Standard fixture pattern correctly implemented.

The autouse fixture properly manages two Ably client instances with appropriate cleanup.

test/ably/realtime/realtimechannel_publish_test.py (1)

17-20: Async autouse fixture correctly replaces per-test asyncSetUp

setup just initializes self.test_vars via TestApp.get_test_vars() before each test, matching the prior async setup semantics without altering test logic. Looks good.

test/ably/realtime/eventemitter_test.py (1)

3-4: Fixture + import wiring for TestEventEmitter is consistent and correct

Adding import pytest and the async autouse setup fixture that populates self.test_vars aligns this class with the rest of the realtime tests and preserves the previous setup behavior.

Also applies to: 11-13

test/ably/realtime/realtimeconnection_test.py (1)

14-18: Per-test connection setup preserved via async autouse fixture

The new setup fixture re-establishes self.test_vars and self.valid_key_format before each test, matching the old asyncSetUp behavior and supporting tests that depend on these attributes without changing their logic.

test/ably/realtime/realtimeresume_test.py (1)

3-4: Resume tests now use the shared async setup pattern

The added pytest import and async autouse setup fixture correctly mirror the common realtime setup (initializing test_vars and valid_key_format) without impacting the existing resume test logic.

Also applies to: 27-30

test/ably/realtime/realtimeinit_test.py (1)

13-17: Init tests correctly migrated to async autouse fixture

The new setup fixture restores test_vars and valid_key_format for each test, preserving the original initialization semantics while aligning with the pytest fixture approach used elsewhere in the suite.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
test/ably/rest/restcrypto_test.py (1)

204-218: Abstract crypto fixture behavior is correct; consider class scope for efficiency

The new autouse fixture on AbstractTestCryptoWithFixture correctly prepares self.fixture, self.params, self.cipher_params, self.cipher, and self.items for each test. Since this initialization is pure and identical across tests in a class, you could make it a class-scoped fixture to better match the old setUpClass behavior:

-    @pytest.fixture(autouse=True)
-    def setUpClass(self):
+    @pytest.fixture(scope='class', autouse=True)
+    def setUpClass(self):

This is purely an efficiency/readability tweak; current behavior is functionally fine.

If you change the scope, re-run the crypto fixture tests to ensure pytest still discovers and applies the fixture to both TestCryptoWithFixture128 and TestCryptoWithFixture256.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b73ad72 and 77430b9.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (27)
  • conftest.py (1 hunks)
  • pyproject.toml (2 hunks)
  • test/ably/conftest.py (1 hunks)
  • test/ably/realtime/eventemitter_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_vcdiff_test.py (2 hunks)
  • test/ably/realtime/realtimeconnection_test.py (1 hunks)
  • test/ably/realtime/realtimeinit_test.py (1 hunks)
  • test/ably/realtime/realtimeresume_test.py (2 hunks)
  • test/ably/rest/encoders_test.py (5 hunks)
  • test/ably/rest/restauth_test.py (7 hunks)
  • test/ably/rest/restcapability_test.py (1 hunks)
  • test/ably/rest/restchannelhistory_test.py (1 hunks)
  • test/ably/rest/restchannelpublish_test.py (2 hunks)
  • test/ably/rest/restchannels_test.py (1 hunks)
  • test/ably/rest/restchannelstatus_test.py (2 hunks)
  • test/ably/rest/restcrypto_test.py (2 hunks)
  • test/ably/rest/restinit_test.py (1 hunks)
  • test/ably/rest/restpaginatedresult_test.py (3 hunks)
  • test/ably/rest/restpresence_test.py (2 hunks)
  • test/ably/rest/restpush_test.py (2 hunks)
  • test/ably/rest/restrequest_test.py (2 hunks)
  • test/ably/rest/reststats_test.py (2 hunks)
  • test/ably/rest/resttime_test.py (1 hunks)
  • test/ably/rest/resttoken_test.py (2 hunks)
  • test/ably/utils.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
  • test/ably/realtime/realtimechannel_test.py
  • test/ably/realtime/realtimeconnection_test.py
  • test/ably/realtime/realtimechannel_vcdiff_test.py
  • test/ably/rest/restchannels_test.py
  • test/ably/rest/resttoken_test.py
  • test/ably/realtime/realtimechannel_publish_test.py
  • test/ably/rest/restchannelpublish_test.py
  • test/ably/realtime/realtimeresume_test.py
  • test/ably/rest/restpush_test.py
  • test/ably/rest/restchannelstatus_test.py
  • test/ably/rest/restpresence_test.py
  • test/ably/rest/restcapability_test.py
🧰 Additional context used
🧬 Code graph analysis (9)
test/ably/conftest.py (1)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_test_vars (41-70)
  • clear_test_vars (105-115)
test/ably/rest/restchannelhistory_test.py (4)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_ably_rest (73-77)
  • get_test_vars (41-70)
test/ably/rest/restcrypto_test.py (1)
ably/util/crypto.py (2)
  • CipherParams (16-42)
  • get_cipher (163-168)
test/ably/rest/encoders_test.py (1)
ably/util/crypto.py (4)
  • CipherParams (16-42)
  • secret_key (29-30)
  • secret_key (110-111)
  • algorithm (25-26)
test/ably/realtime/realtimeinit_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/rest/restauth_test.py (6)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restrequest_test.py (1)
  • setup (16-27)
test/ably/rest/resttime_test.py (1)
  • setup (17-20)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_ably_rest (73-77)
  • get_test_vars (41-70)
test/ably/rest/resttime_test.py (6)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/realtime/eventemitter_test.py (5)
test/ably/utils.py (1)
  • BaseAsyncTestCase (36-51)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restpaginatedresult_test.py (8)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpresence_test.py (1)
  • setup (15-22)
🪛 Gitleaks (8.30.0)
test/ably/rest/encoders_test.py

[high] 150-150: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 354-354: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: check (3.10)
  • GitHub Check: check (3.12)
  • GitHub Check: check (3.13)
  • GitHub Check: check (3.11)
  • GitHub Check: check (3.8)
  • GitHub Check: check (3.9)
🔇 Additional comments (17)
pyproject.toml (1)

45-46: LGTM! pytest-asyncio configuration is correctly set up.

The pytest-asyncio dependency is added with appropriate version constraints for Python 3.7 and 3.8+, and asyncio_mode = "auto" enables automatic detection of async tests, which aligns well with the fixture-based migration across the test suite.

Also applies to: 96-96

conftest.py (1)

1-5: LGTM! Proper pytest-asyncio plugin configuration.

The module-level plugin loading ensures pytest-asyncio is available for all async fixtures and tests throughout the suite.

test/ably/realtime/eventemitter_test.py (1)

11-13: LGTM! Clean migration to pytest fixture.

The autouse fixture properly replaces asyncSetUp. No teardown is needed for this test class, so the absence of yield is appropriate.

test/ably/rest/encoders_test.py (1)

25-29: LGTM! Fixtures correctly implement setup and teardown.

All four test classes properly migrate to pytest fixtures with yield-based teardown to close the Ably client. The pattern is consistent and correct.

Note: The static analysis warnings about "Generic API Keys" on lines 150 and 354 are false positives—these are hardcoded test cipher keys (keyfordecrypt_16), not real secrets.

Also applies to: 147-152, 260-264, 351-356

test/ably/realtime/realtimeinit_test.py (1)

13-16: LGTM! Proper fixture-based setup.

The migration from asyncSetUp to an autouse fixture is correct and consistent with other realtime test files in the PR.

test/ably/utils.py (1)

17-17: LGTM! Base classes properly decoupled from unittest.

Removing unittest inheritance completes the migration to pytest-only test infrastructure. The classes now serve as pure mixins providing utility methods, which is the appropriate pattern for pytest-based tests.

Also applies to: 36-36

test/ably/rest/restinit_test.py (1)

15-17: LGTM! Straightforward migration to pytest fixture.

The autouse fixture properly replaces asyncSetUp with minimal changes to the initialization logic.

test/ably/rest/restrequest_test.py (1)

15-27: LGTM! Fixture properly handles complex setup and teardown.

The autouse fixture correctly migrates the test lifecycle, pre-populating test data in the setup phase before yield, and cleaning up the Ably client in the teardown phase after yield. This is the correct pattern for fixtures that need both initialization and cleanup.

test/ably/rest/restpaginatedresult_test.py (1)

1-1: Pytest-asyncio fixture migration for paginated result tests looks correct

The new async autouse fixture sets up the Ably client and respx routes once per test and reliably tears everything down via yield, matching the previous setup/teardown semantics while integrating cleanly with pytest.

Please run this test module under the new pytest-asyncio configuration to confirm the async fixture is discovered correctly and the respx mocks behave as expected.

Also applies to: 30-69

test/ably/rest/resttime_test.py (1)

16-20: Async autouse fixture for time tests is consistent and safe

The new async autouse fixture correctly owns a single AblyRest instance per test and closes it after yield, while tests that create extra clients still close them explicitly. This keeps resource lifetimes clear and matches the previous lifecycle.

Please run this module with the updated pytest-asyncio config to confirm the async fixture is executed for both async and sync tests as intended.

test/ably/conftest.py (1)

1-1: Session-scoped async fixture for TestApp lifecycle looks appropriate

The test_app_setup session fixture cleanly initialises TestApp state once via get_test_vars() and tears it down with clear_test_vars() after all tests. That matches the intended global test-app lifecycle and fits the pytest-asyncio model.

Please confirm that pytest-asyncio is enabled in your pytest configuration so this session-scoped async fixture runs correctly and that no other code calls TestApp.clear_test_vars() independently during the session.

Also applies to: 6-10

test/ably/rest/restauth_test.py (5)

29-32: TestAuth async fixture correctly centralizes test_vars setup

The new async autouse fixture that awaits TestApp.get_test_vars() before each test keeps self.test_vars initialisation consistent without introducing additional teardown requirements.

Please verify pytest-asyncio is correctly handling this async fixture (no warnings about unawaited coroutines) when you run just the TestAuth tests.


171-176: AuthAuthorize fixture correctly manages a shared Ably client per test

The async autouse fixture now owns creation and teardown of self.ably, while tests that need extra clients still manage those explicitly. Reassignments of self.ably in specific tests are handled by the fixture’s final await self.ably.close(), preserving previous teardown semantics.

Please run the TestAuthAuthorize suite under pytest to ensure there are no lingering open connections or double-close issues around self.ably.


326-329: RequestToken fixture mirrors other test_vars-only setups

Using an async autouse fixture here to populate self.test_vars matches the pattern in TestAuth; since Ably clients are created and closed inside individual tests, no teardown logic is needed in the fixture.

Confirm that TestRequestToken tests still pass and that no test implicitly depended on a class-level asyncSetUp beyond test_vars initialisation.


485-533: RenewToken fixture accurately wraps Ably client and respx router lifecycle

The new async autouse fixture sets up self.ably, the mocked API, and routes, then tears them down via yield, including stopping/resetting the router and closing whatever self.ably currently refers to. This preserves the previous behavior where tests can rebind self.ably and rely on teardown to close the final instance.

Please re-run the TestRenewToken tests and watch for any unexpected extra calls to the mocked routes, which would indicate fixture scope or teardown issues.


588-590: RenewExpiredToken fixture correctly scopes respx mocks per test

The async fixture constructs the mock router and routes, starts it, then stops and resets it after yield. Since Ably clients are created and closed inside the tests themselves, this separation of responsibilities keeps the mocking layer and client lifecycle clean.

Run just the TestRenewExpiredToken tests to confirm the router isn’t leaking state between tests (e.g., publish_attempts and time_route responses behave as expected on each run).

Also applies to: 635-637

test/ably/rest/restcrypto_test.py (1)

21-28: RestCrypto async fixture correctly manages two Ably clients per test

The async autouse fixture now ensures both self.ably and self.ably2 are created before each test and reliably closed after yield, which is a faithful translation of the previous asyncSetUp/asyncTearDown pattern.

Please run the crypto REST tests with pytest-asyncio enabled to confirm that no unclosed client warnings or connection leaks appear.

@ttypic ttypic force-pushed the feat/update-pytest-config branch from 77430b9 to 1f9ac31 Compare December 8, 2025 14:01
@github-actions github-actions bot temporarily deployed to staging/pull/647/features December 8, 2025 14:02 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
test/ably/realtime/realtimeinit_test.py (1)

13-17: Async autouse fixture migration looks correct; verify pytest-asyncio integration and BaseAsyncTestCase inheritance

The async autouse setup fixture reproduces the old async setup behavior and keeps the tests readable. Two small follow‑ups:

  1. Since this is an async fixture, consider using pytest_asyncio.fixture (if that’s what you use elsewhere) to make the asyncio handling explicit and avoid relying on implicit plugin behavior. For example:
-import pytest
+import pytest
+import pytest_asyncio
@@
-class TestRealtimeInit(BaseAsyncTestCase):
-    @pytest.fixture(autouse=True)
+class TestRealtimeInit(BaseAsyncTestCase):
+    @pytest_asyncio.fixture(autouse=True)
     async def setup(self):
         self.test_vars = await TestApp.get_test_vars()
         self.valid_key_format = "api:key"
  1. Please double‑check that BaseAsyncTestCase no longer subclasses unittest.TestCase (or IsolatedAsyncioTestCase). If it still does, pytest may treat this as a unittest‑style test case and ignore the fixture, which would break the new setup strategy.
test/ably/realtime/realtimechannel_vcdiff_test.py (1)

4-4: Async autouse fixture migration looks good; please confirm pytest‑asyncio mode

Using an async autouse fixture on the test class is a clean replacement for the old async setup and keeps all per‑test initialization local. Given that the fixture is declared with @pytest.fixture (not @pytest_asyncio.fixture), please double‑check that pytest‑asyncio is configured to treat async fixtures defined this way as asyncio‑driven (e.g., asyncio_mode = auto), otherwise the coroutine may not be awaited in strict mode and self.test_vars/self.test_data won’t be initialized as expected. If you intend to stay in strict mode, consider switching this (and the similar patterns in the other test modules) to @pytest_asyncio.fixture for clarity and robustness.

Also applies to: 36-38

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 77430b9 and 1f9ac31.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (28)
  • .github/workflows/check.yml (1 hunks)
  • conftest.py (1 hunks)
  • pyproject.toml (2 hunks)
  • test/ably/conftest.py (1 hunks)
  • test/ably/realtime/eventemitter_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_vcdiff_test.py (2 hunks)
  • test/ably/realtime/realtimeconnection_test.py (1 hunks)
  • test/ably/realtime/realtimeinit_test.py (1 hunks)
  • test/ably/realtime/realtimeresume_test.py (2 hunks)
  • test/ably/rest/encoders_test.py (5 hunks)
  • test/ably/rest/restauth_test.py (7 hunks)
  • test/ably/rest/restcapability_test.py (1 hunks)
  • test/ably/rest/restchannelhistory_test.py (1 hunks)
  • test/ably/rest/restchannelpublish_test.py (2 hunks)
  • test/ably/rest/restchannels_test.py (1 hunks)
  • test/ably/rest/restchannelstatus_test.py (2 hunks)
  • test/ably/rest/restcrypto_test.py (2 hunks)
  • test/ably/rest/restinit_test.py (1 hunks)
  • test/ably/rest/restpaginatedresult_test.py (3 hunks)
  • test/ably/rest/restpresence_test.py (2 hunks)
  • test/ably/rest/restpush_test.py (2 hunks)
  • test/ably/rest/restrequest_test.py (2 hunks)
  • test/ably/rest/reststats_test.py (2 hunks)
  • test/ably/rest/resttime_test.py (1 hunks)
  • test/ably/rest/resttoken_test.py (2 hunks)
  • test/ably/utils.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
  • test/ably/realtime/realtimechannel_test.py
  • conftest.py
  • test/ably/rest/restrequest_test.py
  • test/ably/realtime/realtimechannel_publish_test.py
  • test/ably/rest/restchannelpublish_test.py
  • pyproject.toml
  • test/ably/rest/restcapability_test.py
  • test/ably/rest/resttoken_test.py
  • test/ably/realtime/realtimeresume_test.py
  • test/ably/rest/restchannelstatus_test.py
  • test/ably/rest/restcrypto_test.py
  • test/ably/realtime/eventemitter_test.py
  • test/ably/conftest.py
🧰 Additional context used
🧬 Code graph analysis (10)
test/ably/rest/restchannelhistory_test.py (5)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restinit_test.py (7)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restrequest_test.py (1)
  • setup (16-27)
test/ably/rest/restpaginatedresult_test.py (7)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpresence_test.py (1)
  • setup (15-22)
test/ably/realtime/realtimechannel_vcdiff_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/resttime_test.py (8)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/testapp.py (2)
  • TestApp (37-115)
  • get_ably_rest (73-77)
test/ably/rest/restpresence_test.py (5)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restchannels_test.py (7)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restpresence_test.py (1)
  • setup (15-22)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_test_vars (41-70)
  • get_ably_rest (73-77)
test/ably/realtime/realtimeinit_test.py (9)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (1)
  • setup (30-37)
test/ably/realtime/realtimeconnection_test.py (4)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (8)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restrequest_test.py (1)
  • setup (16-27)
test/ably/rest/resttime_test.py (1)
  • setup (17-20)
test/ably/testapp.py (2)
  • TestApp (37-115)
  • get_ably_rest (73-77)
ably/util/crypto.py (4)
  • CipherParams (16-42)
  • secret_key (29-30)
  • secret_key (110-111)
  • algorithm (25-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: check (3.10)
  • GitHub Check: check (3.8)
  • GitHub Check: check (3.11)
  • GitHub Check: check (3.13)
  • GitHub Check: check (3.9)
  • GitHub Check: check (3.12)
  • GitHub Check: check (3.14)
🔇 Additional comments (22)
test/ably/realtime/realtimeconnection_test.py (1)

14-15: LGTM! Clean migration from unittest to pytest fixtures.

The migration from asyncSetUp to pytest's @pytest.fixture(autouse=True) is correct and maintains the same per-test setup behavior. The pattern is consistent with other test files in this PR (eventemitter_test.py, realtimechannel_test.py, realtimeinit_test.py, realtimeresume_test.py).

test/ably/rest/restpresence_test.py (1)

191-198: LGTM!

The fixture correctly follows the pytest pattern with proper resource cleanup in the teardown phase (after yield). The teardown order is also correct: releasing the channel before closing the client.

test/ably/rest/encoders_test.py (5)

8-8: LGTM! Pytest import added for fixture decorators.

The pytest import is correctly added to support the @pytest.fixture decorators used throughout this file.


25-29: LGTM! Correctly migrated to pytest fixture.

The migration from asyncSetUp/asyncTearDown to @pytest.fixture(autouse=True) is correct:

  • autouse=True ensures the fixture runs before each test method
  • Yield-based teardown properly closes the Ably client
  • Pattern is consistent with other migrated test files in the codebase

147-152: LGTM! Fixture correctly initializes encryption test dependencies.

The fixture properly initializes both the Ably client and cipher parameters needed for encryption tests. The migration pattern is consistent with the rest of the file.


260-264: LGTM! Binary protocol setup correctly migrated.

The fixture correctly uses the default binary protocol (no use_binary_protocol=False override), which is appropriate for this test class. The migration pattern is correct.


351-356: LGTM! Encryption with binary protocol correctly configured.

The fixture properly combines binary protocol encoding with encryption setup. The migration maintains the same initialization logic while adopting the pytest fixture pattern.

test/ably/utils.py (2)

17-17: LGTM! Base class migration aligns with pytest fixtures.

The removal of unittest.TestCase inheritance is correct and necessary for the pytest-based fixture approach adopted throughout the test suite.


36-36: LGTM! Async base class migration aligns with pytest-asyncio.

The removal of IsolatedAsyncioTestCase inheritance is correct and aligns with the pytest-asyncio fixture pattern used across the test suite.

test/ably/rest/restpush_test.py (1)

24-44: LGTM! Proper pytest-asyncio fixture migration.

The async fixture correctly:

  • Initializes test state (Ably client, devices, subscriptions) before yield
  • Performs teardown (removes subscriptions, devices, closes client) after yield
  • Uses autouse=True for automatic application to all test methods

This is a clean migration from unittest-style asyncSetUp/asyncTearDown to pytest fixtures.

test/ably/rest/restchannels_test.py (1)

15-20: LGTM! Clean async fixture migration.

The setup fixture correctly initializes test_vars and ably client, then performs proper cleanup via yield-based teardown. This matches the pytest-asyncio pattern used consistently across the test suite.

test/ably/rest/restpaginatedresult_test.py (1)

30-68: LGTM! Comprehensive async fixture with proper mock cleanup.

The setup fixture correctly:

  • Initializes the Ably client with mocked HTTP routes
  • Starts the mock interception
  • Performs initial paginated queries
  • Executes teardown (stops mocks, resets, closes client) after yield

This is a thorough migration from unittest-style lifecycle to pytest fixtures.

test/ably/rest/restinit_test.py (1)

15-17: LGTM! Minimal fixture appropriate for initialization tests.

The setup fixture only initializes test_vars without teardown, which is appropriate since most tests in this file create and clean up their own short-lived Ably instances (e.g., lines 166-180, 206-216).

test/ably/rest/resttime_test.py (1)

16-20: LGTM! Proper async fixture with cleanup.

The setup fixture correctly initializes the Ably client and ensures proper cleanup via yield-based teardown. This aligns with the pytest-asyncio pattern used throughout the test suite.

test/ably/rest/restchannelhistory_test.py (1)

16-21: LGTM! Clean async fixture migration.

The setup fixture correctly initializes the Ably client and test_vars, then performs proper cleanup via yield-based teardown. This matches the pytest-asyncio pattern used consistently across the test suite.

.github/workflows/check.yml (1)

21-21: Remove Python 3.14 from the test matrix or ensure httpcore >= 1.0.8 is installed.

Python 3.14 was officially released on October 7, 2025, and GitHub Actions fully supports it. However, httpx requires httpcore >= 1.0.8 to avoid an AttributeError on Python 3.14, and some edge-case compatibility issues remain. pytest-asyncio supports Python 3.14 as of v1.3.0. Either pin httpcore to a compatible version in your dependencies or remove Python 3.14 from the test matrix until downstream issues are fully resolved.

test/ably/rest/reststats_test.py (1)

26-27: Async autouse fixture now correctly handles both one‑time seeding and per‑test teardown

The new setup fixture creates both REST clients, seeds stats only once via if not TestRestAppStatsSetup.__stats_added, and always reaches the yield, so self.ably.close() / self.ably_text.close() run for every test instance. This fixes the earlier leak caused by returning before teardown while preserving the “POST only once” behavior.

If you plan to run these tests under xdist or other parallelization, it may be worth double‑checking that the shared __stats_added flag and /stats seeding remain acceptable under concurrent execution.

Also applies to: 71-76

test/ably/rest/restauth_test.py (5)

29-31: TestAuth async fixture cleanly replaces asyncSetUp

The autouse async setup fixture that just populates self.test_vars is a straightforward drop‑in for the old asyncSetUp with no teardown needs; usage by the synchronous and async tests below remains unchanged.


171-176: TestAuthAuthorize fixture correctly manages self.ably lifecycle

The async autouse setup fixture now creates self.ably and self.test_vars and tears down via yield by always calling await self.ably.close() post‑test. This keeps behavior equivalent to the former asyncSetUp/asyncTearDown while ensuring cleanup still happens even if a test fails, and remains compatible with VaryByProtocolTestsMetaclass / per_protocol_setup.


326-329: TestRequestToken async fixture mirrors prior asyncSetUp semantics

Here the autouse async setup fixture only loads self.test_vars, matching the previous per‑test initialization without introducing new teardown responsibilities; the token‑request tests below instantiate and close their own Ably clients as before.


485-487: TestRenewToken fixture correctly owns Ably + respx router lifecycle

The new async autouse setup fixture now encapsulates creation of self.ably, the respx mock router, and related routes, then uses yield to guarantee self.mocked_api.stop(quiet=True), reset(), and await self.ably.close() are called after each test. This preserves the previous behavior while making cleanup robust to test failures, including cases where individual tests reassign self.ably.

Also applies to: 528-533


588-591: TestRenewExpiredToken fixture provides robust respx teardown

The async autouse setup fixture initializes the respx router and routes, starts it once per test, and the post‑yield teardown (stop(quiet=True) + reset()) ensures no cross‑test leakage of HTTP mocks. Ably clients remain local to tests and are explicitly closed there, so responsibilities are cleanly separated.

Also applies to: 633-637

@ttypic ttypic requested a review from owenpearson December 8, 2025 15:06
replace `setUp`/`tearDown` methods with pytest fixtures

Replaced `asyncSetUp`/`asyncTearDown` in test cases with `@pytest.fixture` for improved pytest integration. Added `pytest-asyncio` as a dependency. Updated version bump to `2.1.3`.
@ttypic ttypic force-pushed the feat/update-pytest-config branch from 1f9ac31 to c9be051 Compare December 8, 2025 15:14
@github-actions github-actions bot temporarily deployed to staging/pull/647/features December 8, 2025 15:15 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
test/ably/rest/restpresence_test.py (1)

14-22: Move channel release to teardown phase.

The channel is released on line 21 during setup (before yield), but resource cleanup should occur in the teardown phase (after yield). This is inconsistent with TestPresenceCrypt (line 197) which correctly releases the channel after yield.

🧹 Nitpick comments (1)
conftest.py (1)

1-2: Optional: Remove leading blank lines.

The two blank lines at the top of the file are unnecessary.

Apply this diff to clean up:

-
-
 # Configure pytest-asyncio
 pytest_plugins = ('pytest_asyncio',)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1f9ac31 and c9be051.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (28)
  • .github/workflows/check.yml (1 hunks)
  • conftest.py (1 hunks)
  • pyproject.toml (2 hunks)
  • test/ably/conftest.py (1 hunks)
  • test/ably/realtime/eventemitter_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_test.py (1 hunks)
  • test/ably/realtime/realtimechannel_vcdiff_test.py (2 hunks)
  • test/ably/realtime/realtimeconnection_test.py (1 hunks)
  • test/ably/realtime/realtimeinit_test.py (1 hunks)
  • test/ably/realtime/realtimeresume_test.py (2 hunks)
  • test/ably/rest/encoders_test.py (5 hunks)
  • test/ably/rest/restauth_test.py (7 hunks)
  • test/ably/rest/restcapability_test.py (1 hunks)
  • test/ably/rest/restchannelhistory_test.py (1 hunks)
  • test/ably/rest/restchannelpublish_test.py (2 hunks)
  • test/ably/rest/restchannels_test.py (1 hunks)
  • test/ably/rest/restchannelstatus_test.py (2 hunks)
  • test/ably/rest/restcrypto_test.py (2 hunks)
  • test/ably/rest/restinit_test.py (1 hunks)
  • test/ably/rest/restpaginatedresult_test.py (3 hunks)
  • test/ably/rest/restpresence_test.py (2 hunks)
  • test/ably/rest/restpush_test.py (2 hunks)
  • test/ably/rest/restrequest_test.py (2 hunks)
  • test/ably/rest/reststats_test.py (2 hunks)
  • test/ably/rest/resttime_test.py (1 hunks)
  • test/ably/rest/resttoken_test.py (2 hunks)
  • test/ably/utils.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
  • test/ably/rest/restinit_test.py
  • test/ably/realtime/realtimechannel_vcdiff_test.py
  • test/ably/rest/restchannelpublish_test.py
  • test/ably/rest/restchannels_test.py
  • test/ably/rest/restchannelhistory_test.py
  • pyproject.toml
  • test/ably/rest/resttoken_test.py
  • test/ably/realtime/realtimeinit_test.py
  • test/ably/rest/restchannelstatus_test.py
  • test/ably/rest/resttime_test.py
  • .github/workflows/check.yml
🧰 Additional context used
🧬 Code graph analysis (12)
test/ably/realtime/realtimechannel_test.py (4)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/realtime/eventemitter_test.py (3)
test/ably/utils.py (1)
  • BaseAsyncTestCase (36-51)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
test/ably/rest/restpresence_test.py (3)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/conftest.py (1)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_test_vars (41-70)
  • clear_test_vars (105-115)
test/ably/rest/encoders_test.py (1)
ably/util/crypto.py (4)
  • CipherParams (16-42)
  • secret_key (29-30)
  • secret_key (110-111)
  • algorithm (25-26)
test/ably/rest/restpush_test.py (4)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/reststats_test.py (3)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restcapability_test.py (1)
  • setup (12-16)
ably/rest/rest.py (2)
  • http (108-109)
  • stats (79-85)
test/ably/realtime/realtimeresume_test.py (4)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/rest/restrequest_test.py (7)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • setup (454-459)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcrypto_test.py (1)
  • setup (22-28)
test/ably/rest/restinit_test.py (1)
  • setup (16-17)
test/ably/rest/restpaginatedresult_test.py (1)
  • setup (31-68)
test/ably/rest/restpaginatedresult_test.py (4)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannels_test.py (1)
  • setup (16-20)
test/ably/rest/restcapability_test.py (10)
test/ably/realtime/eventemitter_test.py (1)
  • setup (12-13)
test/ably/realtime/realtimechannel_test.py (1)
  • setup (16-18)
test/ably/realtime/realtimeconnection_test.py (1)
  • setup (15-17)
test/ably/realtime/realtimeinit_test.py (1)
  • setup (14-16)
test/ably/realtime/realtimeresume_test.py (1)
  • setup (28-30)
test/ably/rest/encoders_test.py (4)
  • setup (26-29)
  • setup (148-152)
  • setup (261-264)
  • setup (352-356)
test/ably/rest/restauth_test.py (5)
  • setup (30-31)
  • setup (172-176)
  • setup (327-328)
  • setup (486-532)
  • setup (589-637)
test/ably/rest/restchannelhistory_test.py (1)
  • setup (17-21)
test/ably/rest/restchannelpublish_test.py (2)
  • setup (30-37)
  • get_ably_rest (533-535)
test/ably/testapp.py (3)
  • TestApp (37-115)
  • get_test_vars (41-70)
  • get_ably_rest (73-77)
test/ably/rest/restcrypto_test.py (1)
ably/util/crypto.py (2)
  • CipherParams (16-42)
  • get_cipher (163-168)
🪛 Gitleaks (8.30.0)
test/ably/rest/encoders_test.py

[high] 150-150: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[high] 354-354: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: check (3.14)
  • GitHub Check: check (3.11)
  • GitHub Check: check (3.13)
  • GitHub Check: check (3.12)
  • GitHub Check: check (3.10)
  • GitHub Check: check (3.9)
  • GitHub Check: check (3.8)
🔇 Additional comments (29)
test/ably/rest/encoders_test.py (5)

8-8: LGTM! Import added for pytest fixtures.

The pytest import is correctly added to support the @pytest.fixture decorators used throughout the file.


25-29: LGTM! Correct pytest-asyncio fixture pattern.

The migration from asyncSetUp/asyncTearDown to a pytest fixture is correctly implemented. The autouse=True ensures this runs for each test method, and the yield-based teardown properly cleans up resources.


147-152: LGTM! Correct pytest-asyncio fixture pattern.

The migration is correctly implemented with proper setup and teardown.

Regarding the static analysis hint about line 150: The hardcoded key 'keyfordecrypt_16' is a test fixture used consistently across encryption tests, not a real secret. This is a false positive.


260-264: LGTM! Consistent fixture implementation.

The fixture correctly initializes the Ably client with binary protocol and ensures proper cleanup.


351-356: LGTM! Consistent fixture implementation.

The fixture correctly implements the pytest-asyncio pattern with proper setup and teardown for binary protocol encryption tests.

Regarding the static analysis hint about line 354: The hardcoded key 'keyfordecrypt_16' is a test fixture, not a real secret. This is a false positive.

conftest.py (1)

1-4: LGTM! pytest-asyncio plugin configured correctly.

The pytest_plugins configuration properly enables pytest-asyncio for the test suite.

test/ably/realtime/eventemitter_test.py (2)

3-3: LGTM! pytest import added correctly.


11-13: LGTM! Fixture correctly replaces asyncSetUp.

The autouse fixture properly replaces the asyncSetUp lifecycle method and will run automatically before each test. The initialization logic is preserved correctly.

test/ably/realtime/realtimeresume_test.py (2)

3-3: LGTM! pytest import added.


27-30: LGTM! Fixture correctly replaces asyncSetUp.

The migration from asyncSetUp to an autouse pytest fixture is correct and consistent with the broader pattern across realtime tests.

test/ably/realtime/realtimechannel_test.py (1)

15-18: LGTM! Fixture migration is correct.

The autouse pytest fixture properly replaces the asyncSetUp method and follows the established pattern across the test suite.

test/ably/realtime/realtimechannel_publish_test.py (1)

17-19: LGTM! Fixture correctly replaces asyncSetUp.

The autouse fixture properly migrates the setup logic from asyncSetUp to pytest-asyncio. Individual tests handle their own resource cleanup, which is appropriate for this test class.

test/ably/utils.py (2)

17-17: LGTM! unittest.TestCase inheritance removed correctly.

Removing the unittest base class is appropriate for the migration to pytest. The class can now be used as a plain utility/mixin class with pytest test classes.


36-36: LGTM! IsolatedAsyncioTestCase inheritance removed correctly.

The removal of unittest async test case inheritance aligns with the pytest-asyncio migration. The class methods remain functional as utilities for pytest-based async tests.

test/ably/realtime/realtimeconnection_test.py (1)

14-17: LGTM! Fixture migration is correct.

The autouse pytest fixture correctly replaces the asyncSetUp lifecycle method, consistent with the broader test suite migration.

test/ably/rest/restpush_test.py (1)

24-44: LGTM! Fixture correctly replaces asyncSetUp/asyncTearDown with yield pattern.

The fixture properly migrates both setup and teardown logic using the pytest yield pattern. Setup occurs before yield (creating devices and subscriptions), and teardown occurs after (cleaning up resources and closing the ably client). This is the correct approach when the fixture creates shared resources (like self.ably) used across multiple tests.

test/ably/rest/restpaginatedresult_test.py (1)

30-68: LGTM! Fixture migration follows best practices.

The async fixture correctly initializes resources (ably client, mocked API, paginated results) before yield, then performs cleanup (stops mocks, closes client) in the teardown phase after yield. This aligns with the pytest fixture pattern used throughout the PR.

test/ably/conftest.py (1)

6-10: LGTM! Proper migration to pytest-asyncio session fixture.

The fixture correctly manages test application lifecycle at session scope, initializing test vars before yield and cleaning up after. This replaces the manual event loop management with pytest-asyncio's native async fixture support.

test/ably/rest/restpresence_test.py (1)

191-198: LGTM! Proper fixture teardown pattern.

The TestPresenceCrypt fixture correctly releases the channel after yield during teardown, following proper pytest fixture cleanup patterns.

test/ably/rest/restcrypto_test.py (2)

21-28: LGTM! Standard fixture migration.

The setup fixture follows the standard pattern: initializes resources before yield, performs cleanup after yield.


204-217: The fixture scope is correctly set to function scope and should not be changed.

The current implementation runs the fixture before each test method, which is intentional. The test_encode method resets self.cipher_params and self.cipher during execution, indicating that per-test initialization is required. Using scope='class' would break this pattern by reusing the same cipher instance across tests, causing state pollution.

However, renaming setUpClass to setup or setup_class for consistency with pytest conventions would improve clarity, since the function-scope pytest fixture decorator contradicts the setUpClass class-level naming convention.

Likely an incorrect or invalid review comment.

test/ably/rest/restcapability_test.py (1)

11-16: LGTM! Standard fixture migration.

The fixture correctly initializes test resources before yield and closes the ably client during teardown after yield, following the established pattern.

test/ably/rest/reststats_test.py (1)

26-76: LGTM! Fixture properly ensures teardown always runs.

The fixture correctly inverts the conditional logic so that yield is always reached, ensuring the teardown code (lines 75-76) that closes both ably clients always executes. This addresses the resource leak that would have occurred with an early return before yield.

test/ably/rest/restrequest_test.py (1)

15-27: LGTM! Fixture migration includes test data setup.

The fixture properly initializes test resources and populates test data (20 events) before yield, then closes the ably client during teardown. The pattern is consistent with other test fixtures in the PR.

test/ably/rest/restauth_test.py (5)

29-31: LGTM! Simple setup fixture without teardown.

The fixture only initializes test_vars without acquiring resources that need cleanup, so no yield or teardown is necessary.


171-176: LGTM! Standard fixture with proper teardown.

The fixture correctly initializes ably client and test_vars before yield, then closes the ably client during teardown.


326-328: LGTM! Simple setup fixture without teardown.

The fixture only initializes test_vars without acquiring resources that need cleanup, so no yield or teardown is necessary.


485-532: LGTM! Complex fixture with proper mock and client teardown.

The fixture correctly:

  1. Initializes test state, ably client, and mocked API routes before yield
  2. Performs teardown after yield: stops/resets mocks with quiet=True and closes ably client
  3. Uses quiet=True to avoid spurious failures when not all mocked routes are called

588-637: LGTM! Complex fixture with proper mock teardown.

The fixture correctly initializes test state and mocked API routes before yield, then stops and resets mocks with quiet=True during teardown. The pattern is consistent with other complex fixtures in this file.

Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

looks good, couple questions:

Copy link
Member

@owenpearson owenpearson left a comment

Choose a reason for hiding this comment

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

lgtm 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants