Skip to content

MPT-19489 unskip some chat participants e2e tests#258

Merged
jentyk merged 1 commit intomainfrom
feat/MPT-19489
Mar 30, 2026
Merged

MPT-19489 unskip some chat participants e2e tests#258
jentyk merged 1 commit intomainfrom
feat/MPT-19489

Conversation

@jentyk
Copy link
Copy Markdown
Member

@jentyk jentyk commented Mar 30, 2026

Closes MPT-19489

  • Re-enabled three previously skipped chat participants e2e tests (removed pytest.mark.skip) in both async and sync suites:
    • test_list_chat_participants
    • test_update_chat_participant_not_found
    • test_delete_chat_participant_not_found
  • Updated expected HTTP status in test_delete_chat_participant_not_found from HTTPStatus.NOT_FOUND to HTTPStatus.BAD_REQUEST (TODO: verify 400 is correct)

@jentyk jentyk requested a review from a team as a code owner March 30, 2026 08:21
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 0204176b-85d2-4fa0-a518-77e8da3bbb47

📥 Commits

Reviewing files that changed from the base of the PR and between a885323 and f9bc77e.

📒 Files selected for processing (2)
  • tests/e2e/helpdesk/chats/participants/test_async_participants.py
  • tests/e2e/helpdesk/chats/participants/test_sync_participants.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/e2e/helpdesk/chats/participants/test_async_participants.py

📝 Walkthrough

Walkthrough

Removed skip markers from three chat-participant tests in both async and sync E2E test files so they now run; adjusted test_delete_chat_participant_not_found to expect HTTPStatus.BAD_REQUEST instead of HTTPStatus.NOT_FOUND and added a TODO to confirm the correct status.

Changes

Cohort / File(s) Summary
Chat Participant Tests
tests/e2e/helpdesk/chats/participants/test_async_participants.py, tests/e2e/helpdesk/chats/participants/test_sync_participants.py
Removed pytest.mark.skip from test_list_chat_participants, test_update_chat_participant_not_found, and test_delete_chat_participant_not_found. Updated test_delete_chat_participant_not_found expected status from HTTPStatus.NOT_FOUND to HTTPStatus.BAD_REQUEST and added a TODO to verify correctness.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed The PR title contains exactly one Jira issue key in the required MPT-XXXX format: MPT-19489.
Test Coverage Required ✅ Passed The pull request only modifies test files within the tests/ folder, specifically unskipping test cases and updating HTTP status codes.
Single Commit Required ✅ Passed The PR contains exactly one commit (f9bc77e: test(helpdesk): unskip some chat participants e2e tests), satisfying the single commit requirement.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

Copy link
Copy Markdown

@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 (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 whether BAD_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

📥 Commits

Reviewing files that changed from the base of the PR and between 9ef1497 and a885323.

📒 Files selected for processing (2)
  • tests/e2e/helpdesk/chats/participants/test_async_participants.py
  • tests/e2e/helpdesk/chats/participants/test_sync_participants.py

@sonarqubecloud
Copy link
Copy Markdown

@jentyk jentyk merged commit 1842c36 into main Mar 30, 2026
4 checks passed
@jentyk jentyk deleted the feat/MPT-19489 branch March 30, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants