You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2026/09/16 20:00:50 UTC
(superset) 21/34: fix(reports): fail closed after capture rejection (#44191)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch 7.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit b7f3e2690365e5366d7a00e834bbc29e15e93c4d
Author: Mafi <ma...@gmail.com>
AuthorDate: Tue Sep 15 09:06:31 2026 +1000
fix(reports): fail closed after capture rejection (#44191)
(cherry picked from commit 447544ea27e784b7331a442d8497d8dc3e1634d1)
---
UPDATING.md | 9 ++
docs/admin_docs/configuration/alerts-reports.mdx | 10 ++
superset/commands/report/execute.py | 33 +++++
superset/utils/report_execution.py | 23 ++++
superset/utils/screenshot_utils.py | 75 +++++++----
superset/utils/webdriver.py | 1 +
tests/integration_tests/reports/commands_tests.py | 7 +-
tests/unit_tests/commands/report/execute_test.py | 152 ++++++++++++++++++++++
tests/unit_tests/utils/test_report_execution.py | 19 +++
tests/unit_tests/utils/test_screenshot_utils.py | 76 +++++++++--
tests/unit_tests/utils/webdriver_test.py | 4 +-
11 files changed, 368 insertions(+), 41 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 09e3a0d948e..6f23f4ca519 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -53,6 +53,15 @@ Scheduled report and alert captures require chart readiness to remain stable
immediately before Chromium captures the image. A capture that re-enters a loading
state during that window fails instead of delivering a screenshot with spinners.
+### Scheduled rendered reports fail closed after capture rejection
+
+Scheduled PDF and PNG delivery requires an accepted report capture context. A
+terminal rejection from standard, tiled, or combined-image validation is sticky
+for that execution and is checked again at the notification boundary. Reports and
+alerts cannot deliver a rendered artifact after rejection, even if an intermediate
+capture layer accidentally catches the original error. Text-only failure
+notifications and thumbnail capture behavior are unchanged.
+
### Resample "Fill the entire time range"
Charts with Resample can enable **Fill the entire time range** so gap-filling
diff --git a/docs/admin_docs/configuration/alerts-reports.mdx b/docs/admin_docs/configuration/alerts-reports.mdx
index c6c8b4c9c0f..76f1196a80a 100644
--- a/docs/admin_docs/configuration/alerts-reports.mdx
+++ b/docs/admin_docs/configuration/alerts-reports.mdx
@@ -486,6 +486,16 @@ applies to alerts that attach reports. Check worker logs for
`ScreenshotBlankCaptureError` and `report_capture_validation` when investigating
these failures.
+Rendered report delivery is fail closed. A terminal rejection from the standard,
+per-tile, or combined-image validation stage is recorded on the report execution
+context and checked again immediately before every notification send. A PDF or PNG
+without an accepted capture context is also rejected. This applies equally to
+report schedules and alerts that attach rendered reports. Text-only failure
+notifications may still be sent. Look for `report_delivery_blocked` when this
+delivery-boundary safeguard is triggered. Thumbnail captures remain permissive and
+the retained-blank event is named `thumbnail_capture_blank_tile_retained` so it
+cannot be mistaken for a report delivery event.
+
Immediately before each scheduled report screenshot, chart readiness must remain
stable for 500 milliseconds, unless less than one stability window of the
execution budget is left, in which case the check is skipped and logged with
diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py
index 17548c26efe..9c0f77d0d97 100644
--- a/superset/commands/report/execute.py
+++ b/superset/commands/report/execute.py
@@ -1351,6 +1351,7 @@ class BaseReportState:
recipient: ReportRecipients,
) -> None:
"""Send one notification, upgrading Slack v1 recipients when required."""
+ self._assert_rendered_delivery_allowed(notification_content)
notification = create_notification(recipient, notification_content)
if app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"]:
logger.info(
@@ -1375,6 +1376,37 @@ class BaseReportState:
notification.send()
+ def _assert_rendered_delivery_allowed(
+ self,
+ notification_content: NotificationContent,
+ ) -> None:
+ """Enforce fail-closed capture provenance before rendered delivery."""
+
+ if not (notification_content.pdf or notification_content.screenshots):
+ return
+
+ report_context = getattr(self, "_report_execution_context", None)
+ rejection_reasons = (
+ report_context.capture_rejection_reasons
+ if report_context is not None
+ else ("missing_capture_context",)
+ )
+ if report_context is not None and not report_context.capture_was_rejected:
+ return
+
+ logger.error(
+ "report_delivery_blocked %s terminal_reason=capture_rejected "
+ "rejection_reasons=%s",
+ report_context.log_context
+ if report_context is not None
+ else self._log_context,
+ ",".join(rejection_reasons),
+ )
+ raise ReportScheduleScreenshotFailedError(
+ "Rendered report delivery blocked because capture validation "
+ "did not produce an accepted artifact"
+ )
+
def _send(
self,
notification_content: NotificationContent,
@@ -1389,6 +1421,7 @@ class BaseReportState:
upgraded_delivery_failed = False
self._slack_v1_upgrade.reset()
report_context = getattr(self, "_report_execution_context", None)
+ self._assert_rendered_delivery_allowed(notification_content)
for recipient in recipients:
try:
log_report_delivery_phase(
diff --git a/superset/utils/report_execution.py b/superset/utils/report_execution.py
index 690dfcaa454..7f91b48e41c 100644
--- a/superset/utils/report_execution.py
+++ b/superset/utils/report_execution.py
@@ -190,6 +190,12 @@ class ReportExecutionContext:
capture_reserve_seconds: float = 0.0
delivery_reserve_seconds: float = 0.0
cleanup_reserve_seconds: float = 0.0
+ _capture_rejection_reasons: list[str] = field(
+ default_factory=list,
+ init=False,
+ compare=False,
+ repr=False,
+ )
def __post_init__(self) -> None:
"""Validate that configured phase reserves fit inside the deadline."""
@@ -234,6 +240,23 @@ class ReportExecutionContext:
return self.delivery_reserve_seconds + self.cleanup_reserve_seconds
+ def reject_capture(self, reason: str) -> None:
+ """Permanently disqualify this execution's rendered artifact delivery."""
+
+ self._capture_rejection_reasons.append(reason)
+
+ @property
+ def capture_rejection_reasons(self) -> tuple[str, ...]:
+ """Return immutable reasons recorded by terminal capture validation."""
+
+ return tuple(self._capture_rejection_reasons)
+
+ @property
+ def capture_was_rejected(self) -> bool:
+ """Return whether any capture stage terminally rejected its output."""
+
+ return bool(self._capture_rejection_reasons)
+
def get_report_task_timeout_options(
*,
diff --git a/superset/utils/screenshot_utils.py b/superset/utils/screenshot_utils.py
index 8f98528d3b9..69b59a43716 100644
--- a/superset/utils/screenshot_utils.py
+++ b/superset/utils/screenshot_utils.py
@@ -945,7 +945,7 @@ def take_tiled_screenshot( # noqa: C901
screenshot_tiles: list[bytes] = []
blank_tile_retries = 0
- contentful_tiles_captured = 0
+ contentful_tile_indexes: list[int] = []
def _raise_if_budget_exhausted() -> None:
elapsed, remaining = _deadline_values()
@@ -1349,13 +1349,16 @@ def take_tiled_screenshot( # noqa: C901
# Exhausted retries must never turn a rejected report
# tile into accepted content. Only thumbnails may
# retain a blank capture below.
+ report_execution_context.reject_capture(
+ f"blank_tile:{i + 1}/{num_tiles}"
+ )
raise ScreenshotBlankCaptureError(
"Chromium returned a blank tile "
f"{i + 1}/{num_tiles} after {capture_attempt} attempts"
)
tile_screenshot = candidate
logger.warning(
- "report_capture_blank_tile_retained tile=%s/%s "
+ "thumbnail_capture_blank_tile_retained tile=%s/%s "
"attempts=%s contentful_chart_holders=%s "
"dominant_pixel_ratio=%.5f near_white_pixel_ratio=%.5f "
"mean_luminance=%.2f luminance_stddev=%.2f entropy=%.3f "
@@ -1414,7 +1417,7 @@ 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
+ contentful_tile_indexes.append(i)
logger.debug(
"Captured tile %s/%s with clip %s%s",
@@ -1470,30 +1473,54 @@ 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:
+ if report_execution_context and contentful_tile_indexes:
+ contentful_indexes = set(contentful_tile_indexes)
+ rejected_combined_tiles: list[int] = []
+ y_offset = 0
+ with Image.open(io.BytesIO(combined_screenshot)) as combined_image:
+ for tile_index, tile in enumerate(screenshot_tiles):
+ with Image.open(io.BytesIO(tile)) as source_tile:
+ tile_width, tile_height = source_tile.size
+ if tile_index in contentful_indexes:
+ combined_region = combined_image.crop(
+ (0, y_offset, tile_width, y_offset + tile_height)
+ )
+ output = io.BytesIO()
+ combined_region.save(output, format="PNG")
+ blankness = get_screenshot_blankness_metrics(output.getvalue())
+ logger.info(
+ "report_capture_validation capture=combined_region "
+ "tile=%s/%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",
+ tile_index + 1,
+ num_tiles,
+ blankness.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 blankness.is_blank:
+ rejected_combined_tiles.append(tile_index + 1)
+ y_offset += tile_height
+
+ if rejected_combined_tiles:
logger.warning(
- "report_capture_blank_combined_retained contentful_tiles=%s%s",
- contentful_tiles_captured,
+ "report_capture_blank_combined_rejected contentful_tiles=%s "
+ "rejected_tiles=%s%s",
+ len(contentful_tile_indexes),
+ rejected_combined_tiles,
context_suffix,
)
+ report_execution_context.reject_capture("blank_combined")
+ raise ScreenshotBlankCaptureError(
+ "Combined report screenshot lost content from validated tiles"
+ )
return combined_screenshot
diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py
index 85ade1b2b58..0cac9d53b04 100644
--- a/superset/utils/webdriver.py
+++ b/superset/utils/webdriver.py
@@ -380,6 +380,7 @@ class WebDriverPlaywright(WebDriverProxy):
context_suffix,
)
if attempt == TILED_SCREENSHOT_MAX_CAPTURE_ATTEMPTS:
+ report_execution_context.reject_capture("blank_standard")
raise ScreenshotBlankCaptureError(
"Chromium returned a blank standard screenshot "
f"after {attempt} attempts"
diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py
index 01654e6f2a5..6490ab4b351 100644
--- a/tests/integration_tests/reports/commands_tests.py
+++ b/tests/integration_tests/reports/commands_tests.py
@@ -84,6 +84,7 @@ from superset.reports.models import (
ReportScheduleValidatorType,
ReportState,
)
+from superset.reports.notifications.base import NotificationContent
from superset.reports.notifications.exceptions import (
NotificationError,
NotificationParamException,
@@ -3101,7 +3102,7 @@ def test_prune_log_soft_time_out(bulk_delete_logs, create_report_email_dashboard
@patch("superset.commands.report.execute.logger")
@patch("superset.commands.report.execute.create_notification")
def test__send_with_client_errors(notification_mock, logger_mock):
- notification_content = "I am some content"
+ notification_content = NotificationContent(name="I am some content", header_data={})
recipients = ["test@foo.com"]
report_state = BaseReportState(ReportSchedule(), datetime.utcnow(), uuid4())
notification_mock.return_value.send.side_effect = NotificationParamException()
@@ -3117,7 +3118,7 @@ def test__send_with_client_errors(notification_mock, logger_mock):
@patch("superset.commands.report.execute.logger")
@patch("superset.commands.report.execute.create_notification")
def test__send_with_multiple_errors(notification_mock, logger_mock):
- notification_content = "I am some content"
+ notification_content = NotificationContent(name="I am some content", header_data={})
recipients = ["test@foo.com", "test2@bar.com"]
report_state = BaseReportState(ReportSchedule(), datetime.utcnow(), uuid4())
notification_mock.return_value.send.side_effect = [
@@ -3145,7 +3146,7 @@ def test__send_with_multiple_errors(notification_mock, logger_mock):
@patch("superset.commands.report.execute.logger")
@patch("superset.commands.report.execute.create_notification")
def test__send_with_server_errors(notification_mock, logger_mock):
- notification_content = "I am some content"
+ notification_content = NotificationContent(name="I am some content", header_data={})
recipients = ["test@foo.com"]
report_state = BaseReportState(ReportSchedule(), datetime.utcnow(), uuid4())
notification_mock.return_value.send.side_effect = NotificationError()
diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py
index 15d14decec1..a5e7c710dbf 100644
--- a/tests/unit_tests/commands/report/execute_test.py
+++ b/tests/unit_tests/commands/report/execute_test.py
@@ -2906,6 +2906,7 @@ def test_send_preserves_transient_upgrade_failure_for_file_reports(
"January 1, 2021",
"execution_id_example",
)
+ report_state._report_execution_context = _active_report_context()
notification_content = NotificationContent(
name="Private channel report",
header_data={
@@ -2987,6 +2988,7 @@ def test_send_classifies_probe_failure_for_file_reports(
"January 1, 2021",
"execution_id_example",
)
+ report_state._report_execution_context = _active_report_context()
notification_content = NotificationContent(
name="Private channel report",
header_data={
@@ -3036,6 +3038,7 @@ def test_send_classifies_malformed_file_recipient_as_client_error(
"January 1, 2021",
"execution_id_example",
)
+ report_state._report_execution_context = _active_report_context()
notification_content = NotificationContent(
name="Private channel report",
header_data={
@@ -3091,6 +3094,7 @@ def test_send_does_not_fall_back_to_slack_v1_for_file_uploads(
"January 1, 2021",
"execution_id_example",
)
+ report_state._report_execution_context = _active_report_context()
notification_content = NotificationContent(
name="Private channel report",
header_data={
@@ -3255,6 +3259,18 @@ def _make_notification_state(
return state
+def _active_report_context() -> ReportExecutionContext:
+ return ReportExecutionContext(
+ execution_id=UUID("084e7ee6-5557-4ecd-9632-b7f39c9ec524"),
+ report_schedule_id=11,
+ deadline=ReportExecutionDeadline(
+ total_seconds=900,
+ started_at=0,
+ _clock=lambda: 0,
+ ),
+ )
+
+
@patch("superset.commands.report.execute.feature_flag_manager")
def test_get_notification_content_png_screenshot(
mock_ff, mocker: MockerFixture
@@ -3275,6 +3291,7 @@ def test_slack_retry_deadline_flows_from_report_state_to_transport(
"""One absolute execution deadline reaches every Slack v2 destination."""
app.config["ALERT_REPORTS_NOTIFICATION_DRY_RUN"] = False
state = _make_notification_state(mocker, report_format=ReportDataFormat.PNG)
+ state._report_execution_context = _active_report_context()
mocker.patch.object(state, "_get_screenshots", return_value=[b"img"])
deadline_factory = mocker.patch(
"superset.commands.report.execute.get_slack_send_retry_deadline",
@@ -3297,6 +3314,141 @@ def test_slack_retry_deadline_flows_from_report_state_to_transport(
assert send_to_channels.call_args.kwargs["retry_deadline"] == 123.0
+@pytest.mark.parametrize(
+ ("schedule_type", "report_format", "content"),
+ [
+ (
+ ReportScheduleType.REPORT,
+ ReportDataFormat.PNG,
+ NotificationContent(
+ name="report",
+ header_data=_make_notification_header(),
+ screenshots=[b"png"],
+ ),
+ ),
+ (
+ ReportScheduleType.REPORT,
+ ReportDataFormat.PDF,
+ NotificationContent(
+ name="report", header_data=_make_notification_header(), pdf=b"%PDF"
+ ),
+ ),
+ (
+ ReportScheduleType.ALERT,
+ ReportDataFormat.PNG,
+ NotificationContent(
+ name="alert",
+ header_data=_make_notification_header(),
+ screenshots=[b"png"],
+ ),
+ ),
+ (
+ ReportScheduleType.ALERT,
+ ReportDataFormat.PDF,
+ NotificationContent(
+ name="alert", header_data=_make_notification_header(), pdf=b"%PDF"
+ ),
+ ),
+ ],
+)
+def test_rejected_capture_blocks_every_rendered_delivery_path(
+ mocker: MockerFixture,
+ schedule_type: ReportScheduleType,
+ report_format: ReportDataFormat,
+ content: NotificationContent,
+) -> None:
+ state = _make_notification_state(
+ mocker,
+ report_format=report_format,
+ schedule_type=schedule_type,
+ )
+ report_context = _active_report_context()
+ report_context.reject_capture("blank_combined")
+ state._report_execution_context = report_context
+ recipient = mocker.Mock(spec=ReportRecipients)
+ send_notification = mocker.patch.object(state, "_send_notification")
+
+ with pytest.raises(
+ ReportScheduleScreenshotFailedError,
+ match="capture validation did not produce an accepted artifact",
+ ):
+ state._send(content, [recipient])
+
+ send_notification.assert_not_called()
+
+
+def test_rejected_capture_does_not_block_text_failure_notification(
+ mocker: MockerFixture,
+) -> None:
+ state = _make_notification_state(mocker, report_format=ReportDataFormat.PDF)
+ report_context = _active_report_context()
+ report_context.reject_capture("blank_combined")
+ state._report_execution_context = report_context
+ recipient = mocker.Mock(spec=ReportRecipients)
+ send_notification = mocker.patch.object(state, "_send_notification")
+ content = NotificationContent(
+ name="failed report",
+ header_data=_make_notification_header(),
+ text="The report capture failed",
+ )
+
+ state._send(content, [recipient])
+
+ send_notification.assert_called_once_with(content, recipient)
+
+
+@pytest.mark.parametrize(
+ "content",
+ [
+ NotificationContent(
+ name="report",
+ header_data=_make_notification_header(),
+ screenshots=[b"png"],
+ ),
+ NotificationContent(
+ name="report", header_data=_make_notification_header(), pdf=b"%PDF"
+ ),
+ ],
+ ids=["png", "pdf"],
+)
+def test_rendered_delivery_without_capture_context_fails_closed(
+ mocker: MockerFixture,
+ content: NotificationContent,
+) -> None:
+ state = _make_notification_state(mocker)
+ recipient = mocker.Mock(spec=ReportRecipients)
+ send_notification = mocker.patch.object(state, "_send_notification")
+
+ with pytest.raises(
+ ReportScheduleScreenshotFailedError,
+ match="capture validation did not produce an accepted artifact",
+ ):
+ state._send(content, [recipient])
+
+ send_notification.assert_not_called()
+
+
+def test_direct_notification_send_cannot_bypass_capture_rejection(
+ mocker: MockerFixture,
+) -> None:
+ state = _make_notification_state(mocker)
+ report_context = _active_report_context()
+ report_context.reject_capture("blank_tile:1/2")
+ state._report_execution_context = report_context
+ recipient = mocker.Mock(spec=ReportRecipients)
+ create_notification = mocker.patch(
+ "superset.commands.report.execute.create_notification"
+ )
+ content = NotificationContent(
+ name="report", header_data=_make_notification_header(), pdf=b"%PDF"
+ )
+
+ with pytest.raises(ReportScheduleScreenshotFailedError):
+ state._send_notification(content, recipient)
+
+ create_notification.assert_not_called()
+
+
def test_slack_retry_deadline_clamps_elapsed_working_timeout(
mocker: MockerFixture,
) -> None:
diff --git a/tests/unit_tests/utils/test_report_execution.py b/tests/unit_tests/utils/test_report_execution.py
index 6c99d53482d..beb08c19b5a 100644
--- a/tests/unit_tests/utils/test_report_execution.py
+++ b/tests/unit_tests/utils/test_report_execution.py
@@ -117,6 +117,25 @@ def test_report_context_rejects_reserves_that_consume_deadline() -> None:
)
+def test_capture_rejection_is_sticky_for_the_execution() -> None:
+ context = ReportExecutionContext(
+ execution_id=UUID("084e7ee6-5557-4ecd-9632-b7f39c9ec524"),
+ report_schedule_id=7,
+ deadline=ReportExecutionDeadline(total_seconds=900),
+ )
+
+ assert context.capture_was_rejected is False
+
+ context.reject_capture("blank_tile:1/2")
+ context.reject_capture("blank_combined")
+
+ assert context.capture_was_rejected is True
+ assert context.capture_rejection_reasons == (
+ "blank_tile:1/2",
+ "blank_combined",
+ )
+
+
def test_report_deadline_rejects_nonpositive_budget() -> None:
with pytest.raises(ValueError, match="greater than zero"):
ReportExecutionDeadline(total_seconds=0)
diff --git a/tests/unit_tests/utils/test_screenshot_utils.py b/tests/unit_tests/utils/test_screenshot_utils.py
index 943ee8441e7..c7187cef183 100644
--- a/tests/unit_tests/utils/test_screenshot_utils.py
+++ b/tests/unit_tests/utils/test_screenshot_utils.py
@@ -490,7 +490,7 @@ class TestTakeTiledScreenshot:
with (
patch(
"superset.utils.screenshot_utils.combine_screenshot_tiles",
- return_value=b"combined",
+ return_value=chart_tile,
) as mock_combine,
patch("superset.utils.screenshot_utils.logger") as mock_logger,
):
@@ -501,7 +501,7 @@ class TestTakeTiledScreenshot:
report_execution_context=_report_context(),
)
- assert result == b"combined"
+ assert result == chart_tile
mock_combine.assert_called_once()
combined_tiles = mock_combine.call_args.args[0]
assert len(combined_tiles) == 1
@@ -539,6 +539,7 @@ class TestTakeTiledScreenshot:
mock_page.evaluate.side_effect = evaluate
mock_page.screenshot.return_value = _png(800, 1000, "white")
+ report_context = _report_context()
with (
patch("superset.utils.screenshot_utils.logger"),
@@ -554,13 +555,14 @@ class TestTakeTiledScreenshot:
mock_page,
"dashboard",
tile_height=1000,
- report_execution_context=_report_context(),
+ report_execution_context=report_context,
)
assert mock_page.screenshot.call_count == 3
+ assert report_context.capture_rejection_reasons == ("blank_tile:1/2",)
mock_combine.assert_not_called()
- def test_blank_combined_image_is_advisory_after_contentful_tiles_pass(
+ def test_blank_combined_image_fails_closed_after_contentful_tiles_pass(
self, mock_page
):
element_info = {"height": 1000, "top": 0, "left": 0, "width": 800}
@@ -577,27 +579,73 @@ class TestTakeTiledScreenshot:
mock_page.evaluate.side_effect = evaluate
mock_page.screenshot.return_value = self._create_chart_like_tile()
+ report_context = _report_context()
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,
+ pytest.raises(
+ ScreenshotBlankCaptureError,
+ match="Combined report screenshot lost content",
+ ),
):
- result = take_tiled_screenshot(
+ take_tiled_screenshot(
mock_page,
"dashboard",
tile_height=1000,
- report_execution_context=_report_context(),
+ report_execution_context=report_context,
)
- assert result == _two_tone_blank(800, 1000)
+ assert report_context.capture_rejection_reasons == ("blank_combined",)
assert any(
- call.args[0].startswith("report_capture_blank_combined_retained")
+ call.args[0].startswith("report_capture_blank_combined_rejected")
and call.args[1] == 1
+ and call.args[2] == [1]
for call in mock_logger.warning.call_args_list
)
+ def test_sparse_contentful_tile_is_not_diluted_by_empty_tiles(self, mock_page):
+ element_info = {"height": 8000, "top": 0, "left": 0, "width": 800}
+ holder_count_calls = 0
+
+ def evaluate(script, _arg=None):
+ nonlocal holder_count_calls
+ if "scrollWidth" in script:
+ return element_info
+ if script == CONTENTFUL_CHART_HOLDERS_IN_CLIP_JS:
+ holder_count_calls += 1
+ return {
+ "total": 1,
+ "contentful": 1 if holder_count_calls == 1 else 0,
+ }
+ if "requestAnimationFrame" in script or "window.scrollTo" in script:
+ return None
+ return [{"chartId": "7", "state": "rendered"}]
+
+ sparse_content = Image.new("RGB", (800, 1000), "white")
+ ImageDraw.Draw(sparse_content).rectangle((50, 50, 149, 149), fill=(64, 64, 64))
+ sparse_output = io.BytesIO()
+ sparse_content.save(sparse_output, format="PNG")
+ empty_tile = _png(800, 1000, "white")
+ mock_page.evaluate.side_effect = evaluate
+ mock_page.screenshot.side_effect = [sparse_output.getvalue()] + [empty_tile] * 7
+ report_context = _report_context()
+
+ result = take_tiled_screenshot(
+ mock_page,
+ "dashboard",
+ tile_height=1000,
+ report_execution_context=report_context,
+ )
+
+ # The whole image is statistically blank because valid empty-state tiles
+ # dominate it, but its content-bearing region remains valid.
+ assert get_screenshot_blankness_metrics(result).is_blank is True
+ assert report_context.capture_was_rejected is False
+ assert mock_page.screenshot.call_count == 8
+
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)
@@ -723,10 +771,11 @@ class TestTakeTiledScreenshot:
mock_page.evaluate.side_effect = evaluate
mock_page.screenshot.side_effect = screenshot
+ combined = self._create_chart_like_tile()
with patch(
"superset.utils.screenshot_utils.combine_screenshot_tiles",
- return_value=b"combined",
+ return_value=combined,
):
result = take_tiled_screenshot(
mock_page,
@@ -735,7 +784,7 @@ class TestTakeTiledScreenshot:
report_execution_context=context,
)
- assert result == b"combined"
+ assert result == combined
first_call, second_call = mock_page.screenshot.call_args_list
assert first_call.kwargs["timeout"] == 120_000
assert second_call.kwargs["timeout"] == 70_000
@@ -802,7 +851,7 @@ class TestTakeTiledScreenshot:
for call in mock_logger.warning.call_args_list
)
assert any(
- call.args[0].startswith("report_capture_blank_tile_retained")
+ call.args[0].startswith("thumbnail_capture_blank_tile_retained")
for call in mock_logger.warning.call_args_list
)
@@ -832,10 +881,11 @@ class TestTakeTiledScreenshot:
return None
mock_page.wait_for_function.side_effect = wait_for_function
+ combined = self._create_chart_like_tile()
with patch(
"superset.utils.screenshot_utils.combine_screenshot_tiles",
- return_value=b"combined",
+ return_value=combined,
):
result = take_tiled_screenshot(
mock_page,
@@ -844,7 +894,7 @@ class TestTakeTiledScreenshot:
report_execution_context=_report_context(),
)
- assert result == b"combined"
+ assert result == combined
assert any(
call.args == ("() => window.__supersetRepaintComplete === true",)
and call.kwargs == {"timeout": 5000.0}
diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py
index ac9641b9861..e3d5fc74333 100644
--- a/tests/unit_tests/utils/webdriver_test.py
+++ b/tests/unit_tests/utils/webdriver_test.py
@@ -164,6 +164,7 @@ class TestStandardScreenshotValidation:
page = MagicMock()
element = MagicMock()
page.screenshot.return_value = _png("white")
+ report_context = _report_context()
with pytest.raises(
ScreenshotBlankCaptureError,
@@ -174,10 +175,11 @@ class TestStandardScreenshotValidation:
element,
"standalone",
"execution_id=test",
- _report_context(),
+ report_context,
)
assert page.screenshot.call_count == 3
+ assert report_context.capture_rejection_reasons == ("blank_standard",)
def test_blank_empty_state_capture_is_allowed(self):
page = MagicMock()