QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-05-29 20:29:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(reports): scope global spinner wait to non-tiled path only

Browse Source 
The pre-branch viewport-only wait_for_function ran regardless of which
screenshot path was taken. For tiled screenshots, per-tile checks in
take_tiled_screenshot already handle every viewport as we scroll, so
the pre-branch check is redundant there. For non-tiled screenshots,
element.screenshot() captures the full element including below-the-fold
content, so a global (all-spinners) check is correct.

Move the global querySelector('.loading').length === 0 check into each
non-tiled branch and remove it from the pre-branch position, so:
- non-tiled path: global check (waits for all charts including below fold)
- tiled path: no pre-check; per-tile viewport checks in screenshot_utils
  handle spinner detection tile by tile

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Elizabeth Thompson
2026-05-13 22:24:25 +00:00
committed by Amin Ghadersohi
parent 081e58b33d
commit 9f37704fb4
2 changed files with 41 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
superset/utils/webdriver.py
View File

@@ -295,33 +295,6 @@ class WebDriverPlaywright(WebDriverProxy):
url,
)
raise
try:
# charts took too long to load
logger.debug(
"Wait for loading element of charts to be gone at url: %s", url
)
page.wait_for_function(
"""() => {
const els = document.querySelectorAll('.loading');
for (const el of els) {
const r = el.getBoundingClientRect();
if (r.top < window.innerHeight && r.bottom > 0) {
return false;
}
}
return true;
}""",
timeout=self._screenshot_load_wait * 1000,
)
except PlaywrightTimeout:
logger.warning(
"Timed out waiting for charts to load at url %s "
"(SCREENSHOT_LOAD_WAIT=%ss)",
url,
self._screenshot_load_wait,
)
raise
selenium_animation_wait = app.config[
"SCREENSHOT_SELENIUM_ANIMATION_WAIT"
]
@@ -397,10 +370,50 @@ class WebDriverPlaywright(WebDriverProxy):
page, element, element_name
)
else:
# Standard screenshot captures the full element including
# below-the-fold content, so wait for all spinners globally.
try:
logger.debug(
"Wait for loading element of charts to be gone"
" at url: %s",
url,
)
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 "
"(SCREENSHOT_LOAD_WAIT=%ss)",
url,
self._screenshot_load_wait,
)
raise
img = WebDriverPlaywright._get_screenshot(
page, element, element_name
)
else:
# Standard screenshot captures the full element including
# below-the-fold content, so wait for all spinners globally.
try:
logger.debug(
"Wait for loading element of charts to be gone at url: %s",
url,
)
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 "
"(SCREENSHOT_LOAD_WAIT=%ss)",
url,
self._screenshot_load_wait,
)
raise
img = WebDriverPlaywright._get_screenshot(
page, element, element_name
)

11
tests/unit_tests/utils/webdriver_test.py
View File

@@ -684,16 +684,7 @@ class TestWebDriverPlaywrightErrorHandling:
driver.get_screenshot("http://example.com", "test-element", mock_user)
mock_page.wait_for_function.assert_called_once_with(
"""() => {
const els = document.querySelectorAll('.loading');
for (const el of els) {
const r = el.getBoundingClientRect();
if (r.top < window.innerHeight && r.bottom > 0) {
return false;
}
}
return true;
}""",
"() => document.querySelectorAll('.loading').length === 0",
timeout=60 * 1000,
)
# Guard against reintroducing the old snapshot-based approach