From f1f81e8d20ff646b3f83bb6c582bfa600d7fd7a2 Mon Sep 17 00:00:00 2001 From: Akhilesh Negi Date: Fri, 8 Aug 2025 22:25:19 +0530 Subject: [PATCH 1/9] fixes to testcases --- .../app/tests/api/routes/test_responses.py | 313 +++++++++++++++--- 1 file changed, 262 insertions(+), 51 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 483119d58..d42034f89 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -3,9 +3,8 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from sqlmodel import select -import openai -from app.api.routes.responses import router +from app.api.routes.responses import router, process_response from app.models import Project # Wrap the router in a FastAPI app instance @@ -14,6 +13,7 @@ client = TestClient(app) +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -29,11 +29,15 @@ def test_responses_endpoint_success( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header: dict[str, str], ): """Test the /responses endpoint for successful response creation.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials - configure to return different values based on provider def mock_get_credentials_by_provider(*args, **kwargs): provider = kwargs.get("provider") @@ -108,7 +112,16 @@ def return_mock_assistant(*args, **kwargs): assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_dalgo" + assert call_args[0][0].question == "What is Dalgo?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id is None + +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -124,10 +137,14 @@ def test_responses_endpoint_without_vector_store( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header, ): """Test the /responses endpoint when assistant has no vector store configured.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials mock_get_credential.return_value = {"api_key": "test_api_key"} @@ -184,14 +201,13 @@ def test_responses_endpoint_without_vector_store( assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" - # Verify OpenAI client was called without tools - mock_client.responses.create.assert_called_once_with( - model=mock_assistant.model, - previous_response_id=None, - instructions=mock_assistant.instructions, - temperature=mock_assistant.temperature, - input=[{"role": "user", "content": "What is Glific?"}], - ) + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_123" + assert call_args[0][0].question == "What is Glific?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id is None @patch("app.api.routes.responses.get_assistant_by_id") @@ -282,6 +298,7 @@ def test_responses_endpoint_missing_api_key_in_credentials( assert "OpenAI API key not configured" in response_json["error"] +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -297,10 +314,14 @@ def test_responses_endpoint_with_file_search_results( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header, ): """Test the /responses endpoint with file search results in the response.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials mock_get_credential.return_value = {"api_key": "test_api_key"} @@ -330,7 +351,8 @@ def test_responses_endpoint_with_file_search_results( mock_file_search_call.type = "file_search_call" mock_file_search_call.results = [mock_hit1, mock_hit2] - # Setup the mock response object with file search results and proper response ID format + # Setup the mock response object with file search results and proper response ID + # format mock_response = MagicMock() mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" mock_response.output_text = "Test output with search results" @@ -371,16 +393,16 @@ def test_responses_endpoint_with_file_search_results( assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" - # Verify OpenAI client was called with tools - mock_client.responses.create.assert_called_once() - call_args = mock_client.responses.create.call_args[1] - assert "tools" in call_args - assert call_args["tools"][0]["type"] == "file_search" - assert call_args["tools"][0]["vector_store_ids"] == ["vs_test"] - assert "include" in call_args - assert "file_search_call.results" in call_args["include"] + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_dalgo" + assert call_args[0][0].question == "What is Dalgo?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id is None +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -396,10 +418,14 @@ def test_responses_endpoint_with_ancestor_conversation_found( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header: dict[str, str], ): """Test the /responses endpoint when a conversation is found by ancestor ID.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials mock_get_credential.return_value = {"api_key": "test_api_key"} @@ -464,21 +490,16 @@ def test_responses_endpoint_with_ancestor_conversation_found( assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" - # Verify get_conversation_by_ancestor_id was called with correct parameters - mock_get_conversation_by_ancestor_id.assert_called_once() - call_args = mock_get_conversation_by_ancestor_id.call_args - assert ( - call_args[1]["ancestor_response_id"] - == "resp_ancestor1234567890abcdef1234567890" - ) - assert call_args[1]["project_id"] == dalgo_project.id - - # Verify OpenAI client was called with the conversation's response_id as previous_response_id - mock_client.responses.create.assert_called_once() - call_args = mock_client.responses.create.call_args[1] - assert call_args["previous_response_id"] == "resp_latest1234567890abcdef1234567890" + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_dalgo" + assert call_args[0][0].question == "What is Dalgo?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id == "resp_ancestor1234567890abcdef1234567890" +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -494,10 +515,14 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header: dict[str, str], ): """Test the /responses endpoint when no conversation is found by ancestor ID.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials mock_get_credential.return_value = {"api_key": "test_api_key"} @@ -559,23 +584,16 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" - # Verify get_conversation_by_ancestor_id was called with correct parameters - mock_get_conversation_by_ancestor_id.assert_called_once() - call_args = mock_get_conversation_by_ancestor_id.call_args - assert ( - call_args[1]["ancestor_response_id"] - == "resp_ancestor1234567890abcdef1234567890" - ) - assert call_args[1]["project_id"] == dalgo_project.id - - # Verify OpenAI client was called with the original response_id as previous_response_id - mock_client.responses.create.assert_called_once() - call_args = mock_client.responses.create.call_args[1] - assert ( - call_args["previous_response_id"] == "resp_ancestor1234567890abcdef1234567890" - ) + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_dalgo" + assert call_args[0][0].question == "What is Dalgo?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id == "resp_ancestor1234567890abcdef1234567890" +@patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") @@ -591,10 +609,14 @@ def test_responses_endpoint_without_response_id( mock_get_assistant, mock_get_credential, mock_openai, + mock_process_response, db, user_api_key_header: dict[str, str], ): """Test the /responses endpoint when no response_id is provided.""" + # Mock the background task to prevent actual execution + mock_process_response.return_value = None + # Setup mock credentials mock_get_credential.return_value = {"api_key": "test_api_key"} @@ -653,10 +675,199 @@ def test_responses_endpoint_without_response_id( assert response_json["data"]["status"] == "processing" assert response_json["data"]["message"] == "Response creation started" - # Verify get_conversation_by_ancestor_id was not called since response_id is None - mock_get_conversation_by_ancestor_id.assert_not_called() + # Verify that the background task was scheduled with correct parameters + mock_process_response.assert_called_once() + call_args = mock_process_response.call_args + assert call_args[0][0].assistant_id == "assistant_dalgo" + assert call_args[0][0].question == "What is Dalgo?" + assert call_args[0][0].callback_url == "http://example.com/callback" + assert call_args[0][0].response_id is None - # Verify OpenAI client was called with None as previous_response_id + +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.send_callback") +def test_process_response_ancestor_conversation_found( + mock_send_callback, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + db, +): + """Test process_response function when ancestor conversation is found.""" + from app.api.routes.responses import ResponsesAPIRequest + + # Setup mock request + request = ResponsesAPIRequest( + assistant_id="assistant_dalgo", + question="What is Dalgo?", + callback_url="http://example.com/callback", + response_id="resp_ancestor1234567890abcdef1234567890", + ) + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_response = MagicMock() + mock_response.id = "resp_new1234567890abcdef1234567890abcdef" + mock_response.output_text = "Test response" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [] + mock_response.previous_response_id = "resp_latest1234567890abcdef1234567890" + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + + # Setup mock conversation found by ancestor ID + mock_conversation = MagicMock() + mock_conversation.response_id = "resp_latest1234567890abcdef1234567890" + mock_conversation.ancestor_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_get_conversation_by_ancestor_id.return_value = mock_conversation + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + # Call process_response + process_response( + request=request, + client=mock_client, + assistant=mock_assistant, + tracer=mock_tracer, + project_id=dalgo_project.id, + organization_id=1, + session=db, + ) + + # Verify get_conversation_by_ancestor_id was called with correct parameters + mock_get_conversation_by_ancestor_id.assert_called_once_with( + session=db, + ancestor_response_id="resp_ancestor1234567890abcdef1234567890", + project_id=dalgo_project.id, + ) + + # Verify OpenAI client was called with the conversation's response_id as + # previous_response_id + mock_client.responses.create.assert_called_once() + call_args = mock_client.responses.create.call_args[1] + assert call_args["previous_response_id"] == ( + "resp_latest1234567890abcdef1234567890" + ) + + # Verify create_conversation was called + mock_create_conversation.assert_called_once() + + # Verify send_callback was called + mock_send_callback.assert_called_once() + + +@patch("app.api.routes.responses.get_conversation_by_ancestor_id") +@patch("app.api.routes.responses.create_conversation") +@patch("app.api.routes.responses.get_ancestor_id_from_response") +@patch("app.api.routes.responses.send_callback") +def test_process_response_ancestor_conversation_not_found( + mock_send_callback, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + db, +): + """Test process_response function when no ancestor conversation is found.""" + from app.api.routes.responses import ResponsesAPIRequest + + # Setup mock request + request = ResponsesAPIRequest( + assistant_id="assistant_dalgo", + question="What is Dalgo?", + callback_url="http://example.com/callback", + response_id="resp_ancestor1234567890abcdef1234567890", + ) + + # Setup mock assistant + mock_assistant = MagicMock() + mock_assistant.model = "gpt-4o" + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = ["vs_test"] + mock_assistant.max_num_results = 20 + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_response = MagicMock() + mock_response.id = "resp_new1234567890abcdef1234567890abcdef" + mock_response.output_text = "Test response" + mock_response.model = "gpt-4o" + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = [] + mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + + # Setup mock conversation not found by ancestor ID + mock_get_conversation_by_ancestor_id.return_value = None + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Get the Dalgo project ID + dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() + if not dalgo_project: + pytest.skip("Dalgo project not found in the database") + + # Call process_response + process_response( + request=request, + client=mock_client, + assistant=mock_assistant, + tracer=mock_tracer, + project_id=dalgo_project.id, + organization_id=1, + session=db, + ) + + # Verify get_conversation_by_ancestor_id was called with correct parameters + mock_get_conversation_by_ancestor_id.assert_called_once_with( + session=db, + ancestor_response_id="resp_ancestor1234567890abcdef1234567890", + project_id=dalgo_project.id, + ) + + # Verify OpenAI client was called with the original response_id as + # previous_response_id mock_client.responses.create.assert_called_once() call_args = mock_client.responses.create.call_args[1] - assert call_args["previous_response_id"] is None + assert call_args["previous_response_id"] == ( + "resp_ancestor1234567890abcdef1234567890" + ) + + # Verify create_conversation was called + mock_create_conversation.assert_called_once() + + # Verify send_callback was called + mock_send_callback.assert_called_once() From 09189a07481d9027920bd594d0bcb53cd4e6458c Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 13 Aug 2025 12:32:16 +0530 Subject: [PATCH 2/9] using user_api_key instead --- .../app/tests/api/routes/test_responses.py | 62 +++++-------------- 1 file changed, 15 insertions(+), 47 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index d42034f89..3d74106d9 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -5,7 +5,7 @@ from sqlmodel import select from app.api.routes.responses import router, process_response -from app.models import Project + # Wrap the router in a FastAPI app instance app = FastAPI() @@ -32,6 +32,7 @@ def test_responses_endpoint_success( mock_process_response, db, user_api_key_header: dict[str, str], + user_api_key, ): """Test the /responses endpoint for successful response creation.""" @@ -93,11 +94,6 @@ def return_mock_assistant(*args, **kwargs): ) mock_create_conversation.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - request_data = { "assistant_id": "assistant_dalgo", "question": "What is Dalgo?", @@ -140,6 +136,7 @@ def test_responses_endpoint_without_vector_store( mock_process_response, db, user_api_key_header, + user_api_key, ): """Test the /responses endpoint when assistant has no vector store configured.""" # Mock the background task to prevent actual execution @@ -183,11 +180,6 @@ def test_responses_endpoint_without_vector_store( ) mock_create_conversation.return_value = None - # Get the Glific project ID - glific_project = db.exec(select(Project).where(Project.name == "Glific")).first() - if not glific_project: - pytest.skip("Glific project not found in the database") - request_data = { "assistant_id": "assistant_123", "question": "What is Glific?", @@ -317,6 +309,7 @@ def test_responses_endpoint_with_file_search_results( mock_process_response, db, user_api_key_header, + user_api_key, ): """Test the /responses endpoint with file search results in the response.""" # Mock the background task to prevent actual execution @@ -374,11 +367,6 @@ def test_responses_endpoint_with_file_search_results( ) mock_create_conversation.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - request_data = { "assistant_id": "assistant_dalgo", "question": "What is Dalgo?", @@ -421,6 +409,7 @@ def test_responses_endpoint_with_ancestor_conversation_found( mock_process_response, db, user_api_key_header: dict[str, str], + user_api_key, ): """Test the /responses endpoint when a conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -470,11 +459,6 @@ def test_responses_endpoint_with_ancestor_conversation_found( mock_conversation.ancestor_response_id = "resp_ancestor1234567890abcdef1234567890" mock_get_conversation_by_ancestor_id.return_value = mock_conversation - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - request_data = { "assistant_id": "assistant_dalgo", "question": "What is Dalgo?", @@ -518,6 +502,7 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( mock_process_response, db, user_api_key_header: dict[str, str], + user_api_key, ): """Test the /responses endpoint when no conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -564,11 +549,6 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( # Setup mock conversation not found by ancestor ID mock_get_conversation_by_ancestor_id.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - request_data = { "assistant_id": "assistant_dalgo", "question": "What is Dalgo?", @@ -612,6 +592,7 @@ def test_responses_endpoint_without_response_id( mock_process_response, db, user_api_key_header: dict[str, str], + user_api_key, ): """Test the /responses endpoint when no response_id is provided.""" # Mock the background task to prevent actual execution @@ -655,11 +636,6 @@ def test_responses_endpoint_without_response_id( ) mock_create_conversation.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - request_data = { "assistant_id": "assistant_dalgo", "question": "What is Dalgo?", @@ -694,6 +670,7 @@ def test_process_response_ancestor_conversation_found( mock_create_conversation, mock_get_conversation_by_ancestor_id, db, + user_api_key, ): """Test process_response function when ancestor conversation is found.""" from app.api.routes.responses import ResponsesAPIRequest @@ -742,19 +719,14 @@ def test_process_response_ancestor_conversation_found( ) mock_create_conversation.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - # Call process_response process_response( request=request, client=mock_client, assistant=mock_assistant, tracer=mock_tracer, - project_id=dalgo_project.id, - organization_id=1, + project_id=user_api_key.project_id, + organization_id=user_api_key.organization_id, session=db, ) @@ -762,7 +734,7 @@ def test_process_response_ancestor_conversation_found( mock_get_conversation_by_ancestor_id.assert_called_once_with( session=db, ancestor_response_id="resp_ancestor1234567890abcdef1234567890", - project_id=dalgo_project.id, + project_id=user_api_key.project_id, ) # Verify OpenAI client was called with the conversation's response_id as @@ -790,6 +762,7 @@ def test_process_response_ancestor_conversation_not_found( mock_create_conversation, mock_get_conversation_by_ancestor_id, db, + user_api_key, ): """Test process_response function when no ancestor conversation is found.""" from app.api.routes.responses import ResponsesAPIRequest @@ -835,19 +808,14 @@ def test_process_response_ancestor_conversation_not_found( ) mock_create_conversation.return_value = None - # Get the Dalgo project ID - dalgo_project = db.exec(select(Project).where(Project.name == "Dalgo")).first() - if not dalgo_project: - pytest.skip("Dalgo project not found in the database") - # Call process_response process_response( request=request, client=mock_client, assistant=mock_assistant, tracer=mock_tracer, - project_id=dalgo_project.id, - organization_id=1, + project_id=user_api_key.project_id, + organization_id=user_api_key.organization_id, session=db, ) @@ -855,7 +823,7 @@ def test_process_response_ancestor_conversation_not_found( mock_get_conversation_by_ancestor_id.assert_called_once_with( session=db, ancestor_response_id="resp_ancestor1234567890abcdef1234567890", - project_id=dalgo_project.id, + project_id=user_api_key.project_id, ) # Verify OpenAI client was called with the original response_id as From 99cfff13b668de8456d2ebea4a5bd67c6945d04e Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 13 Aug 2025 12:46:34 +0530 Subject: [PATCH 3/9] using functions for fixtures --- .../app/tests/api/routes/test_responses.py | 490 +++++++----------- 1 file changed, 188 insertions(+), 302 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 3d74106d9..4988a2205 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -1,9 +1,6 @@ from unittest.mock import MagicMock, patch -import pytest from fastapi import FastAPI from fastapi.testclient import TestClient -from sqlmodel import select - from app.api.routes.responses import router, process_response @@ -13,6 +10,120 @@ client = TestClient(app) +def create_mock_assistant(model="gpt-4o", vector_store_ids=None, max_num_results=20): + """Create a mock assistant with default or custom values.""" + if vector_store_ids is None: + vector_store_ids = ["vs_test"] + + mock_assistant = MagicMock() + mock_assistant.model = model + mock_assistant.instructions = "Test instructions" + mock_assistant.temperature = 0.1 + mock_assistant.vector_store_ids = vector_store_ids + mock_assistant.max_num_results = max_num_results + return mock_assistant + + +def create_mock_openai_response( + response_id="resp_1234567890abcdef1234567890abcdef1234567890", + output_text="Test output", + model="gpt-4o", + output=None, + previous_response_id=None, +): + """Create a mock OpenAI response with default or custom values.""" + if output is None: + output = [] + + mock_response = MagicMock() + mock_response.id = response_id + mock_response.output_text = output_text + mock_response.model = model + mock_response.usage.input_tokens = 10 + mock_response.usage.output_tokens = 5 + mock_response.usage.total_tokens = 15 + mock_response.output = output + mock_response.previous_response_id = previous_response_id + return mock_response + + +def create_mock_file_search_results(): + """Create mock file search results for testing.""" + mock_hit1 = MagicMock() + mock_hit1.score = 0.95 + mock_hit1.text = "First search result" + + mock_hit2 = MagicMock() + mock_hit2.score = 0.85 + mock_hit2.text = "Second search result" + + mock_file_search_call = MagicMock() + mock_file_search_call.type = "file_search_call" + mock_file_search_call.results = [mock_hit1, mock_hit2] + + return [mock_file_search_call] + + +def create_mock_conversation( + response_id="resp_latest1234567890abcdef1234567890", + ancestor_response_id="resp_ancestor1234567890abcdef1234567890", +): + """Create a mock conversation with default or custom values.""" + mock_conversation = MagicMock() + mock_conversation.response_id = response_id + mock_conversation.ancestor_response_id = ancestor_response_id + return mock_conversation + + +def setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + assistant_model="gpt-4o", + vector_store_ids=None, + conversation_found=True, + response_output=None, +): + """Setup common mocks used across multiple tests.""" + # Setup mock credentials + mock_get_credential.return_value = {"api_key": "test_api_key"} + + # Setup mock assistant + mock_assistant = create_mock_assistant(assistant_model, vector_store_ids) + mock_get_assistant.return_value = mock_assistant + + # Setup mock OpenAI client + mock_client = MagicMock() + mock_openai.return_value = mock_client + + # Setup mock response + mock_response = create_mock_openai_response(output=response_output) + mock_client.responses.create.return_value = mock_response + + # Setup mock tracer + mock_tracer = MagicMock() + mock_tracer_class.return_value = mock_tracer + + # Setup mock CRUD functions + mock_get_ancestor_id_from_response.return_value = ( + "resp_ancestor1234567890abcdef1234567890" + ) + mock_create_conversation.return_value = None + + # Setup mock conversation if needed + if conversation_found: + mock_conversation = create_mock_conversation() + mock_get_conversation_by_ancestor_id.return_value = mock_conversation + else: + mock_get_conversation_by_ancestor_id.return_value = None + + return mock_client, mock_assistant + + @patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") @@ -39,60 +150,16 @@ def test_responses_endpoint_success( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - configure to return different values based on provider - def mock_get_credentials_by_provider(*args, **kwargs): - provider = kwargs.get("provider") - if provider == "openai": - return {"api_key": "test_api_key"} - elif provider == "langfuse": - return { - "public_key": "test_public_key", - "secret_key": "test_secret_key", - "host": "https://cloud.langfuse.com", - } - return None - - mock_get_credential.side_effect = mock_get_credentials_by_provider - - # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 - - # Configure mock to return the assistant for any call - def return_mock_assistant(*args, **kwargs): - return mock_assistant - - mock_get_assistant.side_effect = return_mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup the mock response object with proper response ID format - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = None - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_ancestor1234567890abcdef1234567890" + # Setup common mocks + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, ) - mock_create_conversation.return_value = None request_data = { "assistant_id": "assistant_dalgo", @@ -142,43 +209,18 @@ def test_responses_endpoint_without_vector_store( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} - - # Setup mock assistant without vector store - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = [] # No vector store configured - mock_assistant.max_num_results = 20 - mock_get_assistant.return_value = mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup the mock response object with proper response ID format - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output" - mock_response.model = "gpt-4" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = None - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_ancestor1234567890abcdef1234567890" + # Setup common mocks with no vector store + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + assistant_model="gpt-4", + vector_store_ids=[], ) - mock_create_conversation.return_value = None request_data = { "assistant_id": "assistant_123", @@ -234,11 +276,7 @@ def test_responses_endpoint_no_openai_credentials( ): """Test the /responses endpoint when OpenAI credentials are not configured.""" # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = [] + mock_assistant = create_mock_assistant() mock_get_assistant.return_value = mock_assistant # Setup mock credentials to return None (no credentials) @@ -267,11 +305,7 @@ def test_responses_endpoint_missing_api_key_in_credentials( ): """Test the /responses endpoint when credentials exist but don't have api_key.""" # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = [] + mock_assistant = create_mock_assistant() mock_get_assistant.return_value = mock_assistant # Setup mock credentials without api_key @@ -315,57 +349,17 @@ def test_responses_endpoint_with_file_search_results( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} - - # Setup mock assistant with vector store - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 - mock_get_assistant.return_value = mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup mock file search results - mock_hit1 = MagicMock() - mock_hit1.score = 0.95 - mock_hit1.text = "First search result" - - mock_hit2 = MagicMock() - mock_hit2.score = 0.85 - mock_hit2.text = "Second search result" - - mock_file_search_call = MagicMock() - mock_file_search_call.type = "file_search_call" - mock_file_search_call.results = [mock_hit1, mock_hit2] - - # Setup the mock response object with file search results and proper response ID - # format - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output with search results" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [mock_file_search_call] - mock_response.previous_response_id = None - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_ancestor1234567890abcdef1234567890" + # Setup common mocks with file search results + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + response_output=create_mock_file_search_results(), ) - mock_create_conversation.return_value = None request_data = { "assistant_id": "assistant_dalgo", @@ -415,49 +409,17 @@ def test_responses_endpoint_with_ancestor_conversation_found( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} - - # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 - mock_get_assistant.return_value = mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup the mock response object - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_ancestor1234567890abcdef1234567890" + # Setup common mocks with conversation found + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + conversation_found=True, ) - mock_create_conversation.return_value = None - - # Setup mock conversation found by ancestor ID - mock_conversation = MagicMock() - mock_conversation.response_id = "resp_latest1234567890abcdef1234567890" - mock_conversation.ancestor_response_id = "resp_ancestor1234567890abcdef1234567890" - mock_get_conversation_by_ancestor_id.return_value = mock_conversation request_data = { "assistant_id": "assistant_dalgo", @@ -508,46 +470,17 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} - - # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 - mock_get_assistant.return_value = mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup the mock response object - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_ancestor1234567890abcdef1234567890" + # Setup common mocks with conversation not found + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, + conversation_found=False, ) - mock_create_conversation.return_value = None - - # Setup mock conversation not found by ancestor ID - mock_get_conversation_by_ancestor_id.return_value = None request_data = { "assistant_id": "assistant_dalgo", @@ -598,43 +531,16 @@ def test_responses_endpoint_without_response_id( # Mock the background task to prevent actual execution mock_process_response.return_value = None - # Setup mock credentials - mock_get_credential.return_value = {"api_key": "test_api_key"} - - # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 - mock_get_assistant.return_value = mock_assistant - - # Setup mock OpenAI client - mock_client = MagicMock() - mock_openai.return_value = mock_client - - # Setup the mock response object - mock_response = MagicMock() - mock_response.id = "resp_1234567890abcdef1234567890abcdef1234567890" - mock_response.output_text = "Test output" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = None - mock_client.responses.create.return_value = mock_response - - # Setup mock tracer - mock_tracer = MagicMock() - mock_tracer_class.return_value = mock_tracer - - # Setup mock CRUD functions - mock_get_ancestor_id_from_response.return_value = ( - "resp_1234567890abcdef1234567890abcdef1234567890" + # Setup common mocks + mock_client, mock_assistant = setup_common_mocks( + mock_get_credential, + mock_get_assistant, + mock_openai, + mock_tracer_class, + mock_get_ancestor_id_from_response, + mock_create_conversation, + mock_get_conversation_by_ancestor_id, ) - mock_create_conversation.return_value = None request_data = { "assistant_id": "assistant_dalgo", @@ -684,33 +590,22 @@ def test_process_response_ancestor_conversation_found( ) # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 + mock_assistant = create_mock_assistant() # Setup mock OpenAI client mock_client = MagicMock() - mock_response = MagicMock() - mock_response.id = "resp_new1234567890abcdef1234567890abcdef" - mock_response.output_text = "Test response" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = "resp_latest1234567890abcdef1234567890" + mock_response = create_mock_openai_response( + response_id="resp_new1234567890abcdef1234567890abcdef", + output_text="Test response", + previous_response_id="resp_latest1234567890abcdef1234567890", + ) mock_client.responses.create.return_value = mock_response # Setup mock tracer mock_tracer = MagicMock() # Setup mock conversation found by ancestor ID - mock_conversation = MagicMock() - mock_conversation.response_id = "resp_latest1234567890abcdef1234567890" - mock_conversation.ancestor_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_conversation = create_mock_conversation() mock_get_conversation_by_ancestor_id.return_value = mock_conversation # Setup mock CRUD functions @@ -776,24 +671,15 @@ def test_process_response_ancestor_conversation_not_found( ) # Setup mock assistant - mock_assistant = MagicMock() - mock_assistant.model = "gpt-4o" - mock_assistant.instructions = "Test instructions" - mock_assistant.temperature = 0.1 - mock_assistant.vector_store_ids = ["vs_test"] - mock_assistant.max_num_results = 20 + mock_assistant = create_mock_assistant() # Setup mock OpenAI client mock_client = MagicMock() - mock_response = MagicMock() - mock_response.id = "resp_new1234567890abcdef1234567890abcdef" - mock_response.output_text = "Test response" - mock_response.model = "gpt-4o" - mock_response.usage.input_tokens = 10 - mock_response.usage.output_tokens = 5 - mock_response.usage.total_tokens = 15 - mock_response.output = [] - mock_response.previous_response_id = "resp_ancestor1234567890abcdef1234567890" + mock_response = create_mock_openai_response( + response_id="resp_new1234567890abcdef1234567890abcdef", + output_text="Test response", + previous_response_id="resp_ancestor1234567890abcdef1234567890", + ) mock_client.responses.create.return_value = mock_response # Setup mock tracer From 58ea5b7e54d94e1693890f014b1510ee3b043664 Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Thu, 14 Aug 2025 18:32:59 +0530 Subject: [PATCH 4/9] fixing coderabbit suggestions --- backend/app/tests/api/routes/test_responses.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 4988a2205..0e4317e8e 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -141,9 +141,7 @@ def test_responses_endpoint_success( mock_get_credential, mock_openai, mock_process_response, - db, user_api_key_header: dict[str, str], - user_api_key, ): """Test the /responses endpoint for successful response creation.""" From 636715625d32a36225ea7a8e1b37140767f02ecc Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Thu, 14 Aug 2025 18:41:45 +0530 Subject: [PATCH 5/9] few more cleanups --- .../app/tests/api/routes/test_responses.py | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 0e4317e8e..198f66a73 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -47,23 +47,6 @@ def create_mock_openai_response( return mock_response -def create_mock_file_search_results(): - """Create mock file search results for testing.""" - mock_hit1 = MagicMock() - mock_hit1.score = 0.95 - mock_hit1.text = "First search result" - - mock_hit2 = MagicMock() - mock_hit2.score = 0.85 - mock_hit2.text = "Second search result" - - mock_file_search_call = MagicMock() - mock_file_search_call.type = "file_search_call" - mock_file_search_call.results = [mock_hit1, mock_hit2] - - return [mock_file_search_call] - - def create_mock_conversation( response_id="resp_latest1234567890abcdef1234567890", ancestor_response_id="resp_ancestor1234567890abcdef1234567890", @@ -199,9 +182,7 @@ def test_responses_endpoint_without_vector_store( mock_get_credential, mock_openai, mock_process_response, - db, user_api_key_header, - user_api_key, ): """Test the /responses endpoint when assistant has no vector store configured.""" # Mock the background task to prevent actual execution @@ -269,7 +250,6 @@ def test_responses_endpoint_assistant_not_found( def test_responses_endpoint_no_openai_credentials( mock_get_assistant, mock_get_credential, - db, user_api_key_header, ): """Test the /responses endpoint when OpenAI credentials are not configured.""" @@ -322,66 +302,6 @@ def test_responses_endpoint_missing_api_key_in_credentials( assert "OpenAI API key not configured" in response_json["error"] -@patch("app.api.routes.responses.process_response") -@patch("app.api.routes.responses.OpenAI") -@patch("app.api.routes.responses.get_provider_credential") -@patch("app.api.routes.responses.get_assistant_by_id") -@patch("app.api.routes.responses.LangfuseTracer") -@patch("app.api.routes.responses.get_ancestor_id_from_response") -@patch("app.api.routes.responses.create_conversation") -@patch("app.api.routes.responses.get_conversation_by_ancestor_id") -def test_responses_endpoint_with_file_search_results( - mock_get_conversation_by_ancestor_id, - mock_create_conversation, - mock_get_ancestor_id_from_response, - mock_tracer_class, - mock_get_assistant, - mock_get_credential, - mock_openai, - mock_process_response, - db, - user_api_key_header, - user_api_key, -): - """Test the /responses endpoint with file search results in the response.""" - # Mock the background task to prevent actual execution - mock_process_response.return_value = None - - # Setup common mocks with file search results - mock_client, mock_assistant = setup_common_mocks( - mock_get_credential, - mock_get_assistant, - mock_openai, - mock_tracer_class, - mock_get_ancestor_id_from_response, - mock_create_conversation, - mock_get_conversation_by_ancestor_id, - response_output=create_mock_file_search_results(), - ) - - request_data = { - "assistant_id": "assistant_dalgo", - "question": "What is Dalgo?", - "callback_url": "http://example.com/callback", - } - - response = client.post("/responses", json=request_data, headers=user_api_key_header) - - assert response.status_code == 200 - response_json = response.json() - assert response_json["success"] is True - assert response_json["data"]["status"] == "processing" - assert response_json["data"]["message"] == "Response creation started" - - # Verify that the background task was scheduled with correct parameters - mock_process_response.assert_called_once() - call_args = mock_process_response.call_args - assert call_args[0][0].assistant_id == "assistant_dalgo" - assert call_args[0][0].question == "What is Dalgo?" - assert call_args[0][0].callback_url == "http://example.com/callback" - assert call_args[0][0].response_id is None - - @patch("app.api.routes.responses.process_response") @patch("app.api.routes.responses.OpenAI") @patch("app.api.routes.responses.get_provider_credential") From c447b6fb3d9a006fc029b98f01a62587c2a602e7 Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 20 Aug 2025 15:11:53 +0530 Subject: [PATCH 6/9] removing unnecessary params --- backend/app/tests/api/routes/test_responses.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 198f66a73..fa8044409 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -226,7 +226,6 @@ def test_responses_endpoint_without_vector_store( @patch("app.api.routes.responses.get_assistant_by_id") def test_responses_endpoint_assistant_not_found( mock_get_assistant, - db, user_api_key_header, ): """Test the /responses endpoint when assistant is not found.""" @@ -278,7 +277,6 @@ def test_responses_endpoint_no_openai_credentials( def test_responses_endpoint_missing_api_key_in_credentials( mock_get_assistant, mock_get_credential, - db, user_api_key_header, ): """Test the /responses endpoint when credentials exist but don't have api_key.""" @@ -319,9 +317,7 @@ def test_responses_endpoint_with_ancestor_conversation_found( mock_get_credential, mock_openai, mock_process_response, - db, user_api_key_header: dict[str, str], - user_api_key, ): """Test the /responses endpoint when a conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -380,9 +376,7 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( mock_get_credential, mock_openai, mock_process_response, - db, user_api_key_header: dict[str, str], - user_api_key, ): """Test the /responses endpoint when no conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -441,9 +435,7 @@ def test_responses_endpoint_without_response_id( mock_get_credential, mock_openai, mock_process_response, - db, user_api_key_header: dict[str, str], - user_api_key, ): """Test the /responses endpoint when no response_id is provided.""" # Mock the background task to prevent actual execution From 14a36779098938181ac507e56fc0d2162025e350 Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 20 Aug 2025 15:23:20 +0530 Subject: [PATCH 7/9] cleanups for testcases --- .../app/tests/api/routes/test_responses.py | 75 +++++++++++-------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index fa8044409..52c53d7c1 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -1,13 +1,5 @@ from unittest.mock import MagicMock, patch -from fastapi import FastAPI -from fastapi.testclient import TestClient -from app.api.routes.responses import router, process_response - - -# Wrap the router in a FastAPI app instance -app = FastAPI() -app.include_router(router) -client = TestClient(app) +from app.api.routes.responses import process_response def create_mock_assistant(model="gpt-4o", vector_store_ids=None, max_num_results=20): @@ -125,6 +117,7 @@ def test_responses_endpoint_success( mock_openai, mock_process_response, user_api_key_header: dict[str, str], + client, ): """Test the /responses endpoint for successful response creation.""" @@ -148,7 +141,9 @@ def test_responses_endpoint_success( "callback_url": "http://example.com/callback", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() @@ -183,6 +178,7 @@ def test_responses_endpoint_without_vector_store( mock_openai, mock_process_response, user_api_key_header, + client, ): """Test the /responses endpoint when assistant has no vector store configured.""" # Mock the background task to prevent actual execution @@ -207,7 +203,9 @@ def test_responses_endpoint_without_vector_store( "callback_url": "http://example.com/callback", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() assert response_json["success"] is True @@ -227,6 +225,7 @@ def test_responses_endpoint_without_vector_store( def test_responses_endpoint_assistant_not_found( mock_get_assistant, user_api_key_header, + client, ): """Test the /responses endpoint when assistant is not found.""" # Setup mock assistant to return None (not found) @@ -238,10 +237,13 @@ def test_responses_endpoint_assistant_not_found( "callback_url": "http://example.com/callback", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 404 response_json = response.json() - assert response_json["detail"] == "Assistant not found or not active" + assert response_json["success"] is False + assert response_json["error"] == "Assistant not found or not active" @patch("app.api.routes.responses.get_provider_credential") @@ -250,6 +252,7 @@ def test_responses_endpoint_no_openai_credentials( mock_get_assistant, mock_get_credential, user_api_key_header, + client, ): """Test the /responses endpoint when OpenAI credentials are not configured.""" # Setup mock assistant @@ -265,7 +268,9 @@ def test_responses_endpoint_no_openai_credentials( "callback_url": "http://example.com/callback", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() assert response_json["success"] is False @@ -278,6 +283,7 @@ def test_responses_endpoint_missing_api_key_in_credentials( mock_get_assistant, mock_get_credential, user_api_key_header, + client, ): """Test the /responses endpoint when credentials exist but don't have api_key.""" # Setup mock assistant @@ -293,7 +299,9 @@ def test_responses_endpoint_missing_api_key_in_credentials( "callback_url": "http://example.com/callback", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() assert response_json["success"] is False @@ -318,6 +326,7 @@ def test_responses_endpoint_with_ancestor_conversation_found( mock_openai, mock_process_response, user_api_key_header: dict[str, str], + client, ): """Test the /responses endpoint when a conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -342,7 +351,9 @@ def test_responses_endpoint_with_ancestor_conversation_found( "response_id": "resp_ancestor1234567890abcdef1234567890", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() @@ -377,6 +388,7 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( mock_openai, mock_process_response, user_api_key_header: dict[str, str], + client, ): """Test the /responses endpoint when no conversation is found by ancestor ID.""" # Mock the background task to prevent actual execution @@ -401,7 +413,9 @@ def test_responses_endpoint_with_ancestor_conversation_not_found( "response_id": "resp_ancestor1234567890abcdef1234567890", } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() @@ -436,6 +450,7 @@ def test_responses_endpoint_without_response_id( mock_openai, mock_process_response, user_api_key_header: dict[str, str], + client, ): """Test the /responses endpoint when no response_id is provided.""" # Mock the background task to prevent actual execution @@ -459,7 +474,9 @@ def test_responses_endpoint_without_response_id( # No response_id provided } - response = client.post("/responses", json=request_data, headers=user_api_key_header) + response = client.post( + "/api/v1/responses", json=request_data, headers=user_api_key_header + ) assert response.status_code == 200 response_json = response.json() @@ -532,15 +549,12 @@ def test_process_response_ancestor_conversation_found( tracer=mock_tracer, project_id=user_api_key.project_id, organization_id=user_api_key.organization_id, - session=db, + ancestor_id=mock_conversation.response_id, + latest_conversation=mock_conversation, ) - # Verify get_conversation_by_ancestor_id was called with correct parameters - mock_get_conversation_by_ancestor_id.assert_called_once_with( - session=db, - ancestor_response_id="resp_ancestor1234567890abcdef1234567890", - project_id=user_api_key.project_id, - ) + # process_response doesn't call get_conversation_by_ancestor_id; endpoint resolves it + mock_get_conversation_by_ancestor_id.assert_not_called() # Verify OpenAI client was called with the conversation's response_id as # previous_response_id @@ -612,15 +626,12 @@ def test_process_response_ancestor_conversation_not_found( tracer=mock_tracer, project_id=user_api_key.project_id, organization_id=user_api_key.organization_id, - session=db, + ancestor_id=request.response_id, + latest_conversation=None, ) - # Verify get_conversation_by_ancestor_id was called with correct parameters - mock_get_conversation_by_ancestor_id.assert_called_once_with( - session=db, - ancestor_response_id="resp_ancestor1234567890abcdef1234567890", - project_id=user_api_key.project_id, - ) + # process_response doesn't call get_conversation_by_ancestor_id; endpoint resolves it + mock_get_conversation_by_ancestor_id.assert_not_called() # Verify OpenAI client was called with the original response_id as # previous_response_id From b9bc72fbf93f847efc7349dbf709e6613391c02d Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 20 Aug 2025 15:39:03 +0530 Subject: [PATCH 8/9] coderabbit suggestions --- backend/app/tests/api/routes/test_responses.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 52c53d7c1..42e3894da 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -125,7 +125,7 @@ def test_responses_endpoint_success( mock_process_response.return_value = None # Setup common mocks - mock_client, mock_assistant = setup_common_mocks( + setup_common_mocks( mock_get_credential, mock_get_assistant, mock_openai, @@ -248,9 +248,11 @@ def test_responses_endpoint_assistant_not_found( @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.process_response") def test_responses_endpoint_no_openai_credentials( mock_get_assistant, mock_get_credential, + mock_process_response, user_api_key_header, client, ): @@ -275,13 +277,17 @@ def test_responses_endpoint_no_openai_credentials( response_json = response.json() assert response_json["success"] is False assert "OpenAI API key not configured" in response_json["error"] + # Ensure background task was not scheduled + mock_process_response.assert_not_called() @patch("app.api.routes.responses.get_provider_credential") @patch("app.api.routes.responses.get_assistant_by_id") +@patch("app.api.routes.responses.process_response") def test_responses_endpoint_missing_api_key_in_credentials( mock_get_assistant, mock_get_credential, + mock_process_response, user_api_key_header, client, ): @@ -306,6 +312,8 @@ def test_responses_endpoint_missing_api_key_in_credentials( response_json = response.json() assert response_json["success"] is False assert "OpenAI API key not configured" in response_json["error"] + # Ensure background task was not scheduled + mock_process_response.assert_not_called() @patch("app.api.routes.responses.process_response") From 9f5d574ed8c6d85b6833998926d3c0355074671a Mon Sep 17 00:00:00 2001 From: AkhileshNegi Date: Wed, 20 Aug 2025 15:48:39 +0530 Subject: [PATCH 9/9] fixing the order --- backend/app/tests/api/routes/test_responses.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/test_responses.py b/backend/app/tests/api/routes/test_responses.py index 42e3894da..54a28ca12 100644 --- a/backend/app/tests/api/routes/test_responses.py +++ b/backend/app/tests/api/routes/test_responses.py @@ -250,9 +250,9 @@ def test_responses_endpoint_assistant_not_found( @patch("app.api.routes.responses.get_assistant_by_id") @patch("app.api.routes.responses.process_response") def test_responses_endpoint_no_openai_credentials( + mock_process_response, mock_get_assistant, mock_get_credential, - mock_process_response, user_api_key_header, client, ): @@ -285,9 +285,9 @@ def test_responses_endpoint_no_openai_credentials( @patch("app.api.routes.responses.get_assistant_by_id") @patch("app.api.routes.responses.process_response") def test_responses_endpoint_missing_api_key_in_credentials( + mock_process_response, mock_get_assistant, mock_get_credential, - mock_process_response, user_api_key_header, client, ):