bito-code-review[bot] commented on code in PR #39604:
URL: https://github.com/apache/superset/pull/39604#discussion_r3337417383
##########
tests/unit_tests/mcp_service/test_mcp_server.py:
##########
@@ -158,3 +158,26 @@ def
test_create_event_store_returns_none_when_redis_store_fails():
result = create_event_store(config)
assert result is None
+
+
+def test_create_auth_provider_uses_default_factory_for_mcp_api_key_only() ->
None:
+ """MCP_API_KEY_ENABLED=True should install auth even when FAB API keys are
off."""
+ from superset.mcp_service.server import _create_auth_provider
+
+ flask_app = MagicMock()
+ flask_app.config.get.side_effect = lambda key, default=None: {
+ "MCP_AUTH_FACTORY": None,
+ "MCP_AUTH_ENABLED": False,
+ "MCP_API_KEY_ENABLED": True,
+ "FAB_API_KEY_ENABLED": False,
+ }.get(key, default)
+ auth_provider = MagicMock()
+
+ with patch(
+ "superset.mcp_service.mcp_config.create_default_mcp_auth_factory",
+ return_value=auth_provider,
+ ) as create_default_mcp_auth_factory:
+ result = _create_auth_provider(flask_app)
+
+ assert result is auth_provider
+ create_default_mcp_auth_factory.assert_called_once_with(flask_app)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test coverage for key logic</b></div>
<div id="fix">
The new test correctly verifies `create_default_mcp_auth_factory` is called
when `MCP_API_KEY_ENABLED=True`, but it mocks this factory so
`_build_composite_verifier` (lines 402-411 in mcp_config.py) is never
exercised. The string-normalization logic that wraps `FAB_API_KEY_PREFIXES`
strings into lists is untested. Consider adding tests in `test_mcp_config.py`
that directly invoke `_build_composite_verifier` with various
`FAB_API_KEY_PREFIXES` configs (string `'sst_'`, list `['sst_']`, and invalid
values like `None` that trigger the TypeError guard).
</div>
</div>
<small><i>Code Review Run #68148e</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
