Copilot commented on code in PR #39604:
URL: https://github.com/apache/superset/pull/39604#discussion_r3243625916
##########
tests/unit_tests/mcp_service/test_auth_api_key.py:
##########
@@ -54,100 +84,110 @@ def _disable_api_keys(app):
app.config["MCP_DEV_USERNAME"] = old_dev
+@contextmanager
+def _mock_sm_ctx(app: SupersetApp, mock_sm: MagicMock):
+ """Push an app context with g.user cleared and appbuilder.sm mocked.
+
+ Defaults find_user_with_relationships to None so JWT/dev-user lookups
+ that hit the SM (via load_user_with_relationships) behave as "user not
+ found" without a real DB, matching the pre-refactor db.session behavior.
+ Tests that need a specific return value should set it on mock_sm directly.
+ """
+ mock_sm.find_user_with_relationships.return_value = None
Review Comment:
_mock_sm_ctx unconditionally accesses mock_sm.find_user_with_relationships,
but tests later pass MagicMock(spec=[]), which will raise AttributeError on
that access. Also, load_user_with_relationships() now delegates to the global
superset.security_manager.find_user_with_relationships (not app.appbuilder.sm),
so setting this on appbuilder.sm is ineffective. Remove this line or guard it
(e.g., only set it when the attribute exists), and if you need to avoid DB
lookups in these tests, patch
superset.security_manager.find_user_with_relationships or
superset.mcp_service.auth.load_user_with_relationships instead.
--
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]
