madhushreeag commented on code in PR #43028:
URL: https://github.com/apache/superset/pull/43028#discussion_r3760405179
##########
tests/unit_tests/mcp_service/test_pooled_screenshot.py:
##########
@@ -17,20 +17,21 @@
from unittest.mock import MagicMock, patch
+import pytest
+
from superset.mcp_service.screenshot.pooled_screenshot import
PooledBaseScreenshot
-@patch("superset.mcp_service.screenshot.pooled_screenshot.retry_screenshot_operation")
-def test_get_screenshot_accepts_base_log_context(
- mock_retry_screenshot_operation: MagicMock,
-) -> None:
+def test_get_screenshot_raises_when_playwright_unavailable() -> None:
+ """get_screenshot raises RuntimeError when Playwright is unavailable."""
screenshot = PooledBaseScreenshot("http://example.com";, "digest")
user = MagicMock()
- screenshot.get_screenshot(user, log_context="cache_key=abc")
-
- mock_retry_screenshot_operation.assert_called_once_with(
- screenshot._get_screenshot_internal, # pylint:
disable=protected-access
- user,
- None,
- )
+ with patch("superset.mcp_service.screenshot.pooled_screenshot.super") as
mock_super:
+ mock_super_instance = MagicMock()
+ mock_super.return_value = mock_super_instance
+ mock_super_instance.get_screenshot.side_effect = RuntimeError(
+ "Playwright is required"
+ )
Review Comment:
Replaced the super patch with
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", False) so the test now
exercises the real availability check in WebDriverPlaywright.get_screenshot()
rather than mocking the superclass call. The RuntimeError is now raised through
the actual code path.
--
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]
