fix(mcp): set .embedded on update_dashboard test mocks#41499
Conversation
The shared dashboard_serializer change in #41195 added embedded_uuid=str(dashboard.embedded[0].uuid) if dashboard.embedded else None. Production is correct (real dashboard.embedded is an empty list when not embedded), but update_dashboard flows through the same serializer and its test mocks never set .embedded, so it was a truthy auto-Mock that passed the guard and then failed to subscript, turning master's Python-Unit CI red with "'Mock' object is not subscriptable". Setting embedded=[] on the shared mock builder matches the not-embedded production case. Test-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Review Agent Run #d0a400Actionable 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 #41499 +/- ##
==========================================
+ Coverage 63.95% 64.52% +0.56%
==========================================
Files 2664 2664
Lines 146223 146223
Branches 33733 33733
==========================================
+ Hits 93514 94347 +833
+ Misses 50991 50158 -833
Partials 1718 1718
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:
|
|
I approved this and an earlier PR from @nytai. I'll be merging the former due to FIFO. Thanks Evan! |
|
Thanks! That makes TWO less PRs... a win in my book ;) |
SUMMARY
Master's Python-Unit CI is red. Five tests in
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.pyfail with:The cause is the shared
dashboard_serializerinsuperset/mcp_service/dashboard/schemas.py. #41195 added:That production code is fine: a real
dashboard.embeddedis an empty list when the dashboard isn't embedded, so the guard short-circuits. The problem is thatupdate_dashboardroutes through the same serializer, and the test mocks intest_update_dashboard.pynever set.embedded. Mock hands back a truthy auto-Mock, theif dashboard.embeddedguard passes, anddashboard.embedded[0]blows up. #41195 only updated theget_dashboard_infomocks intest_dashboard_tools.py, not these.Fix is test-only: I set
embedded = []on the shared_mock_dashboardbuilder so it matches the not-embedded production case. No production code changes; the guard inschemas.pyis correct as-is. I checked the sibling tool tests and they don't reach this serializer line, so nothing else needed touching.References #41195.
BEFORE/AFTER
Before:
Python-Unitred, 5 failures intest_update_dashboard.py.After:
pytest tests/unit_tests/mcp_service/dashboard/tool/passes (117 passed locally).TESTING INSTRUCTIONS
ADDITIONAL INFORMATION