You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by va...@apache.org on 2026/04/24 01:03:50 UTC
(superset) branch master updated: fix(reports): poll for spinner absence instead of snapshotting loading elements (#39579)
This is an automated email from the ASF dual-hosted git repository.
Vitor-Avila pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new f0d521dfc26 fix(reports): poll for spinner absence instead of snapshotting loading elements (#39579)
f0d521dfc26 is described below
commit f0d521dfc260447c011dd6b581cd3fa8b30a6899
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Thu Apr 23 18:03:43 2026 -0700
fix(reports): poll for spinner absence instead of snapshotting loading elements (#39579)
Co-authored-by: Claude Sonnet 4.6 <no...@anthropic.com>
---
superset/utils/webdriver.py | 6 +-
tests/unit_tests/utils/webdriver_test.py | 108 +++++++++++++++++++++++++++++++
2 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index f38c67fd64c..3d61411b737 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -300,8 +300,10 @@ class WebDriverPlaywright(WebDriverProxy):
logger.debug(
"Wait for loading element of charts to be gone at url: %s", url
)
- for loading_element in page.locator(".loading").all():
- loading_element.wait_for(state="detached")
+ page.wait_for_function(
+ "() => document.querySelectorAll('.loading').length === 0",
+ timeout=self._screenshot_load_wait * 1000,
+ )
except PlaywrightTimeout:
logger.warning(
"Timed out waiting for charts to load at url %s", url
diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py
index ad55f4b68e6..cf7447eef1d 100644
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -640,6 +640,114 @@ class TestWebDriverPlaywrightErrorHandling:
mock_button.click.assert_called_once()
mock_close_button.click.assert_called_once()
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver.sync_playwright")
+ @patch("superset.utils.webdriver.app")
+ def test_uses_wait_for_function_to_detect_spinners(
+ self, mock_app, mock_sync_playwright
+ ):
+ """wait_for_function polls for spinner absence rather than snapshotting."""
+ mock_user = MagicMock()
+ mock_user.username = "test_user"
+ mock_app.config = {
+ "WEBDRIVER_OPTION_ARGS": [],
+ "WEBDRIVER_WINDOW": {"pixel_density": 1},
+ "SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT": 30000,
+ "SCREENSHOT_PLAYWRIGHT_WAIT_EVENT": "networkidle",
+ "SCREENSHOT_SELENIUM_HEADSTART": 0,
+ "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 0,
+ "SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
+ "SCREENSHOT_TILED_ENABLED": False,
+ "SCREENSHOT_LOCATE_WAIT": 10,
+ "SCREENSHOT_LOAD_WAIT": 60,
+ "SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE": 10,
+ "SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE": 10,
+ }
+
+ mock_playwright_instance = MagicMock()
+ mock_browser = MagicMock()
+ mock_context = MagicMock()
+ mock_page = MagicMock()
+ mock_element = MagicMock()
+
+ mock_sync_playwright.return_value.__enter__.return_value = (
+ mock_playwright_instance
+ )
+ mock_playwright_instance.chromium.launch.return_value = mock_browser
+ mock_browser.new_context.return_value = mock_context
+ mock_context.new_page.return_value = mock_page
+ mock_page.locator.return_value = mock_element
+ mock_element.screenshot.return_value = b"screenshot"
+
+ with patch.object(WebDriverPlaywright, "auth", return_value=mock_context):
+ driver = WebDriverPlaywright("chrome")
+ driver.get_screenshot("http://example.com", "test-element", mock_user)
+
+ mock_page.wait_for_function.assert_called_once_with(
+ "() => document.querySelectorAll('.loading').length === 0",
+ timeout=60 * 1000,
+ )
+ # Guard against reintroducing the old snapshot-based approach
+ loading_locator_calls = [
+ c for c in mock_page.locator.call_args_list if c.args == (".loading",)
+ ]
+ assert loading_locator_calls == []
+
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver.sync_playwright")
+ @patch("superset.utils.webdriver.logger")
+ @patch("superset.utils.webdriver.app")
+ def test_spinner_timeout_logs_warning_and_raises(
+ self, mock_app, mock_logger, mock_sync_playwright
+ ):
+ """Spinner timeout is logged as a warning and re-raised."""
+ from superset.utils.webdriver import PlaywrightTimeout
+
+ mock_user = MagicMock()
+ mock_user.username = "test_user"
+ mock_app.config = {
+ "WEBDRIVER_OPTION_ARGS": [],
+ "WEBDRIVER_WINDOW": {"pixel_density": 1},
+ "SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT": 30000,
+ "SCREENSHOT_PLAYWRIGHT_WAIT_EVENT": "networkidle",
+ "SCREENSHOT_SELENIUM_HEADSTART": 0,
+ "SCREENSHOT_SELENIUM_ANIMATION_WAIT": 0,
+ "SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
+ "SCREENSHOT_TILED_ENABLED": False,
+ "SCREENSHOT_LOCATE_WAIT": 10,
+ "SCREENSHOT_LOAD_WAIT": 60,
+ "SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE": 10,
+ "SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE": 10,
+ }
+
+ mock_playwright_instance = MagicMock()
+ mock_browser = MagicMock()
+ mock_context = MagicMock()
+ mock_page = MagicMock()
+ mock_element = MagicMock()
+
+ mock_sync_playwright.return_value.__enter__.return_value = (
+ mock_playwright_instance
+ )
+ mock_playwright_instance.chromium.launch.return_value = mock_browser
+ mock_browser.new_context.return_value = mock_context
+ mock_context.new_page.return_value = mock_page
+ mock_page.locator.return_value = mock_element
+
+ timeout = PlaywrightTimeout()
+ mock_page.wait_for_function.side_effect = timeout
+
+ with patch.object(WebDriverPlaywright, "auth", return_value=mock_context):
+ driver = WebDriverPlaywright("chrome")
+ with pytest.raises(PlaywrightTimeout) as exc_info:
+ driver.get_screenshot("http://example.com", "test-element", mock_user)
+
+ assert exc_info.value is timeout
+ mock_logger.warning.assert_any_call(
+ "Timed out waiting for charts to load at url %s",
+ "http://example.com",
+ )
+
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver.sync_playwright")
@patch("superset.utils.webdriver.logger")