MPT-16437 Reorganise resource accounts mixins structure #201
Conversation
📝 WalkthroughWalkthroughMixins for account resources were split into separate submodules, a package-level init was added to re-export them, resource modules' imports were updated to the new paths, the previous monolithic mixins file was removed, and tests were reorganized from one large module into focused per-mixin test files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/unit/resources/accounts/mixins/test_activatable_mixin.py`:
- Around line 132-139: The async test function test_async_actions_no_data is
missing the pytest asyncio marker; add the `@pytest.mark.asyncio` decorator
directly above the async def test_async_actions_no_data(...) to ensure the
coroutine is executed by pytest-asyncio (and confirm pytest-asyncio is available
in test deps if not already).
🧹 Nitpick comments (2)
mpt_api_client/resources/accounts/mixins/__init__.py (1)
18-18: Remove unusednoqadirective.The
# noqa: WPS410comment is not recognized by Ruff. If you're using wemake-python-styleguide alongside Ruff, this is fine. Otherwise, remove the unused directive.🧹 Proposed fix
-__all__ = [ # noqa: WPS410 +__all__ = [tests/unit/resources/accounts/mixins/test_validate_mixin.py (1)
41-46: Consider simplifying tests without parameterization.The
@pytest.mark.parametrizewith a single test case adds overhead without providing multiple test scenarios. Sincevalidateis the only action being tested, consider removing the parameterization and passing values directly.However, if there's intent to add more actions later (e.g.,
validate_partial), keeping the parameterization structure makes sense for consistency with other test files.♻️ Optional simplification (if no future actions planned)
-@pytest.mark.parametrize( - ("action", "input_status"), - [ - ("validate", {"id": "OBJ-0000-0001", "status": "update"}), - ], -) -def test_validate_resource_actions(validate_service, action, input_status): +def test_validate_resource_actions(validate_service): + action = "validate" + input_status = {"id": "OBJ-0000-0001", "status": "update"} request_expected_content = b'{"id":"OBJ-0000-0001","status":"update"}'
albertsola
force-pushed
the
MPT-16437/Reorganise-resource-mixins
branch
from
1950f49 to
35ebf36
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/unit/resources/accounts/mixins/test_file_mixins.py`:
- Around line 120-235: Remove the redundant `@pytest.mark.asyncio` decorators from
the async test functions (e.g., test_async_create_file_service,
test_async_create_file_service_no_file, test_async_update_file_service,
test_async_update_file_service_no_file) so they rely on pytest's configured
asyncio auto mode; delete the decorator lines (and any direct uses of
pytest.mark.asyncio in this file) while leaving the async def signatures and
test bodies unchanged.
In `@tests/unit/resources/accounts/mixins/test_invitable_mixin.py`:
- Around line 113-147: Remove the redundant `@pytest.mark.asyncio` decorators from
the async test functions in this file: specifically delete the decorator above
test_async_invitable_resource_actions and above
test_async_invitable_resource_actions_no_data (and any other async test
functions in this diff) so they rely on the repo-wide pytest asyncio auto mode;
leave the async def signatures, awaits (e.g., await
getattr(async_invitable_service, action)(...)), and parametrization intact.
🧹 Nitpick comments (1)
tests/unit/resources/accounts/mixins/test_activatable_mixin.py (1)
108-109: Minor inconsistency in@pytest.mark.asynciousage.
test_async_activatable_resource_actions(line 108) has the@pytest.mark.asynciodecorator, whiletest_async_actions_no_data(line 139) does not. Since the project usesasyncio_mode = "auto", neither requires the marker. Consider removing it from line 108 for consistency, or adding it to both if you prefer explicit markers.Based on learnings: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio.
Also applies to: 139-139
albertsola
force-pushed
the
MPT-16437/Reorganise-resource-mixins
branch
from
35ebf36 to
980ba89
Compare
|
Closes MPT-16437