test(mcp): fix update_dashboard test mock for embedded_uuid - #41471
test(mcp): fix update_dashboard test mock for embedded_uuid#41471gkneighb wants to merge 1 commit into
Conversation
PR apache#41195 added `embedded_uuid=str(dashboard.embedded[0].uuid) if dashboard.embedded else None` to the shared dashboard_serializer but only updated the get_dashboard_info test mocks. update_dashboard's _mock_dashboard left .embedded as an auto-Mock (truthy but not subscriptable), so the serializer hit 'Mock object is not subscriptable' and 5 update_dashboard tests failed on master. Model .embedded as an empty list (matching a real dashboard with no embedded config) so the serializer resolves embedded_uuid to None. Product code is unaffected: on a real Dashboard, .embedded is a list relationship (empty -> None). Test-only fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code Review Agent Run #20ed2eActionable 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 AI Code Review powered by |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| # dashboard_serializer reads embedded[0].uuid when truthy (added in #41195); | ||
| # model it as an empty list so the serializer resolves embedded_uuid to None. | ||
| dashboard.embedded = [] |
There was a problem hiding this comment.
Suggestion: Because this PR modifies _mock_dashboard, add an explicit return type annotation to the function signature so the updated helper is fully typed (for example returning a Mock). [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The changed helper _mock_dashboard is a modified Python function and its signature in the final file still lacks a return type annotation. The custom rule requires updated Python functions to be fully typed, so this is a real violation.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
**Line:** 84:86
**Comment:**
*Custom Rule: Because this PR modifies `_mock_dashboard`, add an explicit return type annotation to the function signature so the updated helper is fully typed (for example returning a `Mock`).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The suggestion to add an explicit return type annotation to the Here is the corrected function signature: from unittest.mock import Mock
def _mock_dashboard(...) -> Mock:
# ... existing implementation ...Regarding other comments on this PR, the review discussion is currently empty, so there are no additional comments to address at this time. tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41471 +/- ##
==========================================
+ Coverage 63.95% 64.52% +0.56%
==========================================
Files 2664 2664
Lines 146184 146184
Branches 33724 33724
==========================================
+ Hits 93497 94325 +828
+ Misses 50972 50142 -830
- Partials 1715 1717 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Whoops, we missed your PR to fix failing mainline branch and merged a later one. Sorry about that! I'll be closing this and will give more attention to PR's chronology that fixing a persistent issue like breaking mainline in the future. |
2 participants
SUMMARY
#41195addedembedded_uuid=str(dashboard.embedded[0].uuid) if dashboard.embedded else Noneto the shareddashboard_serializerand updated theget_dashboard_infotest mocks, but theupdate_dashboardtest's_mock_dashboardhelper was not updated. That helper builds aMockwith "EVERY field the serializer touches explicitly set" — except.embedded, which therefore resolved to an auto-Mock(truthy but not subscriptable). As a result the serializer hitsTypeError: 'Mock' object is not subscriptableand 5 tests intest_update_dashboard.pyfail onmaster:test_update_with_no_fields_is_nooptest_update_title_and_slug_and_publishedtest_update_descriptiontest_empty_slug_clears_slugThis is a test-only gap. Product code is unaffected: on a real
Dashboard,.embeddedis a SQLAlchemy list relationship — empty → falsy →embedded_uuid=None; non-empty →embedded[0].uuidworks.The fix models
.embeddedas an empty list in_mock_dashboard(matching a dashboard with no embedded configuration), so the serializer resolvesembedded_uuidtoNone, consistent with how#41195fixed theget_dashboard_infomocks.TESTING INSTRUCTIONS
Before:
5 failed, 3 passed. After:8 passed.ADDITIONAL INFORMATION
Follow-up to #41195 (test mocks only).
🤖 Generated with Claude Code