diff --git a/src/backend/core/services/livekit_events.py b/src/backend/core/services/livekit_events.py index 2ad1e4ac..155d075a 100644 --- a/src/backend/core/services/livekit_events.py +++ b/src/backend/core/services/livekit_events.py @@ -259,7 +259,6 @@ class LiveKitEventsService: def _handle_egress_ended(self, data): """Handle 'egress_ended' event.""" - # pylint: disable=too-many-branches # Fetch recording try: diff --git a/src/backend/core/tests/recording/service/test_recording_events.py b/src/backend/core/tests/recording/service/test_recording_events.py index fd52c35f..62ec7b17 100644 --- a/src/backend/core/tests/recording/service/test_recording_events.py +++ b/src/backend/core/tests/recording/service/test_recording_events.py @@ -45,34 +45,6 @@ def test_handle_limit_reached_success(mock_notify, mode, notification_type, serv ) -@pytest.mark.parametrize( - ("mode", "notification_type"), - ( - ("screen_recording", "screenRecordingLimitReached"), - ("transcript", "transcriptionLimitReached"), - ), -) -@mock.patch("core.utils.notify_participants") -def test_handle_limit_reached_error(mock_notify, mode, notification_type, service): - """Test handle_limit_reached raises RecordingEventsError when notification fails.""" - - mock_notify.side_effect = NotificationError("Error notifying") - - recording = RecordingFactory(status="active", mode=mode) - - with pytest.raises( - RecordingEventsError, - match=r"Failed to notify participants in room '.+' " - r"about recording limit reached \(recording_id=.+\)", - ): - service.handle_limit_reached(recording) - - assert recording.status == "stopped" - mock_notify.assert_called_once_with( - room_name=str(recording.room.id), notification_data={"type": notification_type} - ) - - @pytest.mark.parametrize( ("mode", "notification_type"), ( @@ -93,28 +65,6 @@ def test_handle_failed_success(mock_notify, mode, notification_type, service): ) -@mock.patch("core.utils.notify_participants") -def test_handle_failed_error(mock_notify, service): - """Test handle_failed raises RecordingEventsError when notification fails.""" - - mock_notify.side_effect = NotificationError("Error notifying") - - recording = RecordingFactory(status="active", mode="screen_recording") - - with pytest.raises( - RecordingEventsError, - match=r"Failed to notify participants in room '.+' " - r"about recording failed \(recording_id=.+\)", - ): - service.handle_failed(recording) - - assert recording.status == "failed" - mock_notify.assert_called_once_with( - room_name=str(recording.room.id), - notification_data={"type": "screenRecordingFailed"}, - ) - - @pytest.mark.parametrize( ("mode", "notification_type"), ( @@ -135,25 +85,48 @@ def test_handle_aborted_success(mock_notify, mode, notification_type, service): ) +@pytest.mark.parametrize( + ("mode", "notification_prefix"), + (("screen_recording", "screenRecording"), ("transcript", "transcription")), +) +@pytest.mark.parametrize( + ("handler", "expected_status", "event", "notification_suffix"), + ( + ("handle_limit_reached", "stopped", "limit reached", "LimitReached"), + ("handle_failed", "failed", "failed", "Failed"), + ("handle_aborted", "aborted", "aborted", "Aborted"), + ), +) @mock.patch("core.utils.notify_participants") -def test_handle_aborted_error(mock_notify, service): - """Test handle_aborted raises RecordingEventsError when notification fails.""" +def test_handle_event_notification_error( # noqa: PLR0913 + mock_notify, + handler, + expected_status, + event, + notification_suffix, + mode, + notification_prefix, + service, +): # pylint: disable=too-many-arguments,too-many-positional-arguments + """Test handlers raise RecordingEventsError when notifying participants fails, + while still applying the recording status of their event. + """ mock_notify.side_effect = NotificationError("Error notifying") - recording = RecordingFactory(status="active", mode="screen_recording") + recording = RecordingFactory(status="active", mode=mode) with pytest.raises( RecordingEventsError, - match=r"Failed to notify participants in room '.+' " - r"about recording aborted \(recording_id=.+\)", + match=rf"Failed to notify participants in room '.+' " + rf"about recording {event} \(recording_id=.+\)", ): - service.handle_aborted(recording) + getattr(service, handler)(recording) - assert recording.status == "aborted" + assert recording.status == expected_status mock_notify.assert_called_once_with( room_name=str(recording.room.id), - notification_data={"type": "screenRecordingAborted"}, + notification_data={"type": f"{notification_prefix}{notification_suffix}"}, ) diff --git a/src/backend/core/tests/services/test_livekit_events.py b/src/backend/core/tests/services/test_livekit_events.py index 66c553ab..e0dce8a2 100644 --- a/src/backend/core/tests/services/test_livekit_events.py +++ b/src/backend/core/tests/services/test_livekit_events.py @@ -435,8 +435,8 @@ def test_handle_egress_ended_unsuccessful_egress_notification_fails( egress_status, recording_status, service, -): # pylint: disable=too-many-arguments,too-many-positional-arguments - """Test that notification failure does not disrupt the update.""" +): + """Test that notification failure does not interrupt the update.""" mock_notify.side_effect = NotificationError("Error notifying") @@ -463,7 +463,7 @@ def test_handle_egress_ended_unsuccessful_egress_notification_fails( def test_handle_egress_ended_logs_livekit_error( # noqa: PLR0913 mock_update_metadata, mock_notify, egress_status, event, service, caplog ): # pylint: disable=too-many-arguments,too-many-positional-arguments - """Should log the reason LiveKit reported an uqnsuccessful egress.""" + """Should log the reason LiveKit reported an unsuccessful egress.""" recording = RecordingFactory(worker_id="worker-1", status="active") mock_data = mock.MagicMock() @@ -506,7 +506,8 @@ def test_handle_egress_ended_does_not_save_on_wrong_status( @pytest.mark.parametrize( - "status", ["failed_to_start", "aborted", "failed_to_stop", "saved", "initiated"] + "status", + ["failed_to_start", "aborted", "failed", "failed_to_stop", "saved", "initiated"], ) @mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_ignores_non_savable_recording(