test: characterization tests for payload_utils assistant predicates#1548
Merged
Conversation
Add unit tests for the existing assistant-thread payload predicates in slack_bolt/request/payload_utils.py (is_event, the user/bot/other message predicates, and is_assistant_event and thread predicates). These tests characterize current behavior and pass against main as-is. Also improve the shared scenario fixtures they rely on: - test_events_assistant: use a C-prefixed channel id for the channel (non-im) message fixtures to match their channel_type - test_events_ignore_self: add channel_type to the reaction item Extracted from the set_suggested_prompts refactor branch so this regression net can land on main first. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1548 +/- ##
==========================================
+ Coverage 91.37% 91.44% +0.06%
==========================================
Files 228 228
Lines 7285 7285
==========================================
+ Hits 6657 6662 +5
+ Misses 628 623 -5 ☔ View full report in Codecov by Harness. |
WilliamBergamin
added a commit
that referenced
this pull request
Jul 7, 2026
The characterization tests for the pre-existing assistant predicates, and the shared scenario fixture improvements, were moved to a separate branch (PR #1548) so they can land on main first as a regression net. This branch now keeps only the tests for behavior it introduces: the new is_message_event, is_any_im_message_event, and is_im_message_event predicates. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
zimeg
approved these changes
Jul 8, 2026
zimeg
left a comment
zimeg
left a comment
Member
There was a problem hiding this comment.
@WilliamBergamin LGTM! Thanks for keeping coverage confident with ongoing changes. I left one question about fixtures but think this is good to go when the time is right 🚢 💨
| "user_profile": {}, | ||
| "thread_ts": "1726133698.626339", | ||
| "parent_user_id": "W222", | ||
| "channel": "D111", |
Member
There was a problem hiding this comment.
🤖 question: Is keeping this prefixed with D meaningful to the assistant feature or is channel_user_message_event_body intended to be more generic?
Contributor
Author
There was a problem hiding this comment.
From the documentation the channel should start with a C, thats the only reason for the change 🙏
Comment on lines
+114
to
+139
| def test_is_event(self): | ||
| positives = { | ||
| "thread_started": thread_started_event_body, | ||
| "thread_context_changed": thread_context_changed_event_body, | ||
| "user_message_im": user_message_event_body, | ||
| "user_message_im_with_assistant_thread": user_message_event_body_with_assistant_thread, | ||
| "message_changed_im": message_changed_event_body, | ||
| "channel_user_message": channel_user_message_event_body, | ||
| "channel_message_changed": channel_message_changed_event_body, | ||
| "bot_message_channel": bot_message_event_payload, | ||
| "classic_bot_message_channel": classic_bot_message_event_payload, | ||
| "message_deleted_channel": message_deleted_channel_body, | ||
| "reaction_added": reaction_added_event_body, | ||
| "file_share_im": file_share_im_message_body, | ||
| "bot_im_thread": bot_im_thread_message_body, | ||
| "im_no_thread_ts": im_message_no_thread_ts_body, | ||
| } | ||
| negatives = { | ||
| "block_actions": block_actions_body, | ||
| "slash_command": slash_command_body, | ||
| "empty_dict": {}, | ||
| } | ||
| for key, body in positives.items(): | ||
| assert is_event(body), f"{key} should be recognized as an event" | ||
| for key, body in negatives.items(): | ||
| assert not is_event(body), f"{key} should NOT be recognized as an event" |
Member
There was a problem hiding this comment.
🌟 praise: Fun test to see I think!
mwbrooks
reviewed
Jul 8, 2026
mwbrooks
left a comment
mwbrooks
left a comment
Member
There was a problem hiding this comment.
✅ Everything passes locally and this looks great. Thanks for adding these test @WilliamBergamin 🙇🏻
| "thread_ts": "1726133698.626339", | ||
| "parent_user_id": "W222", | ||
| "channel": "D111", | ||
| "channel": "C111", |
Member
There was a problem hiding this comment.
praise: Thanks for updating this!
| "type": "message", | ||
| "channel": "C111", | ||
| "ts": "1599529504.000400", | ||
| "channel_type": "channel", |
| from tests.scenario_tests.test_events_ignore_self import event_body as reaction_added_event_body | ||
| from tests.scenario_tests.test_block_actions import body as block_actions_body | ||
|
|
||
| file_share_im_message_body = build_payload( |
Member
There was a problem hiding this comment.
praise: These are super valuable new fixtures to add.
| for key, body in negatives.items(): | ||
| assert not is_event(body), f"{key} should NOT be recognized as an event" | ||
|
|
||
| def test_is_user_message_event_in_assistant_thread(self): |
Member
There was a problem hiding this comment.
note: Nice to see a separate assistant thread test
WilliamBergamin
added a commit
that referenced
this pull request
Jul 8, 2026
Reunites this branch with PR #1548 (assistant predicate characterization tests + shared scenario fixtures), which was split out to land on main first. Resolved the add/add conflict in test_payload_utils.py by taking the union: this branch's new-behavior tests (is_message_event, is_any_im_message_event, is_im_message_event) plus the assistant-predicate characterization tests from #1548. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
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
3 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.
Summary
Adds unit tests characterizing the existing assistant-thread payload predicates in
slack_bolt/request/payload_utils.py, plus small fixture improvements the tests rely on.Testing
Category
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
./scripts/install_all_and_run_tests.shafter making the changes.