MPT-19489 unskip some chat participants e2e tests#258
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved skip markers from three chat-participant tests in both async and sync E2E test files so they now run; adjusted Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/helpdesk/chats/participants/test_async_participants.py (1)
55-60: Consistent with sync test, but same TODO concern applies.This mirrors the change in
test_sync_participants.py, which is good for consistency. However, the same verification concern applies here—the TODO should be resolved to confirm whetherBAD_REQUEST(400) is the expected API behavior for deleting a non-existent participant.Once the status code is verified in the sync test, ensure this async test reflects the same confirmed expectation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/chats/participants/test_async_participants.py` around lines 55 - 60, Resolve the TODO in test_delete_chat_participant_not_found by confirming the API's expected status for deleting a non-existent participant (compare with the sync test test_delete_chat_participant_not_found in test_sync_participants.py), then update the assertion in this async test (test_delete_chat_participant_not_found) to assert the confirmed status on the raised MPTAPIError (via error.value.status_code) instead of leaving the TODO; ensure async_chat_participants_service.delete(invalid_chat_participant_id) behavior is consistent with the sync test and mirror whatever status (HTTPStatus.BAD_REQUEST or HTTPStatus.NOT_FOUND) is verified as correct.
🤖 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/chats/participants/test_sync_participants.py`:
- Line 48: The test asserts a 400 on participant deletion not-found which is
inconsistent with other tests; change the assertion in test_sync_participants.py
(the failing assertion that checks error.value.status_code) to expect
HTTPStatus.NOT_FOUND instead of HTTPStatus.BAD_REQUEST, make the identical
change in the matching test in test_async_participants.py, and remove the TODO
comment; alternatively, if 400 is intentional, add a short comment explaining
why the participants endpoint returns 400 and remove the TODO so the behavior is
documented.
---
Nitpick comments:
In `@tests/e2e/helpdesk/chats/participants/test_async_participants.py`:
- Around line 55-60: Resolve the TODO in test_delete_chat_participant_not_found
by confirming the API's expected status for deleting a non-existent participant
(compare with the sync test test_delete_chat_participant_not_found in
test_sync_participants.py), then update the assertion in this async test
(test_delete_chat_participant_not_found) to assert the confirmed status on the
raised MPTAPIError (via error.value.status_code) instead of leaving the TODO;
ensure async_chat_participants_service.delete(invalid_chat_participant_id)
behavior is consistent with the sync test and mirror whatever status
(HTTPStatus.BAD_REQUEST or HTTPStatus.NOT_FOUND) is verified as correct.
🪄 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: 6c1e734b-3b5c-4a77-b4cb-fd62f7f2299f
📒 Files selected for processing (2)
tests/e2e/helpdesk/chats/participants/test_async_participants.pytests/e2e/helpdesk/chats/participants/test_sync_participants.py
|



Closes MPT-19489