-
Notifications
You must be signed in to change notification settings - Fork 73
LCORE-1206: add new integration tests for handling the attachments #1228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -330,6 +330,182 @@ async def test_query_v2_endpoint_with_attachments( | |
| assert response.response is not None | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_query_v2_endpoint_empty_payload( | ||
| test_config: AppConfig, | ||
| mock_llama_stack_client: AsyncMockType, | ||
| test_request: Request, | ||
| test_auth: AuthTuple, | ||
| ) -> None: | ||
| """Test query v2 endpoint with minimal payload (no attachments). | ||
|
|
||
| Verifies that a request with only the required query and no attachments | ||
| field does not break the handler and returns 200. | ||
| """ | ||
| _ = test_config | ||
| _ = mock_llama_stack_client | ||
|
|
||
| query_request = QueryRequest(query="what is kubernetes?") | ||
|
|
||
| response = await query_endpoint_handler( | ||
| request=test_request, | ||
| query_request=query_request, | ||
| auth=test_auth, | ||
| mcp_headers={}, | ||
| ) | ||
|
|
||
| assert getattr(response, "status_code", status.HTTP_200_OK) == status.HTTP_200_OK | ||
| assert response.conversation_id is not None | ||
| assert response.response is not None | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_query_v2_endpoint_empty_attachments_list( | ||
| test_config: AppConfig, | ||
| mock_llama_stack_client: AsyncMockType, | ||
| test_request: Request, | ||
| test_auth: AuthTuple, | ||
| ) -> None: | ||
| """Test query v2 endpoint accepts empty attachment list. | ||
|
|
||
| Verifies that POST /v1/query with attachments=[] returns 200 and | ||
| application/json response. | ||
| """ | ||
| _ = test_config | ||
| _ = mock_llama_stack_client | ||
|
|
||
| query_request = QueryRequest( | ||
| query="what is kubernetes?", | ||
| attachments=[], | ||
| ) | ||
|
|
||
| response = await query_endpoint_handler( | ||
| request=test_request, | ||
| query_request=query_request, | ||
| auth=test_auth, | ||
| mcp_headers={}, | ||
| ) | ||
|
|
||
| assert getattr(response, "status_code", status.HTTP_200_OK) == status.HTTP_200_OK | ||
| assert response.conversation_id is not None | ||
| assert response.response is not None | ||
|
Comment on lines
+389
to
+391
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same no-op status code assertion. Same issue as above - 🔧 Proposed fix- assert getattr(response, "status_code", status.HTTP_200_OK) == status.HTTP_200_OK
assert response.conversation_id is not None
assert response.response is not None🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_query_v2_endpoint_multiple_attachments( | ||
| test_config: AppConfig, | ||
| mock_llama_stack_client: AsyncMockType, | ||
| test_request: Request, | ||
| test_auth: AuthTuple, | ||
| ) -> None: | ||
| """Test query v2 endpoint with multiple attachments. | ||
|
|
||
| Verifies that two attachments (log + configuration) are accepted | ||
| and processed. | ||
| """ | ||
| _ = test_config | ||
| _ = mock_llama_stack_client | ||
|
|
||
| query_request = QueryRequest( | ||
| query="what is kubernetes?", | ||
| attachments=[ | ||
| Attachment( | ||
| attachment_type="log", | ||
| content_type="text/plain", | ||
| content="log content", | ||
| ), | ||
| Attachment( | ||
| attachment_type="configuration", | ||
| content_type="application/json", | ||
| content='{"key": "value"}', | ||
| ), | ||
| ], | ||
| ) | ||
|
|
||
| response = await query_endpoint_handler( | ||
| request=test_request, | ||
| query_request=query_request, | ||
| auth=test_auth, | ||
| mcp_headers={}, | ||
| ) | ||
|
|
||
| assert getattr(response, "status_code", status.HTTP_200_OK) == status.HTTP_200_OK | ||
| assert response.conversation_id is not None | ||
| assert response.response is not None | ||
|
Comment on lines
+432
to
+434
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same no-op status code assertion. Same issue as above - remove or fix the meaningless assertion. 🔧 Proposed fix- assert getattr(response, "status_code", status.HTTP_200_OK) == status.HTTP_200_OK
assert response.conversation_id is not None
assert response.response is not None🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_query_v2_endpoint_attachment_unknown_type_returns_422( | ||
| test_config: AppConfig, | ||
| mock_llama_stack_client: AsyncMockType, | ||
| test_request: Request, | ||
| test_auth: AuthTuple, | ||
| ) -> None: | ||
| """Test query v2 endpoint returns 422 for unknown attachment type.""" | ||
| _ = test_config | ||
| _ = mock_llama_stack_client | ||
|
|
||
| query_request = QueryRequest( | ||
| query="what is kubernetes?", | ||
| attachments=[ | ||
| Attachment( | ||
| attachment_type="unknown_type", | ||
| content_type="text/plain", | ||
| content="content", | ||
| ) | ||
| ], | ||
| ) | ||
|
|
||
| with pytest.raises(HTTPException) as exc_info: | ||
| await query_endpoint_handler( | ||
| request=test_request, | ||
| query_request=query_request, | ||
| auth=test_auth, | ||
| mcp_headers={}, | ||
| ) | ||
|
|
||
| assert exc_info.value.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT | ||
| assert isinstance(exc_info.value.detail, dict) | ||
| assert "unknown_type" in exc_info.value.detail["cause"] | ||
| assert "Invalid" in exc_info.value.detail["response"] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_query_v2_endpoint_attachment_unknown_content_type_returns_422( | ||
| test_config: AppConfig, | ||
| mock_llama_stack_client: AsyncMockType, | ||
| test_request: Request, | ||
| test_auth: AuthTuple, | ||
| ) -> None: | ||
| """Test query v2 endpoint returns 422 for unknown attachment content type.""" | ||
| _ = test_config | ||
| _ = mock_llama_stack_client | ||
|
|
||
| query_request = QueryRequest( | ||
| query="what is kubernetes?", | ||
| attachments=[ | ||
| Attachment( | ||
| attachment_type="log", | ||
| content_type="unknown/type", | ||
| content="content", | ||
| ) | ||
| ], | ||
| ) | ||
|
|
||
| with pytest.raises(HTTPException) as exc_info: | ||
| await query_endpoint_handler( | ||
| request=test_request, | ||
| query_request=query_request, | ||
| auth=test_auth, | ||
| mcp_headers={}, | ||
| ) | ||
|
|
||
| assert exc_info.value.status_code == status.HTTP_422_UNPROCESSABLE_CONTENT | ||
| assert isinstance(exc_info.value.detail, dict) | ||
| assert "unknown/type" in exc_info.value.detail["cause"] | ||
| assert "Invalid" in exc_info.value.detail["response"] | ||
|
|
||
|
|
||
| # ========================================== | ||
| # Tool Integration Tests | ||
| # ========================================== | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status code assertion is always true (no-op).
query_endpoint_handlerreturns aQueryResponseobject, not a FastAPIResponse. TheQueryResponsemodel doesn't have astatus_codeattribute, sogetattr(response, "status_code", status.HTTP_200_OK)will always return the default200, making this assertion meaningless.Either remove the status_code assertion (since the handler doesn't raise means success), or verify the response type directly.
🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents