This is an automated email from the ASF dual-hosted git repository.
rebenitez1802 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 97329bb1cf1 fix(reports): detect perceptually blank screenshots
(#44034)
97329bb1cf1 is described below
commit 97329bb1cf17ffaa6fd59befa70284c72c51c79d
Author: Mafi <[email protected]>
AuthorDate: Wed Sep 9 23:18:20 2026 +1000
fix(reports): detect perceptually blank screenshots (#44034)
---
docs/admin_docs/configuration/alerts-reports.mdx | 10 +
superset/utils/screenshot_utils.py | 268 +++++++++++++++++++----
superset/utils/webdriver.py | 169 +++++++++++---
tests/unit_tests/commands/report/execute_test.py | 37 ++++
tests/unit_tests/utils/test_screenshot_utils.py | 261 +++++++++++++++++++---
tests/unit_tests/utils/webdriver_test.py | 205 ++++++++++++++---
6 files changed, 823 insertions(+), 127 deletions(-)
diff --git a/docs/admin_docs/configuration/alerts-reports.mdx
b/docs/admin_docs/configuration/alerts-reports.mdx
index a65dfea77f5..32cad433ca5 100644
--- a/docs/admin_docs/configuration/alerts-reports.mdx
+++ b/docs/admin_docs/configuration/alerts-reports.mdx
@@ -476,6 +476,16 @@ to determine which hosts are internal.
There are many reasons that reports might not be working. Try these steps to
check for specific issues.
+### Blank PDF or PNG captures
+
+Playwright report captures are checked for blank image content as well as chart
+readiness. Captures detected as blank where chart content is expected are
retried
+up to three total attempts, within the execution deadline. If they remain
blank,
+the report fails instead of delivering the blank or partial attachment. This
also
+applies to alerts that attach reports. Check worker logs for
+`ScreenshotBlankCaptureError` and `report_capture_validation` when
investigating
+these failures.
+
### Confirm feature flag is enabled and you have sufficient permissions
If you don't see "Alerts & Reports" under the _Manage_ section of the Settings
dropdown in the Superset UI, you need to enable the `ALERT_REPORTS` feature
flag (see above). Enable another feature flag and check to see that it took
effect, to verify that your config file is getting loaded.
diff --git a/superset/utils/screenshot_utils.py
b/superset/utils/screenshot_utils.py
index f1996561eb3..b48814b2403 100644
--- a/superset/utils/screenshot_utils.py
+++ b/superset/utils/screenshot_utils.py
@@ -19,11 +19,14 @@ from __future__ import annotations
import io
import logging
+import math
import time
+from dataclasses import dataclass
from typing import TYPE_CHECKING
from celery import current_task
-from PIL import Image, UnidentifiedImageError
+from celery.exceptions import SoftTimeLimitExceeded
+from PIL import Image, ImageChops, ImageStat, UnidentifiedImageError
from superset.utils.report_execution import (
ReportExecutionBudgetExceededError,
@@ -45,6 +48,13 @@ EXPAND_SCROLLABLE_CONTENT_MAX_WAIT_SECONDS = 5.0
TILED_SCREENSHOT_CAPTURE_TIMEOUT_SECONDS = 120
TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS = 3
TILED_SCREENSHOT_BLANK_DOMINANT_PIXEL_RATIO = 0.995
+SCREENSHOT_BLANK_MIN_LUMINANCE = 240
+SCREENSHOT_BLANK_MIN_MEAN_LUMINANCE = 250.0
+SCREENSHOT_BLANK_MAX_LUMINANCE_STDDEV = 8.0
+SCREENSHOT_BLANK_MAX_ENTROPY = 1.5
+SCREENSHOT_BLANK_MIN_EDGE_DIFFERENCE = 8
+SCREENSHOT_BLANK_MIN_STRUCTURAL_EDGE_RATIO = 0.02
+SCREENSHOT_BLANK_SAMPLE_SIZES = (256, 1024)
# Runtime task-budget policy shared with the approach introduced in #42118.
# Celery exposes the effective per-task hard/soft limits only on the running
@@ -130,32 +140,124 @@ class ScreenshotCaptureTimeoutError(RuntimeError):
"""Raised when Chromium repeatedly times out while capturing a tile."""
-def is_screenshot_nearly_uniform(screenshot: bytes) -> tuple[bool, float]:
- """Return whether one color occupies nearly all sampled screenshot
pixels."""
+class ScreenshotBlankCaptureError(RuntimeError):
+ """Raised when Chromium repeatedly returns a perceptually blank capture."""
+
+
+@dataclass(frozen=True)
+class ScreenshotBlanknessMetrics:
+ """Metrics used to decide whether a screenshot is perceptually blank."""
+
+ is_blank: bool
+ dominant_pixel_ratio: float
+ near_white_pixel_ratio: float
+ mean_luminance: float
+ luminance_stddev: float
+ entropy: float
+ structural_edge_ratio: float
+
+
+def get_screenshot_blankness_metrics(screenshot: bytes) ->
ScreenshotBlanknessMetrics:
+ """Measure exact-color and perceptual blankness on a sampled screenshot."""
try:
with Image.open(io.BytesIO(screenshot)) as image:
- sample = image.convert("RGB")
- sample.thumbnail((256, 256))
- colors = sample.getcolors(maxcolors=256)
- if not colors:
- return False, 0.0
- dominant_pixels = max(count for count, _color in colors)
- dominant_ratio = dominant_pixels / (sample.width * sample.height)
- return (
- dominant_ratio >= TILED_SCREENSHOT_BLANK_DOMINANT_PIXEL_RATIO,
- dominant_ratio,
+ sample_metrics: list[ScreenshotBlanknessMetrics] = []
+ for sample_size in SCREENSHOT_BLANK_SAMPLE_SIZES:
+ sample = image.convert("RGB")
+ sample.thumbnail((sample_size, sample_size))
+ pixel_count = sample.width * sample.height
+ colors = sample.getcolors(maxcolors=pixel_count) or []
+ dominant_pixels = max(
+ (count for count, _color in colors),
+ default=0,
+ )
+ dominant_ratio = dominant_pixels / pixel_count
+
+ grayscale = sample.convert("L")
+ histogram = grayscale.histogram()
+ near_white_ratio = (
+ sum(histogram[SCREENSHOT_BLANK_MIN_LUMINANCE:]) /
pixel_count
+ )
+ statistics = ImageStat.Stat(grayscale)
+ mean_luminance = float(statistics.mean[0])
+ luminance_stddev = float(statistics.stddev[0])
+ entropy = -sum(
+ (count / pixel_count) * math.log2(count / pixel_count)
+ for count in histogram
+ if count
+ )
+ horizontal_edges = ImageChops.difference(
+ grayscale, ImageChops.offset(grayscale, 1, 0)
+ )
+ vertical_edges = ImageChops.difference(
+ grayscale, ImageChops.offset(grayscale, 0, 1)
+ )
+ edge_histogram = ImageChops.lighter(
+ horizontal_edges, vertical_edges
+ ).histogram()
+ structural_edge_ratio = (
+ sum(edge_histogram[SCREENSHOT_BLANK_MIN_EDGE_DIFFERENCE:])
+ / pixel_count
+ )
+ low_information = (
+ luminance_stddev <= SCREENSHOT_BLANK_MAX_LUMINANCE_STDDEV
+ and entropy <= SCREENSHOT_BLANK_MAX_ENTROPY
+ and structural_edge_ratio
+ <= SCREENSHOT_BLANK_MIN_STRUCTURAL_EDGE_RATIO
+ )
+ perceptually_blank = low_information and (
+ mean_luminance >= SCREENSHOT_BLANK_MIN_MEAN_LUMINANCE
+ or dominant_ratio >=
TILED_SCREENSHOT_BLANK_DOMINANT_PIXEL_RATIO
+ )
+ sample_metrics.append(
+ ScreenshotBlanknessMetrics(
+ is_blank=perceptually_blank,
+ dominant_pixel_ratio=dominant_ratio,
+ near_white_pixel_ratio=near_white_ratio,
+ mean_luminance=mean_luminance,
+ luminance_stddev=luminance_stddev,
+ entropy=entropy,
+ structural_edge_ratio=structural_edge_ratio,
+ )
+ )
+
+ metrics = sample_metrics[-1]
+ return ScreenshotBlanknessMetrics(
+ is_blank=all(sample.is_blank for sample in sample_metrics),
+ dominant_pixel_ratio=metrics.dominant_pixel_ratio,
+ near_white_pixel_ratio=metrics.near_white_pixel_ratio,
+ mean_luminance=metrics.mean_luminance,
+ luminance_stddev=metrics.luminance_stddev,
+ entropy=metrics.entropy,
+ structural_edge_ratio=metrics.structural_edge_ratio,
)
except (OSError, UnidentifiedImageError):
# Combining the tiles remains responsible for rejecting corrupt image
# bytes. This check only identifies valid images with blank pixels.
- return False, 0.0
+ return ScreenshotBlanknessMetrics(False, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0)
+
+
+def is_screenshot_nearly_uniform(screenshot: bytes) -> tuple[bool, float]:
+ """Return the blank decision and dominant-color ratio for compatibility."""
+
+ metrics = get_screenshot_blankness_metrics(screenshot)
+ return metrics.is_blank, metrics.dominant_pixel_ratio
try:
- from playwright.sync_api import TimeoutError as PlaywrightTimeout
+ from playwright.sync_api import (
+ Error as PlaywrightError,
+ TimeoutError as PlaywrightTimeout,
+ )
except ImportError:
- PlaywrightTimeout = Exception
+
+ class PlaywrightError(Exception): # type: ignore[no-redef]
+ """Fallback Playwright error that excludes unrelated exceptions."""
+
+ class PlaywrightTimeout(PlaywrightError): # type: ignore[no-redef] #
noqa: N818
+ """Fallback matching Playwright's timeout error hierarchy."""
+
if TYPE_CHECKING:
try:
@@ -441,6 +543,23 @@ CHART_CONTAINER_STATE_JS = f"""
}}
"""
+CHART_CONTAINER_HAS_RENDERED_CONTENT_JS = f"""
+() => {{
+ const chart = document.querySelector('.chart-container');
+ return chart !== null
+ && chart.querySelector('{CHART_ERROR_OR_EMPTY_SELECTOR}') === null
+ && chart.querySelector('{SLICE_CONTAINER_SELECTOR}') !== null;
+}}
+"""
+
+REPORT_HAS_RENDERED_CHART_HOLDERS_JS = f"""
+() => Array.from(document.querySelectorAll('{CHART_HOLDER_SELECTOR}')).some(
+ holder => holder.querySelector(
+ '{CHART_ERROR_OR_EMPTY_SELECTOR}'
+ ) === null && holder.querySelector('{SLICE_CONTAINER_SELECTOR}') !== null
+)
+"""
+
CHART_CONTAINER_SELECTOR = ".chart-container"
# `.slice_container` (superset-frontend/src/components/Chart/Chart.tsx) is
@@ -636,14 +755,7 @@ def take_tiled_screenshot( # noqa: C901
log_context = report_execution_context.log_context
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. Deciding
- # whether to propagate via `isinstance(e, PlaywrightTimeout)` would be
- # unreliable: when the playwright package isn't installed,
- # `PlaywrightTimeout` is aliased to the bare `Exception` class (see the
- # try/except ImportError above this function), which would make *any*
- # exception -- not just our own deliberate readiness-timeout raise --
- # match `except PlaywrightTimeout` and incorrectly propagate instead of
- # degrading to `None` like every other unexpected error in this function.
+ # checked in the except block at the bottom of this function.
readiness_timeout = False
if screenshot_started_at is None:
screenshot_started_at = time.monotonic()
@@ -771,6 +883,7 @@ def take_tiled_screenshot( # noqa: C901
screenshot_tiles: list[bytes] = []
blank_tile_retries = 0
+ contentful_tiles_captured = 0
def _raise_if_budget_exhausted() -> None:
elapsed, remaining = _deadline_values()
@@ -1082,20 +1195,30 @@ def take_tiled_screenshot( # noqa: C901
) from ex
else:
capture_elapsed = time.monotonic() - capture_started_at
- is_uniform, dominant_ratio =
is_screenshot_nearly_uniform(candidate)
- is_blank = is_uniform and (
+ blankness = get_screenshot_blankness_metrics(candidate)
+ is_blank = blankness.is_blank and (
contentful_chart_holders > 0 or holder_count_failed
)
- logger.debug(
- "Captured tile %s/%s attempt %s/%s in %.2fs "
- "(contentful_chart_holders=%s
dominant_pixel_ratio=%.5f)%s",
+ logger.info(
+ "report_capture_validation capture=tile tile=%s/%s "
+ "attempt=%s/%s capture_elapsed_seconds=%.2f "
+ "contentful_chart_holders=%s is_blank=%s "
+ "dominant_pixel_ratio=%.5f near_white_pixel_ratio=%.5f
"
+ "mean_luminance=%.2f luminance_stddev=%.2f
entropy=%.3f "
+ "structural_edge_ratio=%.5f%s",
i + 1,
num_tiles,
capture_attempt,
TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
capture_elapsed,
contentful_chart_holders,
- dominant_ratio,
+ is_blank,
+ blankness.dominant_pixel_ratio,
+ blankness.near_white_pixel_ratio,
+ blankness.mean_luminance,
+ blankness.luminance_stddev,
+ blankness.entropy,
+ blankness.structural_edge_ratio,
context_suffix,
)
if not is_blank:
@@ -1105,32 +1228,63 @@ def take_tiled_screenshot( # noqa: C901
logger.warning(
"report_capture_blank_tile tile=%s/%s attempt=%s/%s "
"capture_elapsed_seconds=%.2f
contentful_chart_holders=%s "
- "dominant_pixel_ratio=%.5f%s",
+ "dominant_pixel_ratio=%.5f near_white_pixel_ratio=%.5f
"
+ "mean_luminance=%.2f luminance_stddev=%.2f
entropy=%.3f "
+ "structural_edge_ratio=%.5f%s",
i + 1,
num_tiles,
capture_attempt,
TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
capture_elapsed,
contentful_chart_holders,
- dominant_ratio,
+ blankness.dominant_pixel_ratio,
+ blankness.near_white_pixel_ratio,
+ blankness.mean_luminance,
+ blankness.luminance_stddev,
+ blankness.entropy,
+ blankness.structural_edge_ratio,
context_suffix,
)
if capture_attempt ==
TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS:
+ if report_execution_context:
+ # Exhausted retries must never turn a rejected
report
+ # tile into accepted content. Only thumbnails may
+ # retain a blank capture below.
+ raise ScreenshotBlankCaptureError(
+ "Chromium returned a blank tile "
+ f"{i + 1}/{num_tiles} after {capture_attempt}
attempts"
+ )
tile_screenshot = candidate
logger.warning(
- "report_capture_uniform_tile_retained tile=%s/%s "
+ "report_capture_blank_tile_retained tile=%s/%s "
"attempts=%s contentful_chart_holders=%s "
- "dominant_pixel_ratio=%.5f%s",
+ "dominant_pixel_ratio=%.5f
near_white_pixel_ratio=%.5f "
+ "mean_luminance=%.2f luminance_stddev=%.2f
entropy=%.3f "
+ "structural_edge_ratio=%.5f%s",
i + 1,
num_tiles,
capture_attempt,
contentful_chart_holders,
- dominant_ratio,
+ blankness.dominant_pixel_ratio,
+ blankness.near_white_pixel_ratio,
+ blankness.mean_luminance,
+ blankness.luminance_stddev,
+ blankness.entropy,
+ blankness.structural_edge_ratio,
context_suffix,
)
break
_raise_if_budget_exhausted()
+ repaint_timeout = _timeout_seconds(
+ "screenshot_repaint",
+ requested_seconds=5.0,
+ reserve_seconds=(
+ report_execution_context.post_capture_reserve_seconds
+ if report_execution_context
+ else 0.0
+ ),
+ )
try:
page.bring_to_front()
page.evaluate(
@@ -1143,20 +1297,11 @@ def take_tiled_screenshot( # noqa: C901
}));
}"""
)
- repaint_timeout = _timeout_seconds(
- "screenshot_repaint",
- requested_seconds=5.0,
- reserve_seconds=(
-
report_execution_context.post_capture_reserve_seconds
- if report_execution_context
- else 0.0
- ),
- )
page.wait_for_function(
"() => window.__supersetRepaintComplete === true",
timeout=repaint_timeout * 1000,
)
- except Exception: # noqa: BLE001
+ except PlaywrightError:
logger.warning(
"report_capture_repaint_timeout tile=%s/%s
attempt=%s/%s%s",
i + 1,
@@ -1169,6 +1314,8 @@ def take_tiled_screenshot( # noqa: C901
assert tile_screenshot is not None
screenshot_tiles.append(tile_screenshot)
+ if contentful_chart_holders > 0:
+ contentful_tiles_captured += 1
logger.debug(
"Captured tile %s/%s with clip %s%s",
@@ -1224,6 +1371,31 @@ def take_tiled_screenshot( # noqa: C901
log_context=log_context,
)
+ if report_execution_context and contentful_tiles_captured:
+ combined_blankness =
get_screenshot_blankness_metrics(combined_screenshot)
+ logger.info(
+ "report_capture_validation capture=combined "
+ "contentful_tiles=%s is_blank=%s "
+ "dominant_pixel_ratio=%.5f near_white_pixel_ratio=%.5f "
+ "mean_luminance=%.2f luminance_stddev=%.2f entropy=%.3f "
+ "structural_edge_ratio=%.5f%s",
+ contentful_tiles_captured,
+ combined_blankness.is_blank,
+ combined_blankness.dominant_pixel_ratio,
+ combined_blankness.near_white_pixel_ratio,
+ combined_blankness.mean_luminance,
+ combined_blankness.luminance_stddev,
+ combined_blankness.entropy,
+ combined_blankness.structural_edge_ratio,
+ context_suffix,
+ )
+ if combined_blankness.is_blank:
+ logger.warning(
+ "report_capture_blank_combined_retained
contentful_tiles=%s%s",
+ contentful_tiles_captured,
+ context_suffix,
+ )
+
return combined_screenshot
except (ReportExecutionBudgetExceededError,
TiledScreenshotBudgetExceededError):
@@ -1242,13 +1414,15 @@ def take_tiled_screenshot( # noqa: C901
context_suffix,
)
raise
- except ScreenshotCaptureTimeoutError:
- # Preserve the explicit capture-timeout reason for report execution
+ except (ScreenshotBlankCaptureError, ScreenshotCaptureTimeoutError):
+ # 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:
raise
return None
+ except SoftTimeLimitExceeded:
+ raise
except Exception as e:
if readiness_timeout:
# Let the per-tile readiness timeout propagate so the caller
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index 337ab698258..9a5f32e75a8 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -31,6 +31,7 @@ from superset.utils.report_execution import (
ReportExecutionContext,
)
from superset.utils.screenshot_utils import (
+ CHART_CONTAINER_HAS_RENDERED_CONTENT_JS,
CHART_CONTAINER_READY_JS,
CHART_CONTAINER_STATE_JS,
CHART_HOLDERS_READY_JS,
@@ -39,10 +40,14 @@ from superset.utils.screenshot_utils import (
FIND_ALL_UNREADY_CHART_HOLDERS_JS,
FIND_CHART_HOLDER_STATES_JS,
FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS,
+ get_screenshot_blankness_metrics,
REPORT_ALL_CHART_HOLDERS_READY_JS,
+ REPORT_HAS_RENDERED_CHART_HOLDERS_JS,
resolve_screenshot_task_budget_seconds,
+ ScreenshotBlankCaptureError,
ScreenshotTaskBudgetExceededError,
take_tiled_screenshot,
+ TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
)
WindowSize = tuple[int, int]
@@ -75,8 +80,13 @@ except ImportError:
# Define dummy classes when playwright is not available
BrowserContext = Any
- PlaywrightError = Exception
- PlaywrightTimeout = Exception
+
+ class PlaywrightError(Exception): # type: ignore[no-redef]
+ """Fallback Playwright error that does not swallow unrelated
failures."""
+
+ class PlaywrightTimeout(PlaywrightError): # type: ignore[no-redef] #
noqa: N818
+ """Fallback matching Playwright's timeout error hierarchy."""
+
Locator = Any
Page = Any
sync_playwright = None
@@ -240,6 +250,131 @@ class WebDriverPlaywright(WebDriverProxy):
else:
return element.screenshot(**timeout_kwargs)
+ @staticmethod
+ def _get_validated_screenshot(
+ page: Page,
+ element: Locator,
+ element_name: str,
+ log_context: str | None,
+ report_execution_context: ReportExecutionContext | None,
+ ) -> bytes:
+ """Capture a standard screenshot and reject blank report output."""
+
+ context_suffix = f" [{log_context}]" if log_context else ""
+ for attempt in range(1, TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS + 1):
+ capture_timeout = (
+ report_execution_context.deadline.timeout_seconds(
+ "screenshot_capture",
+ reserve_seconds=(
+ report_execution_context.post_capture_reserve_seconds
+ ),
+ )
+ if report_execution_context
+ else None
+ )
+ capture_started_at = time.monotonic()
+ image = WebDriverPlaywright._get_screenshot(
+ page,
+ element,
+ element_name,
+ timeout_seconds=capture_timeout,
+ )
+ capture_elapsed = time.monotonic() - capture_started_at
+ if report_execution_context is None:
+ return image
+
+ blankness = get_screenshot_blankness_metrics(image)
+ try:
+ has_rendered_content = bool(
+ page.evaluate(
+ CHART_CONTAINER_HAS_RENDERED_CONTENT_JS
+ if element_name == "chart-container"
+ else REPORT_HAS_RENDERED_CHART_HOLDERS_JS
+ )
+ )
+ except PlaywrightError:
+ has_rendered_content = False
+ logger.warning(
+ "report_capture_content_state_failed capture=standard%s",
+ context_suffix,
+ exc_info=True,
+ )
+ is_blank = has_rendered_content and blankness.is_blank
+ logger.info(
+ "report_capture_validation capture=standard attempt=%s/%s "
+ "capture_elapsed_seconds=%.2f has_rendered_content=%s
is_blank=%s "
+ "dominant_pixel_ratio=%.5f near_white_pixel_ratio=%.5f "
+ "mean_luminance=%.2f luminance_stddev=%.2f entropy=%.3f "
+ "structural_edge_ratio=%.5f%s",
+ attempt,
+ TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
+ capture_elapsed,
+ has_rendered_content,
+ is_blank,
+ blankness.dominant_pixel_ratio,
+ blankness.near_white_pixel_ratio,
+ blankness.mean_luminance,
+ blankness.luminance_stddev,
+ blankness.entropy,
+ blankness.structural_edge_ratio,
+ context_suffix,
+ )
+ if not is_blank:
+ return image
+
+ logger.warning(
+ "report_capture_blank_standard attempt=%s/%s "
+ "capture_elapsed_seconds=%.2f dominant_pixel_ratio=%.5f "
+ "near_white_pixel_ratio=%.5f mean_luminance=%.2f "
+ "luminance_stddev=%.2f entropy=%.3f
structural_edge_ratio=%.5f%s",
+ attempt,
+ TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
+ capture_elapsed,
+ blankness.dominant_pixel_ratio,
+ blankness.near_white_pixel_ratio,
+ blankness.mean_luminance,
+ blankness.luminance_stddev,
+ blankness.entropy,
+ blankness.structural_edge_ratio,
+ context_suffix,
+ )
+ if attempt == TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS:
+ raise ScreenshotBlankCaptureError(
+ "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,
+ )
+ try:
+ page.bring_to_front()
+ page.evaluate(
+ """() => {
+ window.scrollBy(0, 1);
+ window.scrollBy(0, -1);
+ window.__supersetRepaintComplete = false;
+ requestAnimationFrame(() => requestAnimationFrame(()
=> {
+ window.__supersetRepaintComplete = true;
+ }));
+ }"""
+ )
+ page.wait_for_function(
+ "() => window.__supersetRepaintComplete === true",
+ timeout=repaint_timeout * 1000,
+ )
+ except PlaywrightError:
+ logger.warning(
+ "report_capture_repaint_timeout capture=standard
attempt=%s/%s%s",
+ attempt,
+ TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS,
+ context_suffix,
+ exc_info=True,
+ )
+
+ raise AssertionError("standard screenshot retry loop did not return or
raise")
+
@staticmethod
def _expand_scrollable_content(
page: Page,
@@ -948,21 +1083,12 @@ class WebDriverPlaywright(WebDriverProxy):
user.username if user else "None",
context_suffix,
)
- capture_timeout = (
- report_execution_context.deadline.timeout_seconds(
- "screenshot_capture",
- reserve_seconds=(
-
report_execution_context.post_capture_reserve_seconds
- ),
- )
- if report_execution_context
- else None
- )
- img = WebDriverPlaywright._get_screenshot(
+ img = WebDriverPlaywright._get_validated_screenshot(
page,
element,
element_name,
- timeout_seconds=capture_timeout,
+ log_context,
+ report_execution_context,
)
logger.debug(
"Screenshot result: %d bytes for url: %s%s",
@@ -1020,21 +1146,12 @@ class WebDriverPlaywright(WebDriverProxy):
user.username if user else "None",
context_suffix,
)
- capture_timeout = (
- report_execution_context.deadline.timeout_seconds(
- "screenshot_capture",
- reserve_seconds=(
-
report_execution_context.post_capture_reserve_seconds
- ),
- )
- if report_execution_context
- else None
- )
- img = WebDriverPlaywright._get_screenshot(
+ img = WebDriverPlaywright._get_validated_screenshot(
page,
element,
element_name,
- timeout_seconds=capture_timeout,
+ log_context,
+ report_execution_context,
)
logger.debug(
"Screenshot result: %d bytes for url: %s%s",
diff --git a/tests/unit_tests/commands/report/execute_test.py
b/tests/unit_tests/commands/report/execute_test.py
index 5bf2da33dd8..15d14decec1 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -1774,6 +1774,43 @@ def
test_screenshot_soft_timeout_distinguishes_reports_from_alert_attachments(
state._get_screenshots()
[email protected]("has_chart", [True, False])
+def test_blank_capture_prevents_pdf_generation_and_delivery(
+ app: SupersetApp, mocker: MockerFixture, has_chart: bool
+) -> None:
+ """A rejected capture must abort the PDF before any recipient delivery."""
+ from superset.utils.screenshot_utils import ScreenshotBlankCaptureError
+
+ state = _make_notification_state(
+ mocker, report_format=ReportDataFormat.PDF, has_chart=has_chart
+ )
+ state._report_schedule.custom_width = None
+ state._report_schedule.custom_height = None
+ mocker.patch(
+ "superset.commands.report.execute.resolve_executor_user",
+ return_value=(mocker.Mock(), "executor"),
+ )
+ mocker.patch.object(state, "get_dashboard_urls",
return_value=["/dashboard/1"])
+ screenshot_class = "ChartScreenshot" if has_chart else
"DashboardScreenshot"
+ screenshot = mocker.patch(
+ f"superset.commands.report.execute.{screenshot_class}"
+ ).return_value
+ capture_error = ScreenshotBlankCaptureError("blank capture after 3
attempts")
+ screenshot.get_screenshot.side_effect = capture_error
+ build_pdf = mocker.patch(
+ "superset.commands.report.execute.build_pdf_from_screenshots"
+ )
+ deliver = mocker.patch.object(state, "_send")
+
+ with pytest.raises(ReportScheduleScreenshotFailedError) as exc:
+ state.send()
+
+ assert exc.value.__cause__ is capture_error
+ screenshot.get_screenshot.assert_called_once()
+ build_pdf.assert_not_called()
+ deliver.assert_not_called()
+
+
def test_executor_not_found_error_message_without_username() -> None:
"""
When no username is available, the message falls back to ``(unknown)``
diff --git a/tests/unit_tests/utils/test_screenshot_utils.py
b/tests/unit_tests/utils/test_screenshot_utils.py
index f14f9dc6110..34beab9ac58 100644
--- a/tests/unit_tests/utils/test_screenshot_utils.py
+++ b/tests/unit_tests/utils/test_screenshot_utils.py
@@ -20,7 +20,7 @@ from unittest.mock import MagicMock, patch
from uuid import UUID
import pytest
-from PIL import Image, UnidentifiedImageError
+from PIL import Image, ImageDraw, ImageFont, UnidentifiedImageError
from superset.utils.report_execution import (
ReportExecutionContext,
@@ -29,9 +29,11 @@ from superset.utils.report_execution import (
from superset.utils.screenshot_utils import (
combine_screenshot_tiles,
CONTENTFUL_CHART_HOLDERS_IN_CLIP_JS,
+ get_screenshot_blankness_metrics,
is_screenshot_nearly_uniform,
resolve_screenshot_task_budget_seconds,
SCREENSHOT_TASK_BUDGET_MAX_MARGIN_SECONDS,
+ ScreenshotBlankCaptureError,
ScreenshotCaptureTimeoutError,
ScreenshotTaskBudgetExceededError,
SCROLL_SETTLE_TIMEOUT_MS,
@@ -48,6 +50,16 @@ def _png(width: int, height: int, color: str) -> bytes:
return output.getvalue()
+def _two_tone_blank(width: int = 100, height: int = 100) -> bytes:
+ image = Image.new("RGB", (width, height), "white")
+ for y in range(int(height * 0.85), height):
+ for x in range(width):
+ image.putpixel((x, y), (245, 245, 245))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+ return output.getvalue()
+
+
class TestScreenshotBlankDetection:
def test_uniform_png_is_blank(self):
is_blank, dominant_ratio = is_screenshot_nearly_uniform(_png(100, 100,
"white"))
@@ -68,6 +80,116 @@ class TestScreenshotBlankDetection:
assert is_blank is False
assert dominant_ratio < 0.995
+ def test_two_tone_near_white_png_is_blank(self):
+ is_blank, dominant_ratio =
is_screenshot_nearly_uniform(_two_tone_blank())
+
+ assert is_blank is True
+ assert dominant_ratio == 0.85
+
+ def test_two_tone_gray_below_near_white_cutoff_is_blank(self):
+ image = Image.new("RGB", (100, 100), "white")
+ for y in range(85, 100):
+ for x in range(100):
+ image.putpixel((x, y), (239, 239, 239))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is True
+ assert dominant_ratio == 0.85
+
+ def test_sparse_readable_text_is_not_blank(self):
+ image = Image.new("RGB", (800, 1000), "white")
+ label = Image.new("RGB", (60, 14), "white")
+ ImageDraw.Draw(label).text(
+ (0, 0), "No data", fill="black", font=ImageFont.load_default()
+ )
+ label = label.resize((240, 56), Image.Resampling.NEAREST)
+ image.paste(label, (20, 20))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, _dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is False
+
+ def test_tall_sparse_report_with_content_is_not_blank(self):
+ image = Image.new("RGB", (800, 4000), "white")
+ draw = ImageDraw.Draw(image)
+ draw.rectangle((20, 20, 780, 220), outline="black", width=3)
+ for y in range(60, 220, 40):
+ draw.line((20, y, 780, y), fill="black", width=2)
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, _dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is False
+
+ def test_high_density_light_thin_line_chart_is_not_blank(self):
+ image = Image.new("RGB", (3000, 1200), "white")
+ draw = ImageDraw.Draw(image)
+ for y in (200, 500, 800, 1050):
+ draw.line((180, y, 2850, y), fill=(225, 225, 225), width=2)
+ draw.line((180, 100, 180, 1050), fill=(170, 170, 170), width=3)
+ draw.line((180, 1050, 2850, 1050), fill=(170, 170, 170), width=3)
+ points = [
+ (250, 900),
+ (700, 720),
+ (1200, 790),
+ (1750, 480),
+ (2250, 560),
+ (2800, 250),
+ ]
+ draw.line(points, fill=(145, 180, 210), width=3)
+ for x, y in points:
+ draw.ellipse((x - 5, y - 5, x + 5, y + 5), fill=(145, 180, 210))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ metrics = get_screenshot_blankness_metrics(output.getvalue())
+
+ assert metrics.is_blank is False
+ assert metrics.structural_edge_ratio >= 0.02
+
+ def test_high_density_sparse_light_table_is_not_blank(self):
+ image = Image.new("RGB", (3000, 1200), "white")
+ draw = ImageDraw.Draw(image)
+ columns = (150, 900, 1700, 2850)
+ rows = (150, 350, 550, 750)
+ for y in rows:
+ draw.line((columns[0], y, columns[-1], y), fill=(210, 210, 210),
width=2)
+ for x in columns:
+ draw.line((x, rows[0], x, rows[-1]), fill=(210, 210, 210), width=2)
+ for row in range(3):
+ for column in range(3):
+ x = columns[column] + 35
+ y = rows[row] + 55
+ draw.rectangle(
+ (x, y, x + 180 + column * 80, y + 9), fill=(155, 155, 155)
+ )
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ metrics = get_screenshot_blankness_metrics(output.getvalue())
+
+ assert metrics.is_blank is False
+ assert metrics.structural_edge_ratio >= 0.02
+
+ def test_sparse_png_with_dark_content_is_not_blank(self):
+ image = Image.new("RGB", (100, 100), "white")
+ for x in range(20, 80):
+ for y in range(20, 24):
+ image.putpixel((x, y), (50, 50, 50))
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+
+ is_blank, dominant_ratio =
is_screenshot_nearly_uniform(output.getvalue())
+
+ assert is_blank is False
+ assert dominant_ratio == 0.976
+
def test_non_image_bytes_are_left_for_combination_validation(self):
assert is_screenshot_nearly_uniform(b"not an image") == (False, 0.0)
@@ -320,7 +442,7 @@ class TestTakeTiledScreenshot:
)
assert retry_log.args[1] == 1
- def test_repeated_uniform_tiles_are_retained_with_warning(self, mock_page):
+ def test_repeated_blank_tiles_fail_closed_for_reports(self, mock_page):
element_info = {"height": 2000, "top": 0, "left": 0, "width": 800}
def evaluate(script, _arg=None):
@@ -336,11 +458,48 @@ class TestTakeTiledScreenshot:
mock_page.screenshot.return_value = _png(800, 1000, "white")
with (
- patch("superset.utils.screenshot_utils.logger") as mock_logger,
+ patch("superset.utils.screenshot_utils.logger"),
patch(
- "superset.utils.screenshot_utils.combine_screenshot_tiles",
- return_value=b"combined",
+ "superset.utils.screenshot_utils.combine_screenshot_tiles"
) as mock_combine,
+ pytest.raises(
+ ScreenshotBlankCaptureError,
+ match="blank tile 1/2 after 3 attempts",
+ ),
+ ):
+ take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=1000,
+ report_execution_context=_report_context(),
+ )
+
+ 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
+ ):
+ 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 = self._create_chart_like_tile()
+
+ with (
+ patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles",
+ return_value=_two_tone_blank(800, 1000),
+ ),
+ patch("superset.utils.screenshot_utils.logger") as mock_logger,
):
result = take_tiled_screenshot(
mock_page,
@@ -349,17 +508,44 @@ class TestTakeTiledScreenshot:
report_execution_context=_report_context(),
)
- assert result == b"combined"
- assert mock_page.screenshot.call_count == 6
- assert len(mock_combine.call_args.args[0]) == 2
- retained_warnings = [
- call
+ assert result == _two_tone_blank(800, 1000)
+ assert any(
+ call.args[0].startswith("report_capture_blank_combined_retained")
+ and call.args[1] == 1
for call in mock_logger.warning.call_args_list
- if call.args[0].startswith("report_capture_uniform_tile_retained")
- ]
- assert len(retained_warnings) == 2
+ )
+
+ def test_blank_combined_image_is_allowed_for_terminal_empty_states(self,
mock_page):
+ element_info = {"height": 1000, "top": 0, "left": 0, "width": 800}
+ blank = _two_tone_blank(800, 1000)
+
+ def evaluate(script, _arg=None):
+ if "scrollWidth" in script:
+ return element_info
+ if script == CONTENTFUL_CHART_HOLDERS_IN_CLIP_JS:
+ return {"total": 1, "contentful": 0}
+ if "window.scrollTo" in script:
+ return None
+ return [{"chartId": "7", "state": "empty"}]
+
+ mock_page.evaluate.side_effect = evaluate
+ mock_page.screenshot.return_value = blank
+
+ with patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles",
+ return_value=blank,
+ ):
+ result = take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=1000,
+ report_execution_context=_report_context(),
+ )
+
+ assert result == blank
+ assert mock_page.screenshot.call_count == 1
- def test_single_uniform_content_tile_is_retained(self, mock_page):
+ def test_single_uniform_content_tile_fails_closed_for_reports(self,
mock_page):
element_info = {"height": 1000, "top": 0, "left": 0, "width": 800}
uniform_tile = _png(800, 1000, "navy")
@@ -375,24 +561,21 @@ class TestTakeTiledScreenshot:
mock_page.evaluate.side_effect = evaluate
mock_page.screenshot.return_value = uniform_tile
- with patch(
- "superset.utils.screenshot_utils.combine_screenshot_tiles",
- return_value=b"combined",
- ) as mock_combine:
- result = take_tiled_screenshot(
+ with (
+ patch(
+ "superset.utils.screenshot_utils.combine_screenshot_tiles"
+ ) as mock_combine,
+ pytest.raises(ScreenshotBlankCaptureError),
+ ):
+ take_tiled_screenshot(
mock_page,
"dashboard",
tile_height=2000,
report_execution_context=_report_context(),
)
- assert result == b"combined"
assert mock_page.screenshot.call_count == 3
- mock_combine.assert_called_once_with(
- [uniform_tile],
- allow_partial_fallback=False,
- log_context=_report_context().log_context,
- )
+ mock_combine.assert_not_called()
def test_uniform_tile_without_visible_charts_is_allowed(self, mock_page):
element_info = {"height": 1000, "top": 0, "left": 0, "width": 800}
@@ -536,7 +719,7 @@ class TestTakeTiledScreenshot:
for call in mock_logger.warning.call_args_list
)
assert any(
- call.args[0].startswith("report_capture_uniform_tile_retained")
+ call.args[0].startswith("report_capture_blank_tile_retained")
for call in mock_logger.warning.call_args_list
)
@@ -586,6 +769,32 @@ class TestTakeTiledScreenshot:
)
assert mock_page.screenshot.call_count == 2
+ def test_repaint_preserves_celery_soft_timeout(self, mock_page):
+ from celery.exceptions import SoftTimeLimitExceeded
+
+ 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")
+ mock_page.wait_for_function.side_effect = [None, None,
SoftTimeLimitExceeded()]
+
+ with pytest.raises(SoftTimeLimitExceeded):
+ take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=2000,
+ report_execution_context=_report_context(),
+ )
+
def test_repeated_capture_timeout_preserves_thumbnail_contract(self,
mock_page):
from superset.utils.screenshot_utils import PlaywrightTimeout
diff --git a/tests/unit_tests/utils/webdriver_test.py
b/tests/unit_tests/utils/webdriver_test.py
index a1759a12ec4..e031f847d58 100644
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -15,15 +15,20 @@
# specific language governing permissions and limitations
# under the License.
+import io
+from dataclasses import replace
from unittest.mock import ANY, MagicMock, patch
from uuid import UUID
import pytest
+from celery.exceptions import SoftTimeLimitExceeded
+from PIL import Image, ImageDraw
from superset.utils.report_execution import (
ReportExecutionContext,
ReportExecutionDeadline,
)
+from superset.utils.screenshot_utils import ScreenshotBlankCaptureError
from superset.utils.webdriver import (
check_playwright_availability,
PLAYWRIGHT_AVAILABLE,
@@ -57,6 +62,13 @@ def _report_context(
)
+def _png(color: str) -> bytes:
+ image = Image.new("RGB", (100, 100), color)
+ output = io.BytesIO()
+ image.save(output, format="PNG")
+ return output.getvalue()
+
+
@pytest.fixture()
def mock_app():
"""Mock Flask app with webdriver configuration."""
@@ -92,6 +104,131 @@ class TestPlaywrightAvailabilityCheck:
mock_sync_playwright.assert_not_called()
+class TestStandardScreenshotValidation:
+ def test_blank_capture_retries_then_accepts_content(self):
+ page = MagicMock()
+ element = MagicMock()
+ valid = Image.new("RGB", (100, 100), "white")
+ for x in range(20, 80):
+ for y in range(20, 80):
+ valid.putpixel((x, y), (50, 50, 50))
+ output = io.BytesIO()
+ valid.save(output, format="PNG")
+ page.screenshot.side_effect = [_png("white"), output.getvalue()]
+
+ result = WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "execution_id=test",
+ _report_context(),
+ )
+
+ assert result == output.getvalue()
+ assert page.screenshot.call_count == 2
+ page.bring_to_front.assert_called_once_with()
+ page.wait_for_function.assert_called_once_with(
+ "() => window.__supersetRepaintComplete === true",
+ timeout=5000,
+ )
+
+ def test_repeated_blank_capture_fails_closed(self):
+ page = MagicMock()
+ element = MagicMock()
+ page.screenshot.return_value = _png("white")
+
+ with pytest.raises(
+ ScreenshotBlankCaptureError,
+ match="blank standard screenshot after 3 attempts",
+ ):
+ WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "execution_id=test",
+ _report_context(),
+ )
+
+ assert page.screenshot.call_count == 3
+
+ def test_blank_empty_state_capture_is_allowed(self):
+ page = MagicMock()
+ element = MagicMock()
+ page.screenshot.return_value = _png("white")
+ page.evaluate.return_value = False
+
+ result = WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "execution_id=test",
+ _report_context(),
+ )
+
+ assert result == _png("white")
+ page.screenshot.assert_called_once()
+ page.bring_to_front.assert_not_called()
+
+ def test_repaint_timeout_is_bounded_and_retry_continues(self):
+ from superset.utils.webdriver import PlaywrightTimeout
+
+ page = MagicMock()
+ element = MagicMock()
+ valid = Image.new("RGB", (100, 100), "white")
+ ImageDraw.Draw(valid).rectangle((20, 20, 80, 80), fill="black")
+ output = io.BytesIO()
+ valid.save(output, format="PNG")
+ page.screenshot.side_effect = [_png("white"), output.getvalue()]
+ page.evaluate.return_value = True
+ page.wait_for_function.side_effect = PlaywrightTimeout("stalled
repaint")
+
+ result = WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "execution_id=test",
+ _report_context(),
+ )
+
+ assert result == output.getvalue()
+ page.wait_for_function.assert_called_once_with(
+ "() => window.__supersetRepaintComplete === true",
+ timeout=5000,
+ )
+
+ def test_repaint_preserves_celery_soft_timeout(self):
+ page = MagicMock()
+ element = MagicMock()
+ page.screenshot.return_value = _png("white")
+ page.evaluate.return_value = True
+ page.wait_for_function.side_effect = SoftTimeLimitExceeded()
+
+ with pytest.raises(SoftTimeLimitExceeded):
+ WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ "execution_id=test",
+ _report_context(),
+ )
+
+ def test_non_report_capture_preserves_existing_behavior(self):
+ page = MagicMock()
+ element = MagicMock()
+ page.screenshot.return_value = _png("white")
+
+ result = WebDriverPlaywright._get_validated_screenshot(
+ page,
+ element,
+ "standalone",
+ None,
+ None,
+ )
+
+ assert result == _png("white")
+ page.screenshot.assert_called_once_with(full_page=True)
+
+
class TestWebDriverPlaywrightFallback:
"""Test WebDriverPlaywright fallback behavior when unavailable."""
@@ -380,18 +517,17 @@ class TestWebDriverPlaywrightErrorHandling:
mock_page = MagicMock()
report_execution_context = _report_context()
# Only 2s left for this phase after other phases' reserves.
- report_execution_context = report_execution_context.__class__(
- **{
- **report_execution_context.__dict__,
- "deadline": report_execution_context.deadline.__class__(
- total_seconds=2
- + report_execution_context.capture_reserve_seconds
- + report_execution_context.delivery_reserve_seconds
- + report_execution_context.cleanup_reserve_seconds,
- started_at=0,
- _clock=lambda: 0,
- ),
- }
+ report_execution_context = replace(
+ report_execution_context,
+ deadline=replace(
+ report_execution_context.deadline,
+ total_seconds=2
+ + report_execution_context.capture_reserve_seconds
+ + report_execution_context.delivery_reserve_seconds
+ + report_execution_context.cleanup_reserve_seconds,
+ started_at=0,
+ _clock=lambda: 0,
+ ),
)
WebDriverPlaywright._expand_scrollable_content(
@@ -594,9 +730,7 @@ class TestWebDriverPlaywrightErrorHandling:
"http://example.com";, "test-element", mock_user
)
- # The exact injected instance must propagate — guards against the
- # fallback alias (PlaywrightTimeout = Exception when playwright is
- # not installed) accepting unrelated exceptions.
+ # The exact injected instance must propagate.
assert exc_info.value is timeout
mock_logger.exception.assert_any_call(
"Timed out requesting url %s%s", "http://example.com";, ""
@@ -641,7 +775,7 @@ class TestWebDriverPlaywrightErrorHandling:
mock_chart_container.wait_for.return_value = None
mock_page.wait_for_timeout.return_value = None
- def evaluate_side_effect(script):
+ def evaluate_side_effect(script, *_args):
if script ==
'document.querySelectorAll(".chart-container").length':
return 1
if "const target = document.querySelector" in script:
@@ -733,7 +867,7 @@ class TestWebDriverPlaywrightErrorHandling:
mock_page.wait_for_timeout.return_value = None
mock_take_tiled.return_value = b"tiled_screenshot"
- def evaluate_side_effect(script):
+ def evaluate_side_effect(script, *_args):
if script ==
'document.querySelectorAll(".chart-container").length':
return 25 # chart_count >= threshold
if "const target = document.querySelector" in script:
@@ -816,7 +950,7 @@ class TestWebDriverPlaywrightErrorHandling:
mock_page.locator.side_effect = locator_side_effect
mock_take_tiled.return_value = b"tiled_screenshot"
- def evaluate_side_effect(script):
+ def evaluate_side_effect(script, *_args):
if script ==
'document.querySelectorAll(".chart-container").length':
return 52 # mounted containers
if "const target = document.querySelector" in script:
@@ -970,7 +1104,7 @@ class TestWebDriverPlaywrightErrorHandling:
# it must never be reached by the failure path under test.
mock_page.screenshot.return_value = b"fallback_screenshot"
- def evaluate_side_effect(script):
+ def evaluate_side_effect(script, *_args):
if "querySelectorAll" in script:
return 25 # chart_count >= threshold
if "const target" in script:
@@ -1001,8 +1135,6 @@ class TestWebDriverPlaywrightErrorHandling:
mock_auth.return_value = mock_context
driver = WebDriverPlaywright("chrome")
- # match= keeps this assertion meaningful even when playwright
- # is not installed and PlaywrightTimeout aliases bare
Exception.
with pytest.raises(
PlaywrightTimeout, match="Tiled screenshot failed for url"
):
@@ -1299,6 +1431,8 @@ class TestWebDriverPlaywrightChartReadiness:
self, mock_app, mock_logger, mock_browser_manager
):
"""Elapsed setup time is removed from the task-derived safe budget."""
+ from itertools import chain, repeat
+
mock_user = MagicMock()
mock_user.username = "test_user"
mock_app.config = {
@@ -1315,7 +1449,7 @@ class TestWebDriverPlaywrightChartReadiness:
),
patch(
"superset.utils.webdriver.time.monotonic",
- side_effect=[100.0, 110.0],
+ side_effect=chain([100.0], repeat(110.0)),
),
):
WebDriverPlaywright("chrome").get_screenshot(
@@ -1746,8 +1880,16 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
mock_context, mock_page = self._make_pw_mocks(mock_browser_manager)
# Small dashboard: 3 charts, 1000px height — below both thresholds.
- # First item is consumed by the pre-capture scrollable-content
expansion.
- mock_page.evaluate.side_effect = [None, 3, 1000, [], []]
+ def evaluate_side_effect(script, *_args):
+ if script ==
'document.querySelectorAll(".chart-container").length':
+ return 3
+ if "const target = document.querySelector" in script:
+ return 1000
+ if "dashboard-component-chart-holder" in script:
+ return []
+ return None
+
+ mock_page.evaluate.side_effect = evaluate_side_effect
call_order: list[str] = []
@@ -1789,8 +1931,17 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
"SCREENSHOT_TILED_VIEWPORT_HEIGHT": 600,
}
mock_context, mock_page = self._make_pw_mocks(mock_browser_manager)
- # First item is consumed by the pre-capture scrollable-content
expansion.
- mock_page.evaluate.side_effect = [None, 25, 500, [], []]
+
+ def evaluate_side_effect(script, *_args):
+ if script ==
'document.querySelectorAll(".chart-container").length':
+ return 25
+ if "const target = document.querySelector" in script:
+ return 500
+ if "dashboard-component-chart-holder" in script:
+ return []
+ return None
+
+ mock_page.evaluate.side_effect = evaluate_side_effect
with patch.object(WebDriverPlaywright, "auth",
return_value=mock_context):
result = WebDriverPlaywright("chrome").get_screenshot(
@@ -1885,8 +2036,6 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
mock_page.screenshot.return_value = b"fallback"
with patch.object(WebDriverPlaywright, "auth",
return_value=mock_context):
- # match= keeps this assertion meaningful even when playwright
- # is not installed and PlaywrightTimeout aliases bare Exception.
with pytest.raises(
PlaywrightTimeout, match="Tiled screenshot failed for url"
):
