-
Notifications
You must be signed in to change notification settings - Fork 16.5k
fix(mcp): access wrapped function in dataset tool tests #36295
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
fix(mcp): access wrapped function in dataset tool tests #36295
Conversation
Code Review Agent Run #4cddeeActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36295 +/- ##
==========================================
+ Coverage 60.48% 67.99% +7.50%
==========================================
Files 1931 636 -1295
Lines 76236 46817 -29419
Branches 8568 5081 -3487
==========================================
- Hits 46114 31831 -14283
+ Misses 28017 13710 -14307
+ Partials 2105 1276 -829
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
22b7224 to
d5e34ec
Compare
d5e34ec to
d559471
Compare
The list_datasets function is decorated with @tool, which makes it a FunctionTool object. Tests that directly call the function need to access the underlying wrapped function using __wrapped__. This fixes two failing tests: - TestDatasetSortableColumns::test_list_datasets_with_valid_order_column - TestDatasetSortableColumns::test_default_ordering
cdd2210 to
70a3560
Compare
Antonio-RiveroMartnez
left a comment
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.
lgtm
(cherry picked from commit 2af5a5a)
(cherry picked from commit 2af5a5a)
(cherry picked from commit 2af5a5a)
SUMMARY
Fixed two failing unit tests in the MCP dataset tools test suite by updating them to use the proper MCP client testing pattern.
The tests were incorrectly trying to call the
list_datasetsfunction directly, which is decorated with@toolfromsuperset_core.mcp. The fix updates the tests to use the MCP client approach (viaasync with Client(mcp_server)) to call the tools, matching the pattern used in other MCP test files (e.g., dashboard tests).Failing tests:
TestDatasetSortableColumns::test_list_datasets_with_valid_order_columnTestDatasetSortableColumns::test_default_orderingChanges:
async with Client(mcp_server) as clientandclient.call_tool()BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Test fix only
TESTING INSTRUCTIONS
Expected: All 20 tests pass
ADDITIONAL INFORMATION