chore: replace sleep-based polling with Event synchronization in tests#1467
chore: replace sleep-based polling with Event synchronization in tests#1467WilliamBergamin wants to merge 7 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1467 +/- ##
=======================================
Coverage 91.31% 91.31%
=======================================
Files 229 229
Lines 7266 7266
=======================================
Hits 6635 6635
Misses 631 631 ☔ View full report in Codecov by Sentry. |
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
@WilliamBergamin Amazing find in improvements to the test patterns! 💰
Please feel free to merge whenever's right, but I'm wondering if this is a change that we ought match elsewhere or add to maintainer's guide adjacent?
| from slack_bolt import App, BoltRequest, Assistant, Say, SetSuggestedPrompts, SetStatus, BoltContext | ||
| from slack_bolt import App, Assistant, BoltContext, BoltRequest, Say, SetStatus, SetSuggestedPrompts |
There was a problem hiding this comment.
🌟 praise: Thanks for these reorderings!
👾 ramble: The ruff linter supports patterns similar to isort that we might find useful I hope!
| @@ -54,37 +43,37 @@ def start_thread(say: Say, set_suggested_prompts: SetSuggestedPrompts, set_statu | |||
| set_suggested_prompts( | |||
| prompts=[{"title": "What does SLACK stand for?", "message": "What does SLACK stand for?"}], title="foo" | |||
| ) | |||
| called["value"] = True | |||
| listener_called.set() | |||
|
|
|||
| app.assistant(assistant) | |||
|
|
|||
| request = BoltRequest(body=thread_started_event_body, mode="socket_mode") | |||
| response = app.dispatch(request) | |||
| assert response.status == 200 | |||
| assert_target_called(called) | |||
| assert listener_called.wait(timeout=0.1) is True | |||
There was a problem hiding this comment.
🧪 praise: This builds nice confidence to me! TIL about event threading too!
| response = app.dispatch(request) | ||
| assert response.status == 404 | ||
| assert called["value"] is False | ||
| assert listener_called.wait(timeout=0.1) is False |
There was a problem hiding this comment.
🤓 praise: This is a nice pattern too!
2 participants
Summary
These changes aims to improve the unit tests around assistant behavior by using the python
Eventobject to synchronize threaded test behaviors rather then relying on home grown sleep based pollingThis should make tests faster and more reliable (note: the 0.1s timeout might be to short we can adjust if we see flakiness)
Testing
CI should be sufficient
Category
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.