-
Notifications
You must be signed in to change notification settings - Fork 569
feat: error handling added in integrations with their test cases #155
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,12 +35,16 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MULTIPLE_TOOL_CALL_MESSAGES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IMAGE_URL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BASE64_IMAGE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INVALID_ROLE_MESSAGES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WEATHER_TOOL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CALCULATOR_TOOL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ALL_TOOLS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mock_tool_response, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_valid_chat_response, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_has_tool_calls, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_valid_image_response, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_valid_error_response, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_error_propagation, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extract_tool_calls, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| get_api_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| skip_if_no_api_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -544,6 +548,21 @@ def test_11_integration_specific_features(self, anthropic_client, test_config): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Should prefer calculator for math question | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert tool_calls[0]["name"] == "calculate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @skip_if_no_api_key("anthropic") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_12_error_handling_invalid_roles(self, anthropic_client, test_config): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Test Case 12: Error handling for invalid roles""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with pytest.raises(Exception) as exc_info: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| anthropic_client.messages.create( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model=get_model("anthropic", "chat"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| messages=INVALID_ROLE_MESSAGES, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| max_tokens=100, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Verify the error is properly caught and contains role-related information | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| error = exc_info.value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_valid_error_response(error, "tester") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_error_propagation(error, "anthropic") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+551
to
+565
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. 🧹 Nitpick (assertive) Improve error handling test specificity. The test implementation follows the correct pattern but could be improved:
- def test_12_error_handling_invalid_roles(self, anthropic_client, test_config):
+ def test_12_error_handling_invalid_roles(self, anthropic_client, test_config) -> None:
"""Test Case 12: Error handling for invalid roles"""
- with pytest.raises(Exception) as exc_info:
+ with pytest.raises(Exception, match=r".*(role|invalid|tester).*") as exc_info:The test correctly validates error propagation and follows the established pattern across other integration tests. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)552-552: Missing return type annotation for public function Add return type annotation: (ANN201) 552-552: Missing type annotation for function argument (ANN001) 552-552: Missing type annotation for function argument (ANN001) 552-552: Unused method argument: (ARG002) 554-554: (PT011) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Additional helper functions specific to Anthropic | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def extract_anthropic_tool_calls(response: Any) -> List[Dict[str, Any]]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,15 +31,19 @@ | |||||||||
| MULTIPLE_TOOL_CALL_MESSAGES, | ||||||||||
| IMAGE_URL, | ||||||||||
| BASE64_IMAGE, | ||||||||||
| INVALID_ROLE_MESSAGES, | ||||||||||
|
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. 🧹 Nitpick (assertive) Remove unused import. The - INVALID_ROLE_MESSAGES,📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)34-34: Remove unused import: (F401) 🤖 Prompt for AI Agents |
||||||||||
| WEATHER_TOOL, | ||||||||||
| CALCULATOR_TOOL, | ||||||||||
| assert_valid_chat_response, | ||||||||||
| assert_valid_image_response, | ||||||||||
| assert_valid_error_response, | ||||||||||
| assert_error_propagation, | ||||||||||
| get_api_key, | ||||||||||
| skip_if_no_api_key, | ||||||||||
| COMPARISON_KEYWORDS, | ||||||||||
| WEATHER_KEYWORDS, | ||||||||||
| LOCATION_KEYWORDS, | ||||||||||
| GENAI_INVALID_ROLE_CONTENT, | ||||||||||
| ) | ||||||||||
| from ..utils.config_loader import get_model | ||||||||||
|
|
||||||||||
|
|
@@ -422,6 +426,19 @@ def test_11_integration_specific_features(self, google_client, test_config): | |||||||||
|
|
||||||||||
| assert_valid_chat_response(response3) | ||||||||||
|
|
||||||||||
| @skip_if_no_api_key("google") | ||||||||||
| def test_12_error_handling_invalid_roles(self, google_client, test_config): | ||||||||||
| """Test Case 12: Error handling for invalid roles""" | ||||||||||
| with pytest.raises(Exception) as exc_info: | ||||||||||
| google_client.models.generate_content( | ||||||||||
| model=get_model("google", "chat"), contents=GENAI_INVALID_ROLE_CONTENT | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| # Verify the error is properly caught and contains role-related information | ||||||||||
| error = exc_info.value | ||||||||||
| assert_valid_error_response(error, "tester") | ||||||||||
| assert_error_propagation(error, "google") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| # Additional helper functions specific to Google GenAI | ||||||||||
| def extract_google_function_calls(response: Any) -> List[Dict[str, Any]]: | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,13 +36,18 @@ | |||||||||
| IMAGE_BASE64_MESSAGES, | ||||||||||
| MULTIPLE_IMAGES_MESSAGES, | ||||||||||
| COMPLEX_E2E_MESSAGES, | ||||||||||
| INVALID_ROLE_MESSAGES, | ||||||||||
| WEATHER_TOOL, | ||||||||||
| CALCULATOR_TOOL, | ||||||||||
| mock_tool_response, | ||||||||||
| assert_valid_chat_response, | ||||||||||
| assert_has_tool_calls, | ||||||||||
| assert_valid_image_response, | ||||||||||
| assert_valid_error_response, | ||||||||||
| assert_error_propagation, | ||||||||||
| extract_tool_calls, | ||||||||||
| get_api_key, | ||||||||||
| skip_if_no_api_key, | ||||||||||
|
Comment on lines
+49
to
+50
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. 🧹 Nitpick (assertive) Remove unused imports. The - get_api_key,
- skip_if_no_api_key,📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)49-49: Remove unused import (F401) 50-50: Remove unused import (F401) 🤖 Prompt for AI Agents |
||||||||||
| COMPARISON_KEYWORDS, | ||||||||||
| WEATHER_KEYWORDS, | ||||||||||
| LOCATION_KEYWORDS, | ||||||||||
|
|
@@ -339,6 +344,20 @@ def test_11_integration_specific_features(self, test_config): | |||||||||
|
|
||||||||||
| assert_valid_chat_response(response3) | ||||||||||
|
|
||||||||||
| def test_12_error_handling_invalid_roles(self, test_config): | ||||||||||
| """Test Case 12: Error handling for invalid roles""" | ||||||||||
| with pytest.raises(Exception) as exc_info: | ||||||||||
| litellm.completion( | ||||||||||
| model=get_model("litellm", "chat"), | ||||||||||
| messages=INVALID_ROLE_MESSAGES, | ||||||||||
| max_tokens=100, | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| # Verify the error is properly caught and contains role-related information | ||||||||||
| error = exc_info.value | ||||||||||
| assert_valid_error_response(error, "tester") | ||||||||||
| assert_error_propagation(error, "litellm") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| # Additional helper functions specific to LiteLLM | ||||||||||
| def extract_litellm_tool_calls(response: Any) -> List[Dict[str, Any]]: | ||||||||||
|
|
||||||||||
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.
🧹 Nitpick (assertive)
Remove unused import.
The
ALL_TOOLSimport is not used in this test file.- ALL_TOOLS,📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.11.9)
41-41:
..utils.common.ALL_TOOLSimported but unusedRemove unused import:
..utils.common.ALL_TOOLS(F401)
🤖 Prompt for AI Agents