MPT-19484 unskip tests and refactor fixtures for async and sync cases#267
Merged
MPT-19484 unskip tests and refactor fixtures for async and sync cases#267
Conversation
📝 WalkthroughWalkthroughAdds synchronous and asynchronous pytest fixtures that chain helpdesk case actions (create → query → process), and refactors/enables sync and async tests to consume these fixtures and assert status transitions instead of performing inline API calls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
🧹 Nitpick comments (2)
tests/e2e/helpdesk/cases/test_async_cases.py (1)
42-51: Remove unusedasync_mpt_opsparameter from tests that only validate fixture state.Same as the sync counterparts—
test_process_caseandtest_query_casedon't useasync_mpt_ops. Also, correctly declaring these asdef(notasync def) since they contain noawaitaligns with the project's testing conventions. Based on learnings, pytest-asyncio resolves async fixtures automatically for sync test functions.♻️ Proposed fix
-def test_process_case(async_mpt_ops, async_processed_case): +def test_process_case(async_processed_case): result = async_processed_case.to_dict().get("status") assert result == "Processing" -def test_query_case(async_mpt_ops, async_queried_case): +def test_query_case(async_queried_case): result = async_queried_case.to_dict().get("status") assert result == "Querying"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/cases/test_async_cases.py` around lines 42 - 51, The tests test_process_case and test_query_case declare an unused parameter async_mpt_ops and are already synchronous functions; remove the unused async_mpt_ops parameter from both function signatures and keep them as regular def tests so pytest can resolve the async fixtures (async_processed_case and async_queried_case) automatically; ensure you only reference the needed fixtures async_processed_case and async_queried_case in the signatures for test_process_case and test_query_case respectively.tests/e2e/helpdesk/cases/test_sync_cases.py (1)
42-51: Remove unusedmpt_opsparameter from tests that only validate fixture state.
test_process_caseandtest_query_casereceivempt_opsbut never use it—the tests only assert on the fixture-provided state. Removing unused parameters keeps tests cleaner.♻️ Proposed fix
-def test_process_case(mpt_ops, processed_case): +def test_process_case(processed_case): result = processed_case.to_dict().get("status") assert result == "Processing" -def test_query_case(mpt_ops, queried_case): +def test_query_case(queried_case): result = queried_case.to_dict().get("status") assert result == "Querying"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/cases/test_sync_cases.py` around lines 42 - 51, Remove the unused mpt_ops fixture parameter from the two test functions to avoid unused-argument noise: update the function signatures for test_process_case and test_query_case to accept only the fixtures they use (processed_case and queried_case respectively) and leave the assertions unchanged; ensure there are no other references to mpt_ops inside those functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/helpdesk/cases/conftest.py`:
- Line 30: Fix the typo in the query prompt string used by the fixtures: update
the call to mpt_ops.helpdesk.cases.query (both sync and async fixtures) so the
{"queryPrompt": "More detaisl needed"} argument is corrected to {"queryPrompt":
"More details needed"}; search for mpt_ops.helpdesk.cases.query in this file and
replace the misspelled "detaisl" with "details" in each occurrence.
---
Nitpick comments:
In `@tests/e2e/helpdesk/cases/test_async_cases.py`:
- Around line 42-51: The tests test_process_case and test_query_case declare an
unused parameter async_mpt_ops and are already synchronous functions; remove the
unused async_mpt_ops parameter from both function signatures and keep them as
regular def tests so pytest can resolve the async fixtures (async_processed_case
and async_queried_case) automatically; ensure you only reference the needed
fixtures async_processed_case and async_queried_case in the signatures for
test_process_case and test_query_case respectively.
In `@tests/e2e/helpdesk/cases/test_sync_cases.py`:
- Around line 42-51: Remove the unused mpt_ops fixture parameter from the two
test functions to avoid unused-argument noise: update the function signatures
for test_process_case and test_query_case to accept only the fixtures they use
(processed_case and queried_case respectively) and leave the assertions
unchanged; ensure there are no other references to mpt_ops inside those
functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 12a9505a-371d-4c56-bc77-b1313038b72a
📒 Files selected for processing (3)
tests/e2e/helpdesk/cases/conftest.pytests/e2e/helpdesk/cases/test_async_cases.pytests/e2e/helpdesk/cases/test_sync_cases.py
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/helpdesk/cases/conftest.py (1)
29-30: Consider extracting the repeated query prompt into a module-level constant.
"More details needed"is duplicated in both sync and async fixtures; a shared constant would reduce drift risk.♻️ Optional refactor
+QUERY_PROMPT = {"queryPrompt": "More details needed"} + `@pytest.fixture` def queried_case(mpt_ops, created_case): - return mpt_ops.helpdesk.cases.query(created_case.id, {"queryPrompt": "More details needed"}) + return mpt_ops.helpdesk.cases.query(created_case.id, QUERY_PROMPT) @@ `@pytest.fixture` async def async_queried_case(async_mpt_ops, async_created_case): - return await async_mpt_ops.helpdesk.cases.query( - async_created_case.id, {"queryPrompt": "More details needed"} - ) + return await async_mpt_ops.helpdesk.cases.query(async_created_case.id, QUERY_PROMPT)Also applies to: 45-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/cases/conftest.py` around lines 29 - 30, Extract the repeated query prompt string ("More details needed") into a module-level constant (e.g., QUERY_PROMPT_MORE_DETAILS) and use that constant in the queried_case fixture (function queried_case) and the corresponding async fixture at lines 45-47 so both fixtures reference the same value; update imports/uses in this file accordingly to replace the literal with the new constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/e2e/helpdesk/cases/conftest.py`:
- Around line 29-30: Extract the repeated query prompt string ("More details
needed") into a module-level constant (e.g., QUERY_PROMPT_MORE_DETAILS) and use
that constant in the queried_case fixture (function queried_case) and the
corresponding async fixture at lines 45-47 so both fixtures reference the same
value; update imports/uses in this file accordingly to replace the literal with
the new constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4266b43e-86da-466b-80c0-5fe5ce78166d
📒 Files selected for processing (3)
tests/e2e/helpdesk/cases/conftest.pytests/e2e/helpdesk/cases/test_async_cases.pytests/e2e/helpdesk/cases/test_sync_cases.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/helpdesk/cases/test_sync_cases.py
Member
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
albertsola
approved these changes
Apr 1, 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
2 participants
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.
Closes MPT-19484