feat(EventScheduler): list_events API #172
Conversation
WalkthroughAdds an EventScheduler message-bus handler Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant B as MessageBus
participant ES as EventScheduler
C->>B: emit "mycroft.scheduler.list_events"
B->>ES: deliver Message
ES->>B: reply {"scheduled_events": events}
B-->>C: deliver reply
rect rgb(245,245,255)
note right of ES: Shutdown sequence (listener cleanup)
ES->>B: remove listener "mycroft.scheduler.list_events"
ES->>B: remove other listeners (including "system.clock.synced")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)ovos_bus_client/util/scheduler.py (2)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #172 +/- ##
==========================================
- Coverage 42.27% 39.83% -2.44%
==========================================
Files 16 18 +2
Lines 2188 2337 +149
==========================================
+ Hits 925 931 +6
- Misses 1263 1406 +143 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ovos_bus_client/util/scheduler.py(4 hunks)test/unittests/test_event_scheduler.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ovos_bus_client/util/scheduler.py (2)
ovos_bus_client/client/client.py (2)
on(298-305)remove_all_listeners(345-354)ovos_bus_client/message.py (2)
Message(32-238)reply(166-199)
test/unittests/test_event_scheduler.py (2)
ovos_bus_client/util/scheduler.py (1)
list_events_handler(311-315)ovos_bus_client/message.py (1)
reply(166-199)
🪛 Ruff (0.13.3)
test/unittests/test_event_scheduler.py
24-24: Unused method argument: mock_thread
(ARG002)
39-39: Unused method argument: mock_json_dump
(ARG002)
39-39: Unused method argument: mock_thread
(ARG002)
67-67: Unused method argument: mock_thread
(ARG002)
90-90: Unused method argument: mock_dump
(ARG002)
90-90: Unused method argument: mock_thread
(ARG002)
111-111: Unused method argument: mock_dump
(ARG002)
111-111: Unused method argument: mock_thread
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: license_tests
🔇 Additional comments (5)
ovos_bus_client/util/scheduler.py (3)
72-72: LGTM!The formatting adjustment improves code readability.
98-98: LGTM!The new handler is correctly registered with the messagebus.
364-364: LGTM!The cleanup correctly removes the new listener during shutdown.
test/unittests/test_event_scheduler.py (2)
2-7: LGTM!String literal standardization improves code consistency.
20-106: LGTM!The standardization of string literals to double quotes improves consistency across the test suite.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ovos_bus_client/util/scheduler.py(4 hunks)test/unittests/test_event_scheduler.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/unittests/test_event_scheduler.py (2)
ovos_bus_client/util/scheduler.py (1)
list_events_handler(311-329)ovos_bus_client/message.py (1)
reply(166-199)
ovos_bus_client/util/scheduler.py (2)
ovos_bus_client/client/client.py (4)
on(298-305)emit(164-195)emit(388-412)remove_all_listeners(345-354)ovos_bus_client/message.py (2)
Message(32-238)response(201-214)
🪛 Ruff (0.14.1)
test/unittests/test_event_scheduler.py
24-24: Unused method argument: mock_thread
(ARG002)
39-39: Unused method argument: mock_json_dump
(ARG002)
39-39: Unused method argument: mock_thread
(ARG002)
67-67: Unused method argument: mock_thread
(ARG002)
90-90: Unused method argument: mock_dump
(ARG002)
90-90: Unused method argument: mock_thread
(ARG002)
111-111: Unused method argument: mock_dump
(ARG002)
111-111: Unused method argument: mock_thread
(ARG002)
ovos_bus_client/util/scheduler.py
317-318: Expected ')', found newline
(invalid-syntax)
| def test_list_events_handler(self, mock_open, mock_dump, mock_load, mock_thread): | ||
| """ | ||
| Test list_events_handler returns all scheduled events. | ||
| """ | ||
| mock_load.return_value = "" | ||
| mock_open.return_value = MagicMock() | ||
| emitter = MagicMock() | ||
| es = EventScheduler(emitter) | ||
|
|
||
| # Schedule a couple of events | ||
| es.schedule_event("test-event-1", 900000000000, None, {"data": "test1"}) | ||
| es.schedule_event("test-event-2", 910000000000, 60, {"data": "test2"}) | ||
|
|
||
| # Create a mock message | ||
| mock_message = MagicMock() | ||
| mock_message.context = {"source": "test"} | ||
|
|
||
| # Call the handler | ||
| es.list_events_handler(mock_message) | ||
|
|
||
| # Verify message.reply was called with correct msg_type and data | ||
| mock_message.reply.assert_called_once() | ||
| call_args = mock_message.reply.call_args | ||
| self.assertEqual(call_args[0][0], "mycroft.scheduler.list_events.response") | ||
| self.assertIn("scheduled_events", call_args[1]["data"]) | ||
|
|
||
| # Verify emitter.emit was called with the reply message | ||
| emitter.emit.assert_called() | ||
|
|
||
| # Verify the scheduled events contain our test events | ||
| scheduled_events = call_args[1]["data"]["scheduled_events"] | ||
| self.assertIn("test-event-1", scheduled_events) | ||
| self.assertIn("test-event-2", scheduled_events) | ||
|
|
||
| es.shutdown() |
There was a problem hiding this comment.
Test verifies wrong method call.
The production code calls message.response() (line 317 in scheduler.py), but the test verifies mock_message.reply() was called (line 132). These are different methods on the MagicMock object.
When using MagicMock(), calling message.response() in production invokes the mock's response attribute, not reply. The test should verify response was called.
Apply this diff to fix the test:
# Call the handler
es.list_events_handler(mock_message)
- # Verify message.reply was called with correct msg_type and data
- mock_message.reply.assert_called_once()
- call_args = mock_message.reply.call_args
- self.assertEqual(call_args[0][0], "mycroft.scheduler.list_events.response")
- self.assertIn("scheduled_events", call_args[1]["data"])
+ # Verify message.response was called with correct data
+ mock_message.response.assert_called_once()
+ call_args = mock_message.response.call_args
+ self.assertIn("scheduled_events", call_args[1]["data"])
# Verify emitter.emit was called with the reply message
emitter.emit.assert_called()
# Verify the scheduled events contain our test events
- scheduled_events = call_args[1]["data"]["scheduled_events"]
+ call_args = mock_message.response.call_args
+ scheduled_events = call_args[1]["data"]["scheduled_events"]
self.assertIn("test-event-1", scheduled_events)
self.assertIn("test-event-2", scheduled_events)🧰 Tools
🪛 Ruff (0.14.1)
111-111: Unused method argument: mock_dump
(ARG002)
111-111: Unused method argument: mock_thread
(ARG002)
🤖 Prompt for AI Agents
In test/unittests/test_event_scheduler.py around lines 111 to 145, the test
asserts on mock_message.reply but production code calls message.response();
update the test to assert and inspect mock_message.response instead of
mock_message.reply (replace mock_message.reply.assert_called_once(),
mock_message.reply.call_args usage, and subsequent call_args checks to use
mock_message.response) so the test verifies the actual method invoked by the
scheduler.
mikejgray
force-pushed
the
FEAT_EventSchedulerList
branch
from
769f78c to
5a495bd
Compare
3 participants
Closes #80
Summary by CodeRabbit
New Features
Bug Fixes
Tests