You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2026/09/10 16:17:39 UTC
(superset) 01/02: fix(dashboard): make screenshot task status reliable
This is an automated email from the ASF dual-hosted git repository.
geido pushed a commit to branch geido/screenshot-task-status-signalling
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 802dd53880ae242d0cadba641baf81863b93dfb7
Author: Diego Pucci <di...@gmail.com>
AuthorDate: Thu Sep 10 16:54:31 2026 +0300
fix(dashboard): make screenshot task status reliable
---
superset/dashboards/api.py | 53 +++++---
superset/tasks/thumbnails.py | 6 +-
superset/utils/screenshot_utils.py | 64 ++++++----
superset/utils/screenshots.py | 49 +++++++-
superset/utils/webdriver.py | 102 +++++++++++----
tests/integration_tests/dashboards/api_tests.py | 140 ++++++++++++++++++++-
tests/unit_tests/tasks/test_thumbnails.py | 41 ++++++
tests/unit_tests/utils/screenshot_test.py | 43 +++++++
.../unit_tests/utils/test_screenshot_cache_fix.py | 43 +++++++
tests/unit_tests/utils/test_screenshot_utils.py | 48 +++++++
tests/unit_tests/utils/webdriver_test.py | 78 ++++++++++++
11 files changed, 591 insertions(+), 76 deletions(-)
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 650df5e442f..7e0b4dd1427 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -1969,9 +1969,9 @@ class DashboardRestApi(
image_url = get_url_path(
"DashboardRestApi.screenshot", pk=dashboard.id, digest=cache_key
)
- cache_payload = (
- screenshot_obj.get_from_cache_key(cache_key) or ScreenshotCachePayload()
- )
+ cached_payload = screenshot_obj.get_from_cache_key(cache_key)
+ cache_payload = cached_payload or ScreenshotCachePayload()
+ cache_scope = f"dashboard:{dashboard.id}"
def build_response(status_code: int) -> WerkzeugResponse:
return self.response(
@@ -1983,26 +1983,37 @@ class DashboardRestApi(
task_status=cache_payload.get_status(),
)
- if cache_payload.should_trigger_task(
- force, expected_scope=f"dashboard:{dashboard.id}"
+ if cached_payload is None or cache_payload.should_enqueue_task(
+ force, expected_scope=cache_scope
):
logger.info("Triggering screenshot ASYNC")
- cache_dashboard_screenshot.delay(
- username=get_current_user(),
- guest_token=(
- g.user.guest_token
- if get_current_user() and isinstance(g.user, GuestUser)
- else None
- ),
- dashboard_id=dashboard.id,
- dashboard_url=dashboard_url,
- thumb_size=thumb_size,
- window_size=window_size,
- cache_key=cache_key,
- force=force,
- )
+ cache_payload.pending()
+ cache_payload.set_scope(cache_scope)
+ screenshot_obj.cache.set(cache_key, cache_payload.to_dict())
+ try:
+ cache_dashboard_screenshot.delay(
+ username=get_current_user(),
+ guest_token=(
+ g.user.guest_token
+ if get_current_user() and isinstance(g.user, GuestUser)
+ else None
+ ),
+ dashboard_id=dashboard.id,
+ dashboard_url=dashboard_url,
+ thumb_size=thumb_size,
+ window_size=window_size,
+ cache_key=cache_key,
+ # The API has already invalidated the prior artifact by
+ # persisting PENDING. Avoid making duplicate queued tasks
+ # recompute after another worker has completed the request.
+ force=False,
+ )
+ except Exception: # pylint: disable=broad-except
+ cache_payload.error()
+ screenshot_obj.cache.set(cache_key, cache_payload.to_dict())
+ raise
return build_response(202)
- return build_response(200)
+ return build_response(202 if cache_payload.is_in_progress() else 200)
@expose("/<pk>/screenshot/<digest>/", methods=("GET",))
@validate_feature_flags(["THUMBNAILS", "ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS"])
@@ -2070,6 +2081,8 @@ class DashboardRestApi(
# its image under a different, merely-accessible `pk`.
if cache_payload.get_scope() != f"dashboard:{dashboard.id}":
return self.response_404()
+ if not cache_payload.is_updated():
+ return self.response_404()
try:
image = cache_payload.get_image()
except ScreenshotImageNotAvailableException:
diff --git a/superset/tasks/thumbnails.py b/superset/tasks/thumbnails.py
index cd03cabd53e..7e1b9ccaa5d 100644
--- a/superset/tasks/thumbnails.py
+++ b/superset/tasks/thumbnails.py
@@ -150,7 +150,11 @@ def cache_dashboard_screenshot( # pylint: disable=too-many-arguments
current_user = security_manager.find_user(exec_username)
with override_user(current_user):
- screenshot = DashboardScreenshot(dashboard_url, dashboard.digest)
+ screenshot = DashboardScreenshot(
+ dashboard_url,
+ dashboard.digest,
+ require_complete_capture=True,
+ )
screenshot.cache_scope = f"dashboard:{dashboard.id}"
screenshot.compute_and_cache(
user=current_user,
diff --git a/superset/utils/screenshot_utils.py b/superset/utils/screenshot_utils.py
index b48814b2403..b513a383278 100644
--- a/superset/utils/screenshot_utils.py
+++ b/superset/utils/screenshot_utils.py
@@ -487,9 +487,15 @@ REPORT_ALL_CHART_HOLDERS_READY_JS = (
f"() => {{ {UNREADY_ALL_CHART_HOLDERS_JS_BODY} "
"return holders.length > 0 && unready.length === 0; }"
)
+DASHBOARD_ALL_CHART_HOLDERS_READY_JS = (
+ "() => { if (document.querySelector('.dashboard-grid') === null) "
+ f"return false; {UNREADY_ALL_CHART_HOLDERS_JS_BODY} "
+ "return unready.length === 0; }"
+)
CHART_HOLDERS_MOUNTED_JS = (
f"() => document.querySelectorAll('{CHART_HOLDER_SELECTOR}').length > 0"
)
+DASHBOARD_LAYOUT_READY_JS = "() => document.querySelector('.dashboard-grid') !== null"
FIND_UNREADY_CHART_HOLDERS_JS = (
f"() => {{ {UNREADY_CHART_HOLDERS_JS_BODY} return unready; }}"
)
@@ -719,6 +725,7 @@ def take_tiled_screenshot( # noqa: C901
report_execution_context: ReportExecutionContext | None = None,
url: str | None = None,
screenshot_started_at: float | None = None,
+ require_complete_capture: bool = False,
) -> bytes | None:
"""
Take a tiled screenshot of a large dashboard by scrolling and capturing sections.
@@ -741,6 +748,10 @@ def take_tiled_screenshot( # noqa: C901
task budget -- the same clock _wait_for_charts_ready uses.
Ignored when a report_execution_context provides its own
deadline; falls back to "now" when omitted.
+ require_complete_capture: Whether an API screenshot request should use
+ fail-closed readiness and blank-capture behavior. The rendered DOM,
+ rather than the model's total slice count, defines the active tab's
+ capture contents.
Returns:
Combined screenshot bytes or None if failed
@@ -753,6 +764,12 @@ def take_tiled_screenshot( # noqa: C901
"""
if report_execution_context:
log_context = report_execution_context.log_context
+ expected_chart_count = (
+ report_execution_context.expected_chart_count
+ if report_execution_context
+ else None
+ )
+ strict_capture = report_execution_context is not None or require_complete_capture
context_suffix = f" [{log_context}]" if log_context else ""
# Set right before re-raising the per-tile readiness timeout below, and
# checked in the except block at the bottom of this function.
@@ -820,14 +837,25 @@ def take_tiled_screenshot( # noqa: C901
* 1000
)
- if report_execution_context:
+ if strict_capture:
mount_wait = _timeout_seconds(
"chart_holder_mount",
- reserve_seconds=report_execution_context.readiness_reserve_seconds,
+ requested_seconds=(
+ None if report_execution_context else float(load_wait)
+ ),
+ reserve_seconds=(
+ report_execution_context.readiness_reserve_seconds
+ if report_execution_context
+ else 0.0
+ ),
)
try:
page.wait_for_function(
- CHART_HOLDERS_MOUNTED_JS,
+ (
+ CHART_HOLDERS_MOUNTED_JS
+ if report_execution_context
+ else DASHBOARD_LAYOUT_READY_JS
+ ),
timeout=mount_wait * 1000,
)
except PlaywrightTimeout:
@@ -840,7 +868,7 @@ def take_tiled_screenshot( # noqa: C901
"terminal_reason=zero_holders_timeout states=%s; "
"aborting before dimensions, capture, or delivery",
url,
- report_execution_context.expected_chart_count,
+ expected_chart_count,
len(holder_states),
elapsed,
f"{remaining:.2f}" if remaining is not None else None,
@@ -988,11 +1016,7 @@ def take_tiled_screenshot( # noqa: C901
"terminal_reason=readiness_timeout unready_holders=%s "
"states=%s; aborting before capture or delivery",
url,
- (
- report_execution_context.expected_chart_count
- if report_execution_context
- else None
- ),
+ expected_chart_count,
len(holder_states),
ready_holders,
i + 1,
@@ -1134,17 +1158,13 @@ def take_tiled_screenshot( # noqa: C901
total_chart_holders = 0
contentful_chart_holders = 0
- if (
- total_chart_holders == 0
- and report_execution_context
- and report_execution_context.expected_chart_count
- ):
+ if total_chart_holders == 0 and strict_capture and expected_chart_count:
logger.warning(
"report_capture_no_chart_holders tile=%s/%s "
"expected_holders=%s holder_count_failed=%s%s",
i + 1,
num_tiles,
- report_execution_context.expected_chart_count,
+ expected_chart_count,
holder_count_failed,
context_suffix,
)
@@ -1246,7 +1266,7 @@ def take_tiled_screenshot( # noqa: C901
context_suffix,
)
if capture_attempt == TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS:
- if report_execution_context:
+ if strict_capture:
# Exhausted retries must never turn a rejected report
# tile into accepted content. Only thumbnails may
# retain a blank capture below.
@@ -1351,11 +1371,7 @@ def take_tiled_screenshot( # noqa: C901
"elapsed_seconds=%.2f "
"remaining_seconds=%s%s",
url,
- (
- report_execution_context.expected_chart_count
- if report_execution_context
- else None
- ),
+ expected_chart_count,
len(holder_states),
sum(holder.get("state") in ready_states for holder in holder_states),
sum(holder.get("agGridWaitObserved") is True for holder in holder_states),
@@ -1367,11 +1383,11 @@ def take_tiled_screenshot( # noqa: C901
logger.info("Combining screenshot tiles...%s", context_suffix)
combined_screenshot = combine_screenshot_tiles(
screenshot_tiles,
- allow_partial_fallback=report_execution_context is None,
+ allow_partial_fallback=not strict_capture,
log_context=log_context,
)
- if report_execution_context and contentful_tiles_captured:
+ if strict_capture and contentful_tiles_captured:
combined_blankness = get_screenshot_blankness_metrics(combined_screenshot)
logger.info(
"report_capture_validation capture=combined "
@@ -1418,7 +1434,7 @@ def take_tiled_screenshot( # noqa: C901
# Preserve the explicit blank-capture or timeout reason for report execution
# history instead of degrading it to an anonymous None screenshot.
logger.exception("Tiled screenshot capture rejected%s", context_suffix)
- if report_execution_context:
+ if strict_capture:
raise
return None
except SoftTimeLimitExceeded:
diff --git a/superset/utils/screenshots.py b/superset/utils/screenshots.py
index de3635e4a4a..292d8af8d67 100644
--- a/superset/utils/screenshots.py
+++ b/superset/utils/screenshots.py
@@ -106,13 +106,15 @@ class ScreenshotCachePayload:
def __init__(
self,
image: bytes | None = None,
- status: StatusValues = StatusValues.PENDING,
+ status: StatusValues | None = None,
timestamp: str = "",
scope: str | None = None,
):
self._image = image
self._timestamp = timestamp or datetime.now().isoformat()
- self.status = StatusValues.UPDATED if image else status
+ self.status = status or (
+ StatusValues.UPDATED if image else StatusValues.PENDING
+ )
self._scope = scope
@classmethod
@@ -191,11 +193,38 @@ class ScreenshotCachePayload:
def is_computing_stale(self) -> bool:
"""Check if a COMPUTING status is stale (task likely failed or stuck)."""
+ return self.is_in_progress_stale()
+
+ def is_in_progress_stale(self) -> bool:
+ """Check if a pending or computing request has exceeded its TTL."""
computing_ttl = app.config["THUMBNAIL_COMPUTING_CACHE_TTL"]
return (
datetime.now() - datetime.fromisoformat(self.get_timestamp())
).total_seconds() >= computing_ttl
+ def is_in_progress(self) -> bool:
+ """Return whether screenshot computation has not reached a terminal state."""
+ return self.status in (StatusValues.PENDING, StatusValues.COMPUTING)
+
+ def is_updated(self) -> bool:
+ """Return whether the cached screenshot completed successfully."""
+ return self.status == StatusValues.UPDATED
+
+ def should_enqueue_task(
+ self, force: bool = False, expected_scope: str | None = None
+ ) -> bool:
+ """Check whether an API caller should enqueue screenshot computation.
+
+ A fresh pending or computing payload represents an accepted in-flight
+ request, so additional polling and force requests are coalesced. Stale
+ in-flight payloads remain retryable through the computing cache TTL.
+ """
+ if expected_scope is not None and self._scope != expected_scope:
+ return True
+ if self.is_in_progress():
+ return self.is_in_progress_stale()
+ return self.should_trigger_task(force, expected_scope)
+
def should_trigger_task(
self, force: bool = False, expected_scope: str | None = None
) -> bool:
@@ -241,10 +270,16 @@ class BaseScreenshot:
# every dashboard and chart.
cache_scope: str | None = None
- def __init__(self, url: str, digest: str | None):
+ def __init__(
+ self,
+ url: str,
+ digest: str | None,
+ require_complete_capture: bool = False,
+ ):
self.digest = digest
self.url = url
self.screenshot = None
+ self.require_complete_capture = require_complete_capture
def driver(
self,
@@ -268,6 +303,7 @@ class BaseScreenshot:
user,
log_context=log_context,
report_execution_context=report_execution_context,
+ require_complete_capture=self.require_complete_capture,
)
return self.screenshot
@@ -504,6 +540,7 @@ class DashboardScreenshot(BaseScreenshot):
digest: str | None,
window_size: WindowSize | None = None,
thumb_size: WindowSize | None = None,
+ require_complete_capture: bool = False,
):
# per the element above, dashboard screenshots
# should always capture in standalone
@@ -511,7 +548,11 @@ class DashboardScreenshot(BaseScreenshot):
url,
standalone=DashboardStandaloneMode.REPORT.value,
)
- super().__init__(url, digest)
+ super().__init__(
+ url,
+ digest,
+ require_complete_capture=require_complete_capture,
+ )
self.window_size = window_size or DEFAULT_DASHBOARD_WINDOW_SIZE
self.thumb_size = thumb_size or DEFAULT_DASHBOARD_THUMBNAIL_SIZE
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index 9a5f32e75a8..853f2af1aa8 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -35,6 +35,8 @@ from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
CHART_CONTAINER_STATE_JS,
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
+ DASHBOARD_LAYOUT_READY_JS,
EXPAND_SCROLLABLE_CONTENT_JS,
EXPAND_SCROLLABLE_CONTENT_MAX_WAIT_SECONDS,
FIND_ALL_UNREADY_CHART_HOLDERS_JS,
@@ -169,6 +171,7 @@ class WebDriverProxy(ABC):
user: User | None = None,
log_context: str | None = None,
report_execution_context: ReportExecutionContext | None = None,
+ require_complete_capture: bool = False,
) -> bytes | None:
"""
Run webdriver and return a screenshot
@@ -257,8 +260,9 @@ class WebDriverPlaywright(WebDriverProxy):
element_name: str,
log_context: str | None,
report_execution_context: ReportExecutionContext | None,
+ validate_rendered_content: bool = False,
) -> bytes:
- """Capture a standard screenshot and reject blank report output."""
+ """Capture a standard screenshot and reject blank rendered output."""
context_suffix = f" [{log_context}]" if log_context else ""
for attempt in range(1, TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS + 1):
@@ -280,7 +284,7 @@ class WebDriverPlaywright(WebDriverProxy):
timeout_seconds=capture_timeout,
)
capture_elapsed = time.monotonic() - capture_started_at
- if report_execution_context is None:
+ if report_execution_context is None and not validate_rendered_content:
return image
blankness = get_screenshot_blankness_metrics(image)
@@ -343,10 +347,16 @@ class WebDriverPlaywright(WebDriverProxy):
"Chromium returned a blank standard screenshot "
f"after {attempt} attempts"
)
- repaint_timeout = report_execution_context.deadline.timeout_seconds(
- "screenshot_repaint",
- requested_seconds=5.0,
- reserve_seconds=report_execution_context.post_capture_reserve_seconds,
+ repaint_timeout = (
+ report_execution_context.deadline.timeout_seconds(
+ "screenshot_repaint",
+ requested_seconds=5.0,
+ reserve_seconds=(
+ report_execution_context.post_capture_reserve_seconds
+ ),
+ )
+ if report_execution_context
+ else 5.0
)
try:
page.bring_to_front()
@@ -421,11 +431,12 @@ class WebDriverPlaywright(WebDriverProxy):
log_context: str | None = None,
screenshot_started_at: float | None = None,
report_execution_context: ReportExecutionContext | None = None,
+ require_complete_capture: bool = False,
) -> None:
"""
- Wait for every viewport-visible chart holder to reach a terminal state
- (rendered, or errored/empty) before taking a standard (non-tiled)
- screenshot.
+ Wait for the chart holders included by the capture policy to reach a
+ terminal state (rendered, or errored/empty) before taking a standard
+ (non-tiled) screenshot.
Uses the same positive readiness predicate as the tiled screenshot
path (see `take_tiled_screenshot` in screenshot_utils.py, #42119)
@@ -436,11 +447,12 @@ class WebDriverPlaywright(WebDriverProxy):
check immediately, producing a silently blank screenshot with no
timeout, warning, or error anywhere.
- Scoped to viewport-intersecting chart holders only, same as the tiled
- path: this method's caller never resizes the browser viewport to the
- full dashboard height before capturing, so DashboardVirtualization
- placeholders below the fold haven't mounted anything real yet by
- design and must not block this wait.
+ Thumbnail callers retain viewport-scoped readiness. Scheduled reports
+ and API screenshot requests force all virtualized rows into view and
+ require every holder in the rendered dashboard state to settle because
+ their full-page captures include content below the browser viewport.
+ The API predicate waits for dashboard layout hydration before allowing
+ an active tab with no chart holders to pass.
"""
task_budget: float | None
remaining_budget: float | None
@@ -459,6 +471,9 @@ class WebDriverPlaywright(WebDriverProxy):
if report_execution_context
else None
)
+ strict_dashboard_capture = element_name == "standalone" and (
+ report_execution_context is not None or require_complete_capture
+ )
initial_mounted_holders = len(initial_chart_holder_states)
initial_ready_holders = sum(
holder.get("state") in ready_states
@@ -557,7 +572,7 @@ class WebDriverPlaywright(WebDriverProxy):
)
if element_name == "chart-container":
readiness_predicate = CHART_CONTAINER_READY_JS
- elif report_execution_context:
+ elif strict_dashboard_capture:
# This non-tiled path captures the whole element in one shot
# (`_get_screenshot` uses `full_page=True` / `element.screenshot()`),
# so below-the-fold holders end up in the image. Force every
@@ -570,11 +585,14 @@ class WebDriverPlaywright(WebDriverProxy):
# viewport-scoped predicate because it scrolls each region into view
# before capturing it.
page.evaluate(FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS)
- readiness_predicate = REPORT_ALL_CHART_HOLDERS_READY_JS
+ readiness_predicate = (
+ REPORT_ALL_CHART_HOLDERS_READY_JS
+ if report_execution_context
+ else DASHBOARD_ALL_CHART_HOLDERS_READY_JS
+ )
else:
- # Preserve the thumbnail behavior introduced by #42253. The
- # stricter zero-holder gate is report-specific because an empty
- # dashboard thumbnail is still a valid cache artifact.
+ # Preserve thumbnail behavior: an empty dashboard remains a valid
+ # artifact for callers that do not request fail-closed capture.
readiness_predicate = CHART_HOLDERS_READY_JS
try:
page.wait_for_function(
@@ -625,7 +643,7 @@ class WebDriverPlaywright(WebDriverProxy):
# Surface them explicitly using the non-viewport-scoped scan.
below_fold_unready = (
page.evaluate(FIND_ALL_UNREADY_CHART_HOLDERS_JS)
- if report_execution_context
+ if strict_dashboard_capture
else unready_chart_holders
)
deadline_elapsed = deadline.elapsed_seconds if deadline else elapsed
@@ -706,6 +724,7 @@ class WebDriverPlaywright(WebDriverProxy):
user: User | None = None,
log_context: str | None = None,
report_execution_context: ReportExecutionContext | None = None,
+ require_complete_capture: bool = False,
) -> bytes | None:
screenshot_started_at = time.monotonic()
if report_execution_context:
@@ -714,6 +733,20 @@ class WebDriverPlaywright(WebDriverProxy):
"browser_setup",
reserve_seconds=report_execution_context.readiness_reserve_seconds,
)
+ expected_chart_count = (
+ report_execution_context.expected_chart_count
+ if report_execution_context
+ else None
+ )
+ api_dashboard_capture = (
+ element_name == "standalone" and require_complete_capture
+ )
+ strict_dashboard_capture = element_name == "standalone" and (
+ report_execution_context is not None or require_complete_capture
+ )
+ validate_rendered_content = (
+ report_execution_context is not None or strict_dashboard_capture
+ )
context_suffix = f" [{log_context}]" if log_context else ""
if not PLAYWRIGHT_AVAILABLE:
raise RuntimeError(
@@ -903,17 +936,31 @@ class WebDriverPlaywright(WebDriverProxy):
tiled_enabled = app.config.get("SCREENSHOT_TILED_ENABLED", False)
if tiled_enabled:
+ if api_dashboard_capture:
+ # A dashboard's total slice relationship includes charts
+ # on inactive tabs. Wait for the active layout to hydrate,
+ # then use its holder count for tiling and readiness.
+ page.wait_for_function(
+ DASHBOARD_LAYOUT_READY_JS,
+ timeout=self._screenshot_load_wait * 1000,
+ )
mounted_chart_count = page.evaluate(
'document.querySelectorAll(".chart-container").length'
)
- expected_chart_count = (
- report_execution_context.expected_chart_count
- if report_execution_context
- else None
+ active_holder_states = (
+ page.evaluate(FIND_CHART_HOLDER_STATES_JS)
+ if api_dashboard_capture
+ else []
+ )
+ active_holder_count = (
+ len(active_holder_states)
+ if isinstance(active_holder_states, list)
+ else 0
)
chart_count = max(
mounted_chart_count,
expected_chart_count or 0,
+ active_holder_count,
)
dashboard_height = page.evaluate(
f"""() => {{
@@ -974,7 +1021,7 @@ class WebDriverPlaywright(WebDriverProxy):
use_tiled = likely_large_dashboard and (
height_unknown
or dashboard_height > tile_height
- or report_execution_context is not None
+ or strict_dashboard_capture
)
if use_tiled:
@@ -1003,6 +1050,7 @@ class WebDriverPlaywright(WebDriverProxy):
report_execution_context=report_execution_context,
url=url,
screenshot_started_at=screenshot_started_at,
+ require_complete_capture=require_complete_capture,
)
if not img:
# _get_screenshot() has no wait/readiness logic at
@@ -1051,6 +1099,7 @@ class WebDriverPlaywright(WebDriverProxy):
log_context=log_context,
screenshot_started_at=screenshot_started_at,
report_execution_context=report_execution_context,
+ require_complete_capture=require_complete_capture,
)
# Re-run now that readiness has confirmed every chart
# actually rendered: a grid whose GridReady hadn't
@@ -1089,6 +1138,7 @@ class WebDriverPlaywright(WebDriverProxy):
element_name,
log_context,
report_execution_context,
+ validate_rendered_content=validate_rendered_content,
)
logger.debug(
"Screenshot result: %d bytes for url: %s%s",
@@ -1114,6 +1164,7 @@ class WebDriverPlaywright(WebDriverProxy):
log_context=log_context,
screenshot_started_at=screenshot_started_at,
report_execution_context=report_execution_context,
+ require_complete_capture=require_complete_capture,
)
# Re-run now that readiness has confirmed every chart
# actually rendered: a grid whose GridReady hadn't fired
@@ -1152,6 +1203,7 @@ class WebDriverPlaywright(WebDriverProxy):
element_name,
log_context,
report_execution_context,
+ validate_rendered_content=validate_rendered_content,
)
logger.debug(
"Screenshot result: %d bytes for url: %s%s",
diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py
index ec1d02a82cf..3f48e70e615 100644
--- a/tests/integration_tests/dashboards/api_tests.py
+++ b/tests/integration_tests/dashboards/api_tests.py
@@ -41,7 +41,7 @@ from superset.subjects.models import Subject
from superset.subjects.types import SubjectType
from superset.tags.models import Tag, TaggedObject, TagType, ObjectType
from superset.utils.core import backend, override_user
-from superset.utils.screenshots import ScreenshotCachePayload
+from superset.utils.screenshots import ScreenshotCachePayload, StatusValues
from superset.utils import json
from tests.conftest import with_config
@@ -4158,10 +4158,12 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
security_manager.add_permission_role(gamma_role, write_tags_perm)
security_manager.add_permission_role(gamma_role, tag_dashboards_perm)
- def _cache_screenshot(self, dashboard_id, payload=None):
+ def _cache_screenshot(self, dashboard_id, payload=None, force=False):
if payload is None:
payload = {"dataMask": {}, "activeTabs": [], "anchor": "", "urlParams": []}
uri = f"/api/v1/dashboard/{dashboard_id}/cache_dashboard_screenshot/"
+ if force:
+ uri = f"{uri}?q={rison.dumps({'force': True})}"
return self.client.post(uri, json=payload)
def _get_screenshot(self, dashboard_id, cache_key, download_format):
@@ -4180,6 +4182,118 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
response = self._cache_screenshot(dashboard.id)
assert response.status_code == 202
+ @with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
+ @pytest.mark.usefixtures("create_dashboard_with_tag")
+ @patch("superset.dashboards.api.cache_dashboard_screenshot")
+ @patch("superset.dashboards.api.DashboardScreenshot.cache.set")
+ @patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
+ def test_force_screenshot_invalidates_cached_image_before_enqueue(
+ self, mock_get_from_cache_key, mock_cache_set, mock_cache_task
+ ):
+ self.login(ADMIN_USERNAME)
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.dashboard_title == "dash with tag")
+ .first()
+ )
+ mock_get_from_cache_key.return_value = ScreenshotCachePayload(
+ image=b"old image", scope=f"dashboard:{dashboard.id}"
+ )
+
+ response = self._cache_screenshot(dashboard.id, force=True)
+
+ assert response.status_code == 202
+ assert response.json["task_status"] == "Pending"
+ pending_payload = mock_cache_set.call_args.args[1]
+ assert pending_payload["status"] == "Pending"
+ assert pending_payload["image"] is None
+ assert pending_payload["scope"] == f"dashboard:{dashboard.id}"
+ mock_cache_task.delay.assert_called_once()
+ assert mock_cache_task.delay.call_args.kwargs["force"] is False
+
+ @with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
+ @pytest.mark.usefixtures("create_dashboard_with_tag")
+ @patch("superset.dashboards.api.cache_dashboard_screenshot")
+ @patch("superset.dashboards.api.DashboardScreenshot.cache.set")
+ @patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
+ def test_scope_mismatch_is_replaced_with_scoped_pending_payload(
+ self, mock_get_from_cache_key, mock_cache_set, mock_cache_task
+ ):
+ self.login(ADMIN_USERNAME)
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.dashboard_title == "dash with tag")
+ .first()
+ )
+ mock_get_from_cache_key.return_value = ScreenshotCachePayload(
+ image=b"old image", scope="dashboard:999999"
+ )
+
+ response = self._cache_screenshot(dashboard.id)
+
+ assert response.status_code == 202
+ assert response.json["task_status"] == "Pending"
+ pending_payload = mock_cache_set.call_args.args[1]
+ assert pending_payload["status"] == "Pending"
+ assert pending_payload["image"] is None
+ assert pending_payload["scope"] == f"dashboard:{dashboard.id}"
+ mock_cache_task.delay.assert_called_once()
+
+ @with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
+ @pytest.mark.usefixtures("create_dashboard_with_tag")
+ @patch("superset.dashboards.api.cache_dashboard_screenshot")
+ @patch("superset.dashboards.api.DashboardScreenshot.cache.set")
+ @patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
+ def test_fresh_in_progress_screenshot_request_is_coalesced(
+ self, mock_get_from_cache_key, mock_cache_set, mock_cache_task
+ ):
+ self.login(ADMIN_USERNAME)
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.dashboard_title == "dash with tag")
+ .first()
+ )
+ for status in (StatusValues.PENDING, StatusValues.COMPUTING):
+ mock_get_from_cache_key.return_value = ScreenshotCachePayload(
+ status=status, scope=f"dashboard:{dashboard.id}"
+ )
+
+ response = self._cache_screenshot(dashboard.id, force=True)
+
+ assert response.status_code == 202
+ assert response.json["task_status"] == status.value
+ mock_cache_set.assert_not_called()
+ mock_cache_task.delay.assert_not_called()
+
+ @with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
+ @pytest.mark.usefixtures("create_dashboard_with_tag")
+ @patch("superset.dashboards.api.cache_dashboard_screenshot")
+ @patch("superset.dashboards.api.DashboardScreenshot.cache.set")
+ @patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
+ def test_enqueue_failure_persists_terminal_error(
+ self, mock_get_from_cache_key, mock_cache_set, mock_cache_task
+ ):
+ self.login(ADMIN_USERNAME)
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.dashboard_title == "dash with tag")
+ .first()
+ )
+ mock_get_from_cache_key.return_value = None
+ mock_cache_task.delay.side_effect = RuntimeError("broker unavailable")
+
+ response = self._cache_screenshot(dashboard.id)
+
+ assert response.status_code == 500
+ assert mock_cache_set.call_count == 2
+ pending_payload, error_payload = (
+ call.args[1] for call in mock_cache_set.call_args_list
+ )
+ assert pending_payload["status"] == "Pending"
+ assert error_payload["status"] == "Error"
+ assert error_payload["image"] is None
+ assert error_payload["scope"] == f"dashboard:{dashboard.id}"
+
@with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
@pytest.mark.usefixtures("create_dashboard_with_tag")
def test_cache_dashboard_screenshot_success_permalink_payload(self):
@@ -4351,6 +4465,28 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
response = self._get_screenshot(dashboard.id, cache_key, "pdf")
assert response.status_code == 404
+ @with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
+ @pytest.mark.usefixtures("create_dashboard_with_tag")
+ @patch("superset.dashboards.api.DashboardScreenshot.get_from_cache_key")
+ def test_screenshot_does_not_serve_image_from_non_updated_payload(
+ self, mock_get_from_cache_key
+ ):
+ self.login(ADMIN_USERNAME)
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter(Dashboard.dashboard_title == "dash with tag")
+ .first()
+ )
+ mock_get_from_cache_key.return_value = ScreenshotCachePayload(
+ image=b"old image",
+ status=StatusValues.COMPUTING,
+ scope=f"dashboard:{dashboard.id}",
+ )
+
+ response = self._get_screenshot(dashboard.id, "cache-key", "png")
+
+ assert response.status_code == 404
+
@with_feature_flags(THUMBNAILS=True, ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True)
def test_screenshot_dashboard_not_found(self):
self.login(ADMIN_USERNAME)
diff --git a/tests/unit_tests/tasks/test_thumbnails.py b/tests/unit_tests/tasks/test_thumbnails.py
index 8dee18149f6..f88663582fb 100644
--- a/tests/unit_tests/tasks/test_thumbnails.py
+++ b/tests/unit_tests/tasks/test_thumbnails.py
@@ -143,3 +143,44 @@ def test_cache_dashboard_thumbnail_resolves_cache_key_when_not_provided(
force=False,
cache_key="resolved_cache_key",
)
+
+
+@_apply_patches
+def test_cache_dashboard_screenshot_requires_complete_capture(
+ mock_screenshot_cls: MagicMock,
+ mock_get_url_path: MagicMock,
+ mock_override_user: MagicMock,
+ mock_security_manager: MagicMock,
+ mock_get_executor: MagicMock,
+ mock_dashboard: MagicMock,
+ mock_thumbnail_cache: None,
+) -> None:
+ """Direct API captures opt into fail-closed rendering."""
+ from superset.tasks.thumbnails import cache_dashboard_screenshot
+
+ mock_screenshot = _make_screenshot_mock()
+ mock_screenshot_cls.return_value = mock_screenshot
+ mock_get_executor.return_value = (None, "admin")
+ mock_security_manager.find_user.return_value = MagicMock()
+
+ with patch("superset.models.dashboard.Dashboard.get", return_value=mock_dashboard):
+ cache_dashboard_screenshot(
+ username="admin",
+ dashboard_id=1,
+ dashboard_url="/dashboard/p/test/",
+ force=False,
+ cache_key="test_cache_key",
+ )
+
+ mock_screenshot_cls.assert_called_once_with(
+ "/dashboard/p/test/",
+ "test_digest",
+ require_complete_capture=True,
+ )
+ mock_screenshot.compute_and_cache.assert_called_once_with(
+ user=ANY,
+ window_size=None,
+ thumb_size=None,
+ cache_key="test_cache_key",
+ force=False,
+ )
diff --git a/tests/unit_tests/utils/screenshot_test.py b/tests/unit_tests/utils/screenshot_test.py
index fe64f7d69a7..9f54793b118 100644
--- a/tests/unit_tests/utils/screenshot_test.py
+++ b/tests/unit_tests/utils/screenshot_test.py
@@ -26,8 +26,10 @@ from superset.utils.hashing import hash_from_dict
from superset.utils.screenshots import (
BaseScreenshot,
ChartScreenshot,
+ DashboardScreenshot,
ScreenshotCachePayload,
ScreenshotCachePayloadType,
+ StatusValues,
)
BASE_SCREENSHOT_PATH = "superset.utils.screenshots.BaseScreenshot"
@@ -77,6 +79,25 @@ def test_get_screenshot(mocker: MockerFixture, screenshot_obj):
assert screenshot_data == fake_bytes
+def test_dashboard_screenshot_threads_complete_capture_requirement(
+ mocker: MockerFixture, mock_user
+):
+ screenshot_obj = DashboardScreenshot(
+ "http://example.com",
+ "sample_digest",
+ require_complete_capture=True,
+ )
+ driver = mocker.patch(BASE_SCREENSHOT_PATH + ".driver")
+ driver.return_value.get_screenshot.return_value = FAKE_PNG_BYTES
+
+ screenshot_obj.get_screenshot(mock_user)
+
+ assert (
+ driver.return_value.get_screenshot.call_args.kwargs["require_complete_capture"]
+ is True
+ )
+
+
def test_get_cache_key(app_context, screenshot_obj):
"""Test get_cache_key method"""
expected_cache_key = hash_from_dict(
@@ -308,6 +329,28 @@ class TestScreenshotCachePayloadGetImage:
assert result1 is not result2
+@pytest.mark.parametrize("status", [StatusValues.COMPUTING, StatusValues.ERROR])
+def test_explicit_status_round_trips_with_an_existing_image(
+ status: StatusValues,
+) -> None:
+ payload = ScreenshotCachePayload(
+ image=FAKE_PNG_BYTES,
+ status=status,
+ scope="dashboard:5",
+ )
+
+ restored = ScreenshotCachePayload.from_dict(payload.to_dict())
+
+ assert restored.status == status
+ assert restored.get_status() == status.value
+
+
+def test_image_without_explicit_status_defaults_to_updated() -> None:
+ payload = ScreenshotCachePayload(image=FAKE_PNG_BYTES)
+
+ assert payload.status == StatusValues.UPDATED
+
+
class TestScreenshotCachePayloadScope:
"""
Cache entries are shared across every dashboard and chart in the same
diff --git a/tests/unit_tests/utils/test_screenshot_cache_fix.py b/tests/unit_tests/utils/test_screenshot_cache_fix.py
index 133c8c7e4d8..705dee1a8df 100644
--- a/tests/unit_tests/utils/test_screenshot_cache_fix.py
+++ b/tests/unit_tests/utils/test_screenshot_cache_fix.py
@@ -356,6 +356,49 @@ class TestShouldTriggerTask:
payload_computing = ScreenshotCachePayload(status=StatusValues.COMPUTING)
assert payload_computing.should_trigger_task(force=True) is True
+ @patch("superset.utils.screenshots.app")
+ def test_fresh_pending_request_is_not_enqueued_again(
+ self, mock_app: MagicMock
+ ) -> None:
+ mock_app.config = {"THUMBNAIL_COMPUTING_CACHE_TTL": 300}
+ payload = ScreenshotCachePayload(status=StatusValues.PENDING)
+
+ assert payload.should_enqueue_task(force=False) is False
+ assert payload.should_enqueue_task(force=True) is False
+
+ @patch("superset.utils.screenshots.app")
+ def test_stale_pending_request_is_enqueued_again(self, mock_app: MagicMock) -> None:
+ mock_app.config = {"THUMBNAIL_COMPUTING_CACHE_TTL": 300}
+ old_timestamp = (datetime.now() - timedelta(seconds=400)).isoformat()
+ payload = ScreenshotCachePayload(
+ status=StatusValues.PENDING, timestamp=old_timestamp
+ )
+
+ assert payload.should_enqueue_task(force=False) is True
+
+ @patch("superset.utils.screenshots.app")
+ def test_fresh_in_progress_scope_mismatch_is_enqueued_again(
+ self, mock_app: MagicMock
+ ) -> None:
+ mock_app.config = {"THUMBNAIL_COMPUTING_CACHE_TTL": 300}
+ payload = ScreenshotCachePayload(
+ status=StatusValues.COMPUTING,
+ scope="dashboard:other",
+ )
+
+ assert (
+ payload.should_enqueue_task(
+ force=False,
+ expected_scope="dashboard:expected",
+ )
+ is True
+ )
+
+ def test_force_enqueues_updated_payload(self) -> None:
+ payload = ScreenshotCachePayload(image=b"image_data")
+
+ assert payload.should_enqueue_task(force=True) is True
+
class TestIsComputingStale:
"""Test the is_computing_stale method."""
diff --git a/tests/unit_tests/utils/test_screenshot_utils.py b/tests/unit_tests/utils/test_screenshot_utils.py
index 34beab9ac58..8ba42a91ec7 100644
--- a/tests/unit_tests/utils/test_screenshot_utils.py
+++ b/tests/unit_tests/utils/test_screenshot_utils.py
@@ -477,6 +477,41 @@ class TestTakeTiledScreenshot:
assert mock_page.screenshot.call_count == 3
mock_combine.assert_not_called()
+ def test_repeated_blank_tiles_fail_closed_for_api_screenshots(self, mock_page):
+ element_info = {"height": 1000, "top": 0, "left": 0, "width": 800}
+
+ def evaluate(script, _arg=None):
+ if "scrollWidth" in script:
+ return element_info
+ if script == CONTENTFUL_CHART_HOLDERS_IN_CLIP_JS:
+ return {"total": 1, "contentful": 1}
+ if "requestAnimationFrame" in script or "window.scrollTo" in script:
+ return None
+ return [{"chartId": "7", "state": "rendered"}]
+
+ mock_page.evaluate.side_effect = evaluate
+ mock_page.screenshot.return_value = _png(800, 1000, "white")
+
+ with (
+ patch("superset.utils.screenshot_utils.logger"),
+ patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles"
+ ) as mock_combine,
+ pytest.raises(
+ ScreenshotBlankCaptureError,
+ match="blank tile 1/1 after 3 attempts",
+ ),
+ ):
+ take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=1000,
+ require_complete_capture=True,
+ )
+
+ assert mock_page.screenshot.call_count == 3
+ mock_combine.assert_not_called()
+
def test_blank_combined_image_is_advisory_after_contentful_tiles_pass(
self, mock_page
):
@@ -1104,6 +1139,7 @@ class TestTakeTiledScreenshot:
# Each call uses viewport-scoped JS and the load_wait timeout
mount_call, *tile_calls = mock_page.wait_for_function.call_args_list
assert "length > 0" in mount_call.args[0]
+ assert "arg" not in mount_call.kwargs
assert mount_call.kwargs["timeout"] == 30 * 1000
for call in tile_calls:
js = call[0][0]
@@ -1244,6 +1280,7 @@ class TestTakeTiledScreenshot:
"""
from superset.utils.screenshot_utils import (
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
FIND_CHART_HOLDER_STATES_JS,
FIND_UNREADY_CHART_HOLDERS_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1260,6 +1297,8 @@ class TestTakeTiledScreenshot:
assert "holder.className.match(/\\bdashboard-chart-id-(\\d+)\\b/)" in js
assert "holders.length > 0" not in CHART_HOLDERS_READY_JS
assert "holders.length > 0" in REPORT_CHART_HOLDERS_READY_JS
+ assert ".dashboard-grid" in DASHBOARD_ALL_CHART_HOLDERS_READY_JS
+ assert "holders.length > 0" not in DASHBOARD_ALL_CHART_HOLDERS_READY_JS
assert "rendered" in FIND_CHART_HOLDER_STATES_JS
assert "empty" in FIND_CHART_HOLDER_STATES_JS
@@ -1274,6 +1313,7 @@ class TestTakeTiledScreenshot:
from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
FIND_CHART_HOLDER_STATES_JS,
FIND_UNREADY_CHART_HOLDERS_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1282,6 +1322,7 @@ class TestTakeTiledScreenshot:
for js in (
CHART_CONTAINER_READY_JS,
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
FIND_CHART_HOLDER_STATES_JS,
FIND_UNREADY_CHART_HOLDERS_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1754,6 +1795,7 @@ def test_readiness_predicates_gate_on_unpainted_echarts_hosts() -> None:
key on the ECharts paint marker so a pre-paint canvas is never captured."""
from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
ECHARTS_UNPAINTED_HOST_SELECTOR,
FIND_CHART_HOLDER_STATES_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1763,6 +1805,7 @@ def test_readiness_predicates_gate_on_unpainted_echarts_hosts() -> None:
ECHARTS_UNPAINTED_HOST_SELECTOR == ".echarts-host:not(.echarts-render-finished)"
)
assert ECHARTS_UNPAINTED_HOST_SELECTOR in REPORT_CHART_HOLDERS_READY_JS
+ assert ECHARTS_UNPAINTED_HOST_SELECTOR in DASHBOARD_ALL_CHART_HOLDERS_READY_JS
assert ECHARTS_UNPAINTED_HOST_SELECTOR in CHART_CONTAINER_READY_JS
assert "mounted_unpainted" in FIND_CHART_HOLDER_STATES_JS
@@ -1775,6 +1818,7 @@ def test_readiness_predicates_gate_on_unpainted_ag_grid_hosts() -> None:
CHART_CONTAINER_READY_JS,
CHART_CONTAINER_STATE_JS,
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
FIND_CHART_HOLDER_STATES_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1783,6 +1827,7 @@ def test_readiness_predicates_gate_on_unpainted_ag_grid_hosts() -> None:
assert AG_GRID_HOST_SELECTOR == '[data-themed-ag-grid="true"]'
for predicate in (
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
REPORT_CHART_HOLDERS_READY_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
CHART_CONTAINER_READY_JS,
@@ -1791,6 +1836,7 @@ def test_readiness_predicates_gate_on_unpainted_ag_grid_hosts() -> None:
assert "_agGridFirstDataRendered !== true" in predicate
for predicate in (
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
REPORT_CHART_HOLDERS_READY_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
):
@@ -1806,6 +1852,7 @@ def test_ag_grid_no_rows_overlay_is_a_terminal_empty_state() -> None:
from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
EMPTY_SELECTOR,
REPORT_ALL_CHART_HOLDERS_READY_JS,
REPORT_CHART_HOLDERS_READY_JS,
@@ -1814,6 +1861,7 @@ def test_ag_grid_no_rows_overlay_is_a_terminal_empty_state() -> None:
assert ".ag-overlay-no-rows-wrapper:not(.ag-hidden)" in EMPTY_SELECTOR
for predicate in (
CHART_HOLDERS_READY_JS,
+ DASHBOARD_ALL_CHART_HOLDERS_READY_JS,
REPORT_CHART_HOLDERS_READY_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
CHART_CONTAINER_READY_JS,
diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py
index e031f847d58..093474a0229 100644
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -228,6 +228,27 @@ class TestStandardScreenshotValidation:
assert result == _png("white")
page.screenshot.assert_called_once_with(full_page=True)
+ def test_api_capture_rejects_blank_rendered_content(self):
+ page = MagicMock()
+ element = MagicMock()
+ page.screenshot.return_value = _png("white")
+ page.evaluate.return_value = True
+
+ with pytest.raises(
+ ScreenshotBlankCaptureError,
+ match="blank standard screenshot after 3 attempts",
+ ):
+ WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "cache_key=test",
+ None,
+ validate_rendered_content=True,
+ )
+
+ assert page.screenshot.call_count == 3
+
class TestWebDriverPlaywrightFallback:
"""Test WebDriverPlaywright fallback behavior when unavailable."""
@@ -1355,6 +1376,62 @@ class TestWebDriverPlaywrightChartReadiness:
assert "holders.length > 0" not in predicate
assert result == mock_page.screenshot.return_value
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver._browser_manager")
+ @patch("superset.utils.webdriver.app")
+ def test_api_screenshot_requires_hydrated_layout_and_ready_holders(
+ self,
+ mock_app,
+ mock_browser_manager,
+ ):
+ from superset.utils.webdriver import PlaywrightTimeout
+
+ mock_app.config = {**self._base_config}
+ mock_context, mock_page = self._make_pw_mocks(mock_browser_manager)
+ mock_page.evaluate.return_value = []
+ mock_page.wait_for_function.side_effect = PlaywrightTimeout("zero holders")
+
+ with patch.object(WebDriverPlaywright, "auth", return_value=mock_context):
+ with pytest.raises(PlaywrightTimeout):
+ WebDriverPlaywright("chrome").get_screenshot(
+ "http://example.com",
+ "standalone",
+ MagicMock(),
+ require_complete_capture=True,
+ )
+
+ predicate = mock_page.wait_for_function.call_args.args[0]
+ assert "document.querySelector('.dashboard-grid')" in predicate
+ assert "unready.length === 0" in predicate
+ assert "arg" not in mock_page.wait_for_function.call_args.kwargs
+ mock_page.screenshot.assert_not_called()
+
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver._browser_manager")
+ @patch("superset.utils.webdriver.app")
+ def test_api_screenshot_allows_hydrated_active_tab_without_charts(
+ self,
+ mock_app,
+ mock_browser_manager,
+ ):
+ mock_app.config = {**self._base_config}
+ mock_context, mock_page = self._make_pw_mocks(mock_browser_manager)
+ mock_page.evaluate.return_value = []
+ mock_page.screenshot.return_value = _png("black")
+
+ with patch.object(WebDriverPlaywright, "auth", return_value=mock_context):
+ result = WebDriverPlaywright("chrome").get_screenshot(
+ "http://example.com",
+ "standalone",
+ MagicMock(),
+ require_complete_capture=True,
+ )
+
+ predicate = mock_page.wait_for_function.call_args.args[0]
+ assert "document.querySelector('.dashboard-grid')" in predicate
+ assert "holders.length > 0" not in predicate
+ assert result == _png("black")
+
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver._browser_manager")
@patch("superset.utils.webdriver.app")
@@ -1993,6 +2070,7 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
report_execution_context=None,
url="http://example.com",
screenshot_started_at=ANY,
+ require_complete_capture=False,
)
# The only wait_for_timeout call should be the 0ms headstart; no global
# animation wait should be issued (handled per-tile by take_tiled_screenshot)