fix: resolve unexpected keyword arguments in test tool provider constructors#33242
fix: resolve unexpected keyword arguments in test tool provider constructors#33242warren830 wants to merge 1 commit intolanggenius:mainfrom
Conversation
…est tool provider constructors The test helper _create_test_tool_provider was passing keyword arguments that don't exist as __init__ parameters on the MappedAsDataclass models: - ApiToolProvider: removed icon_dark (no such column), replaced credentials dict with credentials_str JSON string, removed provider_type, added required schema/schema_type_str/tools_str fields - BuiltinToolProvider: removed description/icon/icon_dark (properties or non-existent), replaced credentials with encrypted_credentials, added required user_id field - WorkflowToolProvider: removed icon_dark (no such column), replaced workflow_id with app_id, added required label field - MCPToolProvider: removed description (no such column), replaced provider_icon with icon, added required server_url_hash field Also added import json for json.dumps usage.
dosubot
bot
added
the
size:S
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where a test helper function was attempting to initialize various Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes issues with unexpected keyword arguments in test tool provider constructors by aligning the test data with the model definitions. The changes are accurate and address the problem described. I have one suggestion to improve the robustness of the test data for MCPToolProvider by dynamically calculating the server_url_hash instead of using a hardcoded string. This will make the test data more realistic and prevent potential conflicts.
| @@ -1,3 +1,4 @@ | |||
| import json | |||
There was a problem hiding this comment.
To correctly calculate the server_url_hash for the MCPToolProvider test data, you'll need the hashlib module. Please import it here, keeping the standard library imports alphabetized.
| import json | |
| import hashlib | |
| import json |
| elif provider_type == "mcp": | ||
| provider = MCPToolProvider( | ||
| name=fake.company(), | ||
| description=fake.text(max_nb_chars=100), | ||
| provider_icon='{"background": "#FF6B6B", "content": "🔧"}', | ||
| icon='{"background": "#FF6B6B", "content": "🔧"}', | ||
| tenant_id="test_tenant_id", | ||
| user_id="test_user_id", | ||
| server_url="https://mcp.example.com", | ||
| server_url_hash="test_server_url_hash", | ||
| server_identifier="test_server", | ||
| tools='[{"name": "test_tool", "description": "Test tool"}]', | ||
| authed=True, |
There was a problem hiding this comment.
The server_url_hash should be derived from server_url to make the test data more realistic and robust. Using a hardcoded string can lead to test fragility. This suggestion refactors the MCPToolProvider instantiation to calculate the SHA256 hash from the URL, which is a better practice for test data generation.
| elif provider_type == "mcp": | |
| provider = MCPToolProvider( | |
| name=fake.company(), | |
| description=fake.text(max_nb_chars=100), | |
| provider_icon='{"background": "#FF6B6B", "content": "🔧"}', | |
| icon='{"background": "#FF6B6B", "content": "🔧"}', | |
| tenant_id="test_tenant_id", | |
| user_id="test_user_id", | |
| server_url="https://mcp.example.com", | |
| server_url_hash="test_server_url_hash", | |
| server_identifier="test_server", | |
| tools='[{"name": "test_tool", "description": "Test tool"}]', | |
| authed=True, | |
| elif provider_type == "mcp": | |
| server_url = "https://mcp.example.com" | |
| provider = MCPToolProvider( | |
| name=fake.company(), | |
| icon='{"background": "#FF6B6B", "content": "🔧"}', | |
| tenant_id="test_tenant_id", | |
| user_id="test_user_id", | |
| server_url=server_url, | |
| server_url_hash=hashlib.sha256(server_url.encode('utf-8')).hexdigest(), | |
| server_identifier="test_server", | |
| tools='[{"name": "test_tool", "description": "Test tool"}]', | |
| authed=True, | |
| ) |
1 participant
Description
Fixes #32592
The test helper
_create_test_tool_providerintest_tools_transform_service.pywas passing keyword arguments that don't exist as__init__parameters on theMappedAsDataclassmodels.Changes
ApiToolProvider:
icon_dark(no such column)credentialsdict withcredentials_strJSON string (matching the actual column)provider_type(not a constructor parameter)schema,schema_type_str,tools_strfieldsBuiltinToolProvider:
description,icon,icon_dark(these are either properties or non-existent columns)credentialswithencrypted_credentials(matching the actual column)user_idfieldWorkflowToolProvider:
icon_dark(no such column)workflow_idwithapp_id(matching the actual column)labelfieldMCPToolProvider:
description(no such column)provider_iconwithicon(matching the actual column)server_url_hashfieldAlso added
import jsonforjson.dumpsusage in serializing credential dicts.Type of Change