From 88c38ddeb37f80b5e3bad0d2d62ef4e290112fe1 Mon Sep 17 00:00:00 2001 From: wdeveloper16 Date: Mon, 13 Apr 2026 02:55:58 +0200 Subject: [PATCH] test: migrate conversation rename/delete edge cases to Testcontainers integration tests (#34991) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../services/test_conversation_service.py | 63 +++++++++++++++++++ .../services/test_conversation_service.py | 53 ---------------- 2 files changed, 63 insertions(+), 53 deletions(-) diff --git a/api/tests/test_containers_integration_tests/services/test_conversation_service.py b/api/tests/test_containers_integration_tests/services/test_conversation_service.py index 6180d98b1e..98c38f2b5f 100644 --- a/api/tests/test_containers_integration_tests/services/test_conversation_service.py +++ b/api/tests/test_containers_integration_tests/services/test_conversation_service.py @@ -637,6 +637,40 @@ class TestConversationServiceSummarization: assert conversation.name == new_name assert conversation.updated_at == mock_time + @patch("services.conversation_service.LLMGenerator.generate_conversation_name") + def test_rename_with_auto_generate(self, mock_llm_generator, db_session_with_containers): + """ + Test rename delegates to auto_generate_name when auto_generate is True. + + When auto_generate is True, the service should call auto_generate_name + which uses an LLM to create a descriptive conversation title. + """ + # Arrange + app_model, user = ConversationServiceIntegrationTestDataFactory.create_app_and_account( + db_session_with_containers + ) + conversation = ConversationServiceIntegrationTestDataFactory.create_conversation( + db_session_with_containers, app_model, user + ) + ConversationServiceIntegrationTestDataFactory.create_message( + db_session_with_containers, app_model, conversation, user + ) + generated_name = "Auto Generated Name" + mock_llm_generator.return_value = generated_name + + # Act + result = ConversationService.rename( + app_model=app_model, + conversation_id=conversation.id, + user=user, + name=None, + auto_generate=True, + ) + + # Assert + assert result == conversation + assert conversation.name == generated_name + class TestConversationServiceMessageAnnotation: """ @@ -1066,3 +1100,32 @@ class TestConversationServiceExport: not_deleted = db_session_with_containers.scalar(select(Conversation).where(Conversation.id == conversation.id)) assert not_deleted is not None mock_delete_task.delay.assert_not_called() + + @patch("services.conversation_service.delete_conversation_related_data") + def test_delete_handles_exception_and_rollback(self, mock_delete_task, db_session_with_containers): + """ + Test that delete propagates exceptions and does not trigger the cleanup task. + + When a DB error occurs during deletion, the service must rollback the + transaction and re-raise the exception without scheduling async cleanup. + """ + # Arrange + app_model, user = ConversationServiceIntegrationTestDataFactory.create_app_and_account( + db_session_with_containers + ) + conversation = ConversationServiceIntegrationTestDataFactory.create_conversation( + db_session_with_containers, app_model, user + ) + conversation_id = conversation.id + + # Act — force an error during the delete to exercise the rollback path + with patch("services.conversation_service.db.session.delete", side_effect=Exception("DB error")): + with pytest.raises(Exception, match="DB error"): + ConversationService.delete(app_model=app_model, conversation_id=conversation_id, user=user) + + # Assert — async cleanup must NOT have been scheduled + mock_delete_task.delay.assert_not_called() + + # Conversation is still present because the deletion was never committed + still_there = db_session_with_containers.scalar(select(Conversation).where(Conversation.id == conversation_id)) + assert still_there is not None diff --git a/api/tests/unit_tests/services/test_conversation_service.py b/api/tests/unit_tests/services/test_conversation_service.py index a4359f00b8..68f4c51afe 100644 --- a/api/tests/unit_tests/services/test_conversation_service.py +++ b/api/tests/unit_tests/services/test_conversation_service.py @@ -435,36 +435,6 @@ class TestConversationServiceRename: assert conversation.name == "New Name" mock_db_session.commit.assert_called_once() - @patch("services.conversation_service.db.session") - @patch("services.conversation_service.ConversationService.get_conversation") - @patch("services.conversation_service.ConversationService.auto_generate_name") - def test_rename_with_auto_generate(self, mock_auto_generate, mock_get_conversation, mock_db_session): - """ - Test renaming conversation with auto-generation. - - Should call auto_generate_name when auto_generate is True. - """ - # Arrange - app_model = ConversationServiceTestDataFactory.create_app_mock() - user = ConversationServiceTestDataFactory.create_account_mock() - conversation = ConversationServiceTestDataFactory.create_conversation_mock() - - mock_get_conversation.return_value = conversation - mock_auto_generate.return_value = conversation - - # Act - result = ConversationService.rename( - app_model=app_model, - conversation_id="conv-123", - user=user, - name=None, - auto_generate=True, - ) - - # Assert - assert result == conversation - mock_auto_generate.assert_called_once_with(app_model, conversation) - class TestConversationServiceAutoGenerateName: """Test conversation auto-name generation operations.""" @@ -576,29 +546,6 @@ class TestConversationServiceDelete: mock_db_session.commit.assert_called_once() mock_delete_task.delay.assert_called_once_with(conversation.id) - @patch("services.conversation_service.db.session") - @patch("services.conversation_service.ConversationService.get_conversation") - def test_delete_handles_exception_and_rollback(self, mock_get_conversation, mock_db_session): - """ - Test deletion handles exceptions and rolls back transaction. - - Should rollback database changes when deletion fails. - """ - # Arrange - app_model = ConversationServiceTestDataFactory.create_app_mock() - user = ConversationServiceTestDataFactory.create_account_mock() - conversation = ConversationServiceTestDataFactory.create_conversation_mock() - - mock_get_conversation.return_value = conversation - mock_db_session.delete.side_effect = Exception("Database Error") - - # Act & Assert - with pytest.raises(Exception, match="Database Error"): - ConversationService.delete(app_model, "conv-123", user) - - # Assert rollback was called - mock_db_session.rollback.assert_called_once() - class TestConversationServiceConversationalVariable: """Test conversational variable operations."""