Compare commits

...
Author SHA1 Message Date
Elizabeth Thompson 47f41ffa29 fix(reports): treat still-virtualized holders as not-ready
Drop "virtualized" from TERMINAL_CHART_HOLDER_STATES so an off-screen holder
that never mounted or painted is not counted as a ready/terminal state.

#42901 grows the standalone-capture viewport so off-screen holders mount, but a
holder still virtualized at readiness time -- e.g. a dashboard taller than the
capture viewport cap -- was previously counted terminal and could ship a blank
region. Treating it as not-ready makes the readiness gate fail loud (via the
webdriver unready check) and drops semantic_success rather than silently
delivering a partially blank capture.

virtualized_holders is still reported directly from the state counts, so the
diagnostic logging is unchanged; only ready/unready/semantic_success shift.
2026-08-15 01:38:24 +00:00
Elizabeth ThompsonandClaude 0e44fd6625 fix(reports): guard per-tile diagnostics and emit semantic status once per capture
Review fixes:
- Wrap the per-tile diagnostics evaluate on the readiness-succeeded path
  (same guard as the final block: diagnostics must not discard valid
  tiles). A navigation race or renderer crash during the diagnostics
  call no longer fails a capture whose tiles are already good.
- Emit report_semantic_status once per capture (final block only); the
  per-tile report_readiness_tile INFO lines already carry error_holders,
  and one error chart spanning N tiles was producing N+1 WARNINGs.
- Mixed-state tests pin the field-to-slot mapping of the enriched lines
  (2 rendered + 1 empty + 1 error + 1 unready), the once-per-capture
  semantic status, and the diagnostics-failure regression.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-15 01:38:24 +00:00
Elizabeth ThompsonandClaude 9bbf97b60d feat(reports): structured chart-holder diagnostics parity for capture logging
Enriches the report readiness/capture log lines with the per-state
holder breakdown proven out in production:

- ChartHolderDiagnostics (report_execution.py): structured counts
  separating capture readiness from semantic success, with
  TERMINAL_CHART_HOLDER_STATES and the explicit
  CHART_HOLDER_SEMANTIC_POLICY=deliver_terminal_errors_with_warning.
- report_readiness_poll/_ready/_terminal now carry rendered/empty/
  error/virtualized/unready counts and semantic_success on both the
  Playwright and Selenium paths.
- report_readiness_tile: per-tile readiness diagnostics during tiled
  capture, with each holder's strongest observed terminal state
  aggregated across tiles for the final ready line.
- report_semantic_status: a WARNING whenever capture readiness is
  satisfied but the artifact contains terminal chart errors — delivery
  is not semantic completeness, and this makes the distinction
  operator-visible (and alertable) per tile and per capture.
- Restores the explicit zero-holder thumbnail diagnostic on the
  no-context path.

Logging-only: no readiness predicate, budget, or delivery behavior
changes. Unit tests pin the diagnostics counts and the enriched line
formats.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-15 01:38:24 +00:00
6 changed files with 538 additions and 75 deletions
+67
View File
@@ -20,6 +20,7 @@ from __future__ import annotations
import logging
import time
from collections import Counter
from collections.abc import Callable, Mapping
from dataclasses import dataclass, field
from typing import Any
@@ -27,6 +28,17 @@ from uuid import UUID
logger = logging.getLogger(__name__)
# A holder counts as terminal/ready only if it reached a delivered outcome: a
# painted chart (``rendered``), a legitimate ``empty`` result, or an ``error``.
# ``virtualized`` (off-screen, never mounted or painted) is deliberately NOT
# terminal: counting it as ready let blank reports ship when charts sat below the
# fold. #42901 grows the capture viewport so off-screen holders mount, but a
# holder still ``virtualized`` at readiness time -- e.g. a dashboard taller than
# the standalone-capture viewport cap -- must be treated as not-ready and fail
# loud rather than counted terminal and shipped blank.
TERMINAL_CHART_HOLDER_STATES = frozenset({"rendered", "empty", "error"})
CHART_HOLDER_SEMANTIC_POLICY = "deliver_terminal_errors_with_warning"
# Minimum working allowance kept above the summed phase reserves when a
# per-schedule working_timeout would otherwise squeeze the effective budget
# below what the execution context can represent. A floored budget still
@@ -235,6 +247,61 @@ class ReportExecutionContext:
return self.delivery_reserve_seconds + self.cleanup_reserve_seconds
@dataclass(frozen=True)
class ChartHolderDiagnostics:
"""Structured counts separating capture readiness from semantic success."""
mounted_holders: int
ready_holders: int
rendered_holders: int
empty_holders: int
error_holders: int
virtualized_holders: int
unready_holders: int
@property
def semantic_success(self) -> bool:
"""
Report whether every observed holder completed without a chart error.
An error holder is terminal for browser readiness, but it is not a
semantically correct chart. The staging policy may still deliver the
artifact, with this value and a warning making that distinction explicit.
"""
return (
self.mounted_holders > 0
and self.error_holders == 0
and self.unready_holders == 0
)
@classmethod
def from_holder_states(cls, holder_states: object) -> ChartHolderDiagnostics:
"""Build terminal-state counts from browser diagnostic results."""
if not isinstance(holder_states, list):
holder_states = []
state_counts: Counter[str] = Counter(
state
for holder in holder_states
if isinstance(holder, dict)
and isinstance((state := holder.get("state")), str)
)
mounted_holders = len(holder_states)
ready_holders = sum(
state_counts[state] for state in TERMINAL_CHART_HOLDER_STATES
)
return cls(
mounted_holders=mounted_holders,
ready_holders=ready_holders,
rendered_holders=state_counts["rendered"],
empty_holders=state_counts["empty"],
error_holders=state_counts["error"],
virtualized_holders=state_counts["virtualized"],
unready_holders=max(0, mounted_holders - ready_holders),
)
def get_report_task_timeout_options(
*,
is_report: bool,
+148 -17
View File
@@ -26,6 +26,8 @@ from celery import current_task
from PIL import Image
from superset.utils.report_execution import (
CHART_HOLDER_SEMANTIC_POLICY,
ChartHolderDiagnostics,
ReportExecutionBudgetExceededError,
ReportExecutionContext,
)
@@ -369,6 +371,7 @@ def take_tiled_screenshot( # noqa: C901
readiness_timeout = False
if screenshot_started_at is None:
screenshot_started_at = time.monotonic()
observed_holder_states: dict[str, str] = {}
task_budget = (
None
if report_execution_context
@@ -414,6 +417,35 @@ def take_tiled_screenshot( # noqa: C901
return remaining
return min(float(requested_seconds), remaining)
def _record_visible_holder_states(
holder_states: object,
*,
tile_number: int,
) -> None:
"""Record each report holder's strongest visible terminal state."""
if not isinstance(holder_states, list):
return
state_priority = {"rendered": 1, "empty": 2, "error": 3}
for position, holder in enumerate(holder_states):
if not isinstance(holder, dict):
continue
state = holder.get("state")
if not isinstance(state, str) or state not in state_priority:
continue
chart_id = holder.get("chartId")
holder_key = (
f"chart:{chart_id}"
if chart_id is not None
else f"tile:{tile_number}:holder:{position}"
)
existing_state = observed_holder_states.get(holder_key)
if (
existing_state is None
or state_priority[state] > state_priority[existing_state]
):
observed_holder_states[holder_key] = state
try:
# Get the target element
element = page.locator(f".{element_name}")
@@ -442,16 +474,28 @@ def take_tiled_screenshot( # noqa: C901
)
except PlaywrightTimeout:
holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS)
diagnostics = ChartHolderDiagnostics.from_holder_states(holder_states)
elapsed, remaining = _deadline_values()
logger.warning(
"report_readiness_terminal url=%s expected_holders=%s "
"mounted_holders=%s ready_holders=0 elapsed_seconds=%.2f "
"remaining_seconds=%s effective_wait_seconds=%.2f%s "
"mounted_holders=%s ready_holders=%s rendered_holders=%s "
"empty_holders=%s error_holders=%s virtualized_holders=%s "
"unready_holders=%s semantic_success=%s semantic_policy=%s "
"elapsed_seconds=%.2f remaining_seconds=%s "
"effective_wait_seconds=%.2f%s "
"terminal_reason=zero_holders_timeout states=%s; "
"aborting before dimensions, capture, or delivery",
url,
report_execution_context.expected_chart_count,
len(holder_states),
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
diagnostics.semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
elapsed,
f"{remaining:.2f}" if remaining is not None else None,
mount_wait,
@@ -577,10 +621,7 @@ def take_tiled_screenshot( # noqa: C901
tile_elapsed = time.monotonic() - tile_wait_start
unready_chart_holders = page.evaluate(FIND_UNREADY_CHART_HOLDERS_JS)
holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS)
ready_states = {"rendered", "empty", "error", "virtualized"}
ready_holders = sum(
holder.get("state") in ready_states for holder in holder_states
)
diagnostics = ChartHolderDiagnostics.from_holder_states(holder_states)
elapsed, remaining = _deadline_values()
# A chart failing to load in time is a customer chart-loading
# issue (slow query, error state, etc.), not a Superset system
@@ -589,20 +630,30 @@ def take_tiled_screenshot( # noqa: C901
# made the same call for the other screenshot timeout paths.
logger.warning(
"report_readiness_terminal url=%s expected_holders=%s "
"mounted_holders=%s ready_holders=%s tile=%s/%s "
"tiles_captured=%s/%s "
"mounted_holders=%s ready_holders=%s rendered_holders=%s "
"empty_holders=%s error_holders=%s virtualized_holders=%s "
"unready_holders=%s semantic_success=%s semantic_policy=%s "
"tile=%s/%s tiles_captured=%s/%s "
"tile_elapsed_seconds=%.2f elapsed_seconds=%.2f "
"remaining_seconds=%s effective_wait_seconds=%.2f%s "
"terminal_reason=readiness_timeout unready_holders=%s "
"states=%s; aborting before capture or delivery",
"terminal_reason=readiness_timeout "
"unready_holder_states=%s states=%s; "
"aborting before capture or delivery",
url,
(
report_execution_context.expected_chart_count
if report_execution_context
else None
),
len(holder_states),
ready_holders,
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
diagnostics.semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
i + 1,
num_tiles,
len(screenshot_tiles),
@@ -619,6 +670,52 @@ def take_tiled_screenshot( # noqa: C901
raise
else:
tile_elapsed = time.monotonic() - tile_wait_start
if report_execution_context:
try:
holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS)
if not isinstance(holder_states, list):
holder_states = []
except Exception: # noqa: BLE001 # diagnostics must not discard valid tiles
logger.warning(
"Unable to collect per-tile chart-holder diagnostics%s",
context_suffix,
exc_info=True,
)
holder_states = []
diagnostics = ChartHolderDiagnostics.from_holder_states(
holder_states
)
_record_visible_holder_states(
holder_states,
tile_number=i + 1,
)
elapsed, remaining = _deadline_values()
logger.info(
"report_readiness_tile url=%s expected_holders=%s "
"mounted_holders=%s ready_holders=%s rendered_holders=%s "
"empty_holders=%s error_holders=%s "
"virtualized_holders=%s unready_holders=%s "
"semantic_success=%s semantic_policy=%s tile=%s/%s "
"tile_elapsed_seconds=%.2f elapsed_seconds=%.2f "
"remaining_seconds=%s%s",
url,
report_execution_context.expected_chart_count,
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
diagnostics.semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
i + 1,
num_tiles,
tile_elapsed,
elapsed,
f"{remaining:.2f}" if remaining is not None else None,
context_suffix,
)
logger.debug(
"Tile %s/%s chart holders ready after %.2fs "
"(effective_wait=%.2fs)%s",
@@ -760,23 +857,57 @@ def take_tiled_screenshot( # noqa: C901
exc_info=True,
)
holder_states = []
ready_states = {"rendered", "empty", "error", "virtualized"}
diagnostics = ChartHolderDiagnostics.from_holder_states(
(
[{"state": state} for state in observed_holder_states.values()]
if observed_holder_states
else holder_states
)
)
elapsed, remaining = _deadline_values()
logger.info(
"report_readiness_ready url=%s expected_holders=%s mounted_holders=%s "
"ready_holders=%s elapsed_seconds=%.2f remaining_seconds=%s%s",
"ready_holders=%s rendered_holders=%s empty_holders=%s "
"error_holders=%s virtualized_holders=%s unready_holders=%s "
"semantic_success=%s semantic_policy=%s elapsed_seconds=%.2f "
"remaining_seconds=%s%s",
url,
(
report_execution_context.expected_chart_count
if report_execution_context
else None
),
len(holder_states),
sum(holder.get("state") in ready_states for holder in holder_states),
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
diagnostics.semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
elapsed,
f"{remaining:.2f}" if remaining is not None else None,
context_suffix,
)
if diagnostics.error_holders:
logger.warning(
"report_semantic_status url=%s expected_holders=%s "
"rendered_holders=%s empty_holders=%s error_holders=%s "
"semantic_success=false semantic_policy=%s%s; "
"capture completed, but the report contains terminal chart errors",
url,
(
report_execution_context.expected_chart_count
if report_execution_context
else None
),
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
CHART_HOLDER_SEMANTIC_POLICY,
context_suffix,
)
logger.info("Combining screenshot tiles...%s", context_suffix)
combined_screenshot = combine_screenshot_tiles(
screenshot_tiles,
+99 -39
View File
@@ -28,7 +28,10 @@ from flask import current_app as app
from superset.extensions import machine_auth_provider_factory
from superset.utils.report_execution import (
CHART_HOLDER_SEMANTIC_POLICY,
ChartHolderDiagnostics,
ReportExecutionContext,
TERMINAL_CHART_HOLDER_STATES,
)
from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
@@ -271,23 +274,25 @@ class WebDriverPlaywright(WebDriverProxy):
if report_execution_context:
log_context = report_execution_context.log_context
context_suffix = f" [{log_context}]" if log_context else ""
ready_states = {"rendered", "empty", "error", "virtualized"}
initial_chart_holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS)
initial_unready_chart_holders = [
holder
for holder in initial_chart_holder_states
if holder.get("state") not in ready_states
if holder.get("state") not in TERMINAL_CHART_HOLDER_STATES
]
initial_diagnostics = ChartHolderDiagnostics.from_holder_states(
initial_chart_holder_states
)
initial_semantic_success = (
initial_diagnostics.semantic_success
if element_name == "standalone"
else None
)
expected_holders = (
report_execution_context.expected_chart_count
if report_execution_context
else None
)
initial_mounted_holders = len(initial_chart_holder_states)
initial_ready_holders = sum(
holder.get("state") in ready_states
for holder in initial_chart_holder_states
)
deadline = (
report_execution_context.deadline if report_execution_context else None
)
@@ -295,31 +300,50 @@ class WebDriverPlaywright(WebDriverProxy):
deadline_remaining = deadline.remaining_seconds if deadline else None
logger.info(
"report_readiness_poll url=%s expected_holders=%s mounted_holders=%s "
"ready_holders=%s elapsed_seconds=%s remaining_seconds=%s%s states=%s",
"ready_holders=%s rendered_holders=%s empty_holders=%s "
"error_holders=%s virtualized_holders=%s unready_holders=%s "
"semantic_success=%s semantic_policy=%s elapsed_seconds=%s "
"remaining_seconds=%s%s states=%s",
url,
expected_holders,
initial_mounted_holders,
initial_ready_holders,
initial_diagnostics.mounted_holders,
initial_diagnostics.ready_holders,
initial_diagnostics.rendered_holders,
initial_diagnostics.empty_holders,
initial_diagnostics.error_holders,
initial_diagnostics.virtualized_holders,
initial_diagnostics.unready_holders,
initial_semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
f"{deadline_elapsed:.2f}" if deadline_elapsed is not None else None,
f"{deadline_remaining:.2f}" if deadline_remaining is not None else None,
context_suffix,
initial_chart_holder_states,
)
if element_name == "standalone" and not initial_chart_holder_states:
logger.info(
"report_readiness_waiting_for_mount url=%s expected_holders=%s "
"mounted_holders=0 ready_holders=0 elapsed_seconds=%s "
"remaining_seconds=%s%s",
url,
expected_holders,
f"{deadline_elapsed:.2f}" if deadline_elapsed is not None else None,
(
f"{deadline_remaining:.2f}"
if deadline_remaining is not None
else None
),
context_suffix,
)
if report_execution_context:
logger.info(
"report_readiness_waiting_for_mount url=%s expected_holders=%s "
"mounted_holders=0 ready_holders=0 elapsed_seconds=%s "
"remaining_seconds=%s%s",
url,
expected_holders,
f"{deadline_elapsed:.2f}" if deadline_elapsed is not None else None,
(
f"{deadline_remaining:.2f}"
if deadline_remaining is not None
else None
),
context_suffix,
)
else:
# Keep the thumbnail contract and its explicit diagnostic.
# Empty dashboards remain valid thumbnail artifacts; scheduled
# reports pass a context and use the fail-closed predicate below.
logger.warning(
"dashboard capture proceeding with zero chart holders — "
"readiness gate inactive"
)
if initial_unready_chart_holders:
logger.info(
"Chart holders not ready before polling at url %s%s: %s",
@@ -424,11 +448,11 @@ class WebDriverPlaywright(WebDriverProxy):
unready_chart_holders = [
holder
for holder in chart_holder_states
if holder.get("state") not in ready_states
if holder.get("state") not in TERMINAL_CHART_HOLDER_STATES
]
mounted_holders = len(chart_holder_states)
ready_holders = sum(
holder.get("state") in ready_states for holder in chart_holder_states
diagnostics = ChartHolderDiagnostics.from_holder_states(chart_holder_states)
semantic_success = (
diagnostics.semantic_success if element_name == "standalone" else None
)
deadline_elapsed = deadline.elapsed_seconds if deadline else elapsed
deadline_remaining = (
@@ -436,14 +460,25 @@ class WebDriverPlaywright(WebDriverProxy):
)
logger.warning(
"report_readiness_terminal url=%s expected_holders=%s "
"mounted_holders=%s ready_holders=%s elapsed_seconds=%.2f "
"remaining_seconds=%s effective_wait_seconds=%.2f%s "
"terminal_reason=readiness_timeout unready_holders=%s states=%s; "
"mounted_holders=%s ready_holders=%s rendered_holders=%s "
"empty_holders=%s error_holders=%s virtualized_holders=%s "
"unready_holders=%s semantic_success=%s semantic_policy=%s "
"elapsed_seconds=%.2f remaining_seconds=%s "
"effective_wait_seconds=%.2f%s "
"terminal_reason=readiness_timeout "
"unready_holder_states=%s states=%s; "
"aborting before capture or delivery",
url,
expected_holders,
mounted_holders,
ready_holders,
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
deadline_elapsed,
(
f"{deadline_remaining:.2f}"
@@ -479,9 +514,9 @@ class WebDriverPlaywright(WebDriverProxy):
)
return
chart_holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS)
mounted_holders = len(chart_holder_states)
ready_holders = sum(
holder.get("state") in ready_states for holder in chart_holder_states
diagnostics = ChartHolderDiagnostics.from_holder_states(chart_holder_states)
semantic_success = (
diagnostics.semantic_success if element_name == "standalone" else None
)
deadline_elapsed = deadline.elapsed_seconds if deadline else elapsed
deadline_remaining = (
@@ -489,15 +524,40 @@ class WebDriverPlaywright(WebDriverProxy):
)
logger.info(
"report_readiness_ready url=%s expected_holders=%s mounted_holders=%s "
"ready_holders=%s elapsed_seconds=%.2f remaining_seconds=%s%s",
"ready_holders=%s rendered_holders=%s empty_holders=%s "
"error_holders=%s virtualized_holders=%s unready_holders=%s "
"semantic_success=%s semantic_policy=%s elapsed_seconds=%.2f "
"remaining_seconds=%s%s",
url,
expected_holders,
mounted_holders,
ready_holders,
diagnostics.mounted_holders,
diagnostics.ready_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
diagnostics.virtualized_holders,
diagnostics.unready_holders,
semantic_success,
CHART_HOLDER_SEMANTIC_POLICY,
deadline_elapsed,
(f"{deadline_remaining:.2f}" if deadline_remaining is not None else None),
context_suffix,
)
if diagnostics.error_holders:
logger.warning(
"report_semantic_status url=%s expected_holders=%s "
"rendered_holders=%s empty_holders=%s error_holders=%s "
"semantic_success=false semantic_policy=%s%s; "
"capture readiness is satisfied, but the report contains "
"terminal chart errors",
url,
expected_holders,
diagnostics.rendered_holders,
diagnostics.empty_holders,
diagnostics.error_holders,
CHART_HOLDER_SEMANTIC_POLICY,
context_suffix,
)
def get_screenshot( # pylint: disable=too-many-locals, too-many-statements # noqa: C901
self,
@@ -19,6 +19,7 @@ from uuid import UUID
import pytest
from superset.utils.report_execution import (
ChartHolderDiagnostics,
get_report_task_timeout_options,
MIN_REPORT_EXECUTION_WORK_SECONDS,
ReportExecutionBudgetExceededError,
@@ -44,6 +45,51 @@ def _report_config(**overrides: int) -> dict[str, int | bool]:
return config
def test_chart_holder_diagnostics_separate_terminal_errors_from_success() -> None:
diagnostics = ChartHolderDiagnostics.from_holder_states(
[
{"chartId": "1", "state": "rendered"},
{"chartId": "2", "state": "rendered"},
{"chartId": "3", "state": "empty"},
{"chartId": "4", "state": "error"},
{"chartId": "5", "state": "virtualized"},
]
)
assert diagnostics.mounted_holders == 5
# ``virtualized`` is no longer a terminal/ready state (it never painted), so
# only rendered + empty + error count as ready and the off-screen holder is
# unready.
assert diagnostics.ready_holders == 4
assert diagnostics.rendered_holders == 2
assert diagnostics.empty_holders == 1
assert diagnostics.error_holders == 1
assert diagnostics.virtualized_holders == 1
assert diagnostics.unready_holders == 1
assert diagnostics.semantic_success is False
def test_chart_holder_diagnostics_count_unready_holders() -> None:
diagnostics = ChartHolderDiagnostics.from_holder_states(
[
{"chartId": "1", "state": "rendered"},
{"chartId": "2", "state": "waiting_on_database"},
{"chartId": "3", "state": "nothing_mounted"},
]
)
assert diagnostics.ready_holders == 1
assert diagnostics.unready_holders == 2
assert diagnostics.semantic_success is False
def test_chart_holder_diagnostics_do_not_treat_zero_holders_as_success() -> None:
diagnostics = ChartHolderDiagnostics.from_holder_states([])
assert diagnostics.mounted_holders == 0
assert diagnostics.semantic_success is False
def test_report_deadline_derives_phase_timeout_from_one_clock() -> None:
clock_value = 100.0
deadline = ReportExecutionDeadline(
@@ -207,3 +253,26 @@ def test_report_execution_config_rejects_invalid_startup_values(
def test_report_execution_config_accepts_defaults() -> None:
validate_report_execution_config(_report_config())
def test_virtualized_holders_are_not_semantic_success() -> None:
# A blank report can ship when off-screen (``virtualized``) holders are
# counted terminal: 8 painted + 2 off-screen holders would log
# semantic_success=True and deliver a partially blank capture. ``virtualized``
# is not a ready state, so those holders stay unready and semantic_success is
# False. (#42901 grows the viewport so they normally mount; this is the
# defense-in-depth for holders still off-screen at readiness time.)
diagnostics = ChartHolderDiagnostics.from_holder_states(
[{"chartId": str(i), "state": "rendered"} for i in range(8)]
+ [
{"chartId": "8", "state": "virtualized"},
{"chartId": "9", "state": "virtualized"},
]
)
assert diagnostics.mounted_holders == 10
assert diagnostics.rendered_holders == 8
assert diagnostics.virtualized_holders == 2
assert diagnostics.ready_holders == 8
assert diagnostics.unready_holders == 2
assert diagnostics.semantic_success is False
+149 -13
View File
@@ -517,6 +517,139 @@ class TestTakeTiledScreenshot:
assert "dashboard-component-chart-holder" in js
assert call[1]["timeout"] == 30 * 1000
def test_tile_line_reports_mixed_holder_states(self, mock_page):
"""The per-tile enriched line must map each state to its own field —
a swap between (say) rendered and error counts must fail this test."""
mixed = [
{"chartId": "1", "state": "rendered"},
{"chartId": "2", "state": "rendered"},
{"chartId": "3", "state": "empty"},
{"chartId": "4", "state": "error"},
{"chartId": "5", "state": "waiting_on_database"},
]
mock_page.wait_for_function.side_effect = None
mock_page.wait_for_function.return_value = None
mock_page.evaluate.side_effect = [
{"height": 5000, "top": 100, "left": 50, "width": 800}, # dimensions
None,
mixed, # tile 1: scroll, per-tile diagnostics
None,
mixed, # tile 2
None,
mixed, # tile 3
mixed, # final diagnostics
]
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
take_tiled_screenshot(
mock_page,
"dashboard",
tile_height=2000,
load_wait=30,
report_execution_context=_report_context(),
)
tile_calls = [
call
for call in mock_logger.info.call_args_list
if call.args and call.args[0].startswith("report_readiness_tile")
]
assert len(tile_calls) == 3
message = tile_calls[0].args[0] % tile_calls[0].args[1:]
assert "mounted_holders=5" in message
assert "ready_holders=4" in message
assert "rendered_holders=2" in message
assert "empty_holders=1" in message
assert "error_holders=1" in message
assert "virtualized_holders=0" in message
assert "unready_holders=1" in message
assert "semantic_success=False" in message
def test_final_semantic_status_fires_exactly_once_for_error_holders(
self, mock_page
):
"""One error chart spanning every tile emits ONE report_semantic_status
WARNING for the capture (final block), not one per tile — the per-tile
INFO lines already carry error_holders."""
with_error = [
{"chartId": "1", "state": "rendered"},
{"chartId": "2", "state": "error"},
]
mock_page.wait_for_function.side_effect = None
mock_page.wait_for_function.return_value = None
mock_page.evaluate.side_effect = [
{"height": 5000, "top": 100, "left": 50, "width": 800},
None,
with_error,
None,
with_error,
None,
with_error,
with_error,
]
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
take_tiled_screenshot(
mock_page,
"dashboard",
tile_height=2000,
load_wait=30,
report_execution_context=_report_context(),
)
semantic_calls = [
call
for call in mock_logger.warning.call_args_list
if call.args and call.args[0].startswith("report_semantic_status")
]
assert len(semantic_calls) == 1
message = semantic_calls[0].args[0] % semantic_calls[0].args[1:]
assert "error_holders=1" in message
assert "semantic_success=false" in message
def test_per_tile_diagnostics_failure_does_not_discard_capture(self, mock_page):
"""Diagnostics must not discard valid tiles: a non-timeout evaluate
failure on the per-tile diagnostics path logs a warning and the
capture still succeeds."""
final_states = [{"chartId": "1", "state": "rendered"}]
mock_page.wait_for_function.side_effect = None
mock_page.wait_for_function.return_value = None
mock_page.evaluate.side_effect = [
{"height": 5000, "top": 100, "left": 50, "width": 800},
None,
RuntimeError("Execution context was destroyed"),
None,
RuntimeError("Execution context was destroyed"),
None,
RuntimeError("Execution context was destroyed"),
final_states,
]
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
with patch(
"superset.utils.screenshot_utils.combine_screenshot_tiles",
return_value=b"combined",
):
result = take_tiled_screenshot(
mock_page,
"dashboard",
tile_height=2000,
load_wait=30,
report_execution_context=_report_context(),
)
assert result == b"combined"
assert mock_page.screenshot.call_count == 3
assert any(
call.args
and call.args[0].startswith(
"Unable to collect per-tile chart-holder diagnostics"
)
for call in mock_logger.warning.call_args_list
)
def test_per_tile_readiness_timeout_raises_and_skips_capture(self, mock_page):
"""A per-tile readiness timeout raises and does not capture that tile.
@@ -559,21 +692,23 @@ class TestTakeTiledScreenshot:
mock_logger.error.assert_not_called()
mock_logger.warning.assert_called_once()
warning_args = mock_logger.warning.call_args[0]
assert "unready" in warning_args[0].lower()
assert warning_args[3] == 1 # mounted holders
assert warning_args[4] == 0 # ready holders
assert warning_args[5] == 1 # tile index
assert warning_args[6] == 3 # total tiles
assert warning_args[7] == 0 # tiles captured so far
assert warning_args[8] == 3 # total tiles
assert isinstance(warning_args[9], float) # tile elapsed
assert isinstance(warning_args[10], float) # total elapsed
assert warning_args[12] == 30 # effective wait
assert "capture_kind=report" in warning_args[13]
warning_message = warning_args[0] % warning_args[1:]
assert "mounted_holders=1" in warning_message
assert "ready_holders=0" in warning_message
assert "rendered_holders=0" in warning_message
assert "empty_holders=0" in warning_message
assert "error_holders=0" in warning_message
assert "unready_holders=1" in warning_message
assert "tile=1/3" in warning_message
assert "tiles_captured=0/3" in warning_message
assert "effective_wait_seconds=30.00" in warning_message
assert "capture_kind=report" in warning_message
# Diagnostic payload identifies chart id AND the state it's stuck in
# (spinner mounted vs nothing mounted vs waiting-on-database) so a
# slow query can be told apart from the virtualization race.
assert warning_args[14] == [{"chartId": "42", "state": "waiting_on_database"}]
assert (
"unready_holder_states=[{'chartId': '42', 'state': 'waiting_on_database'}]"
) in warning_message
def test_timeout_warning_includes_log_context(self, mock_page):
"""The log context (e.g. report execution id) is threaded through for
@@ -600,7 +735,8 @@ class TestTakeTiledScreenshot:
)
warning_args = mock_logger.warning.call_args[0]
assert warning_args[13] == " [execution_id=abc-123]"
warning_message = warning_args[0] % warning_args[1:]
assert " [execution_id=abc-123]" in warning_message
def test_chart_holder_with_nothing_mounted_blocks_wait(self, mock_page):
"""Regression test for the vacuous-pass race (PR #39895).
+6 -6
View File
@@ -398,9 +398,9 @@ class TestWebDriverPlaywrightErrorHandling:
assert "terminal_reason=readiness_timeout" in warning_call.args[0]
assert warning_call.args[1] == "http://example.com"
assert warning_call.args[3] == 1 # mounted holders
assert warning_call.args[4] == 0 # ready holders
assert warning_call.args[7] == 60
assert warning_call.args[9] == [{"chartId": "42", "state": "nothing_mounted"}]
assert warning_call.args[9] == 1 # unready holders
assert warning_call.args[16] == [{"chartId": "42", "state": "nothing_mounted"}]
assert warning_call.args[17] == [{"chartId": "42", "state": "nothing_mounted"}]
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver._browser_manager")
@@ -1058,7 +1058,7 @@ class TestWebDriverPlaywrightChartReadiness:
log_context="execution_id=abc-123",
)
assert mock_logger.warning.call_args.args[8] == " [execution_id=abc-123]"
assert mock_logger.warning.call_args.args[15] == " [execution_id=abc-123]"
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver._browser_manager")
@@ -1327,8 +1327,8 @@ class TestWebDriverPlaywrightChartReadiness:
diagnostics,
)
failure_args = mock_logger.warning.call_args.args
assert failure_args[9] == diagnostics
assert failure_args[10] == diagnostics
assert failure_args[16] == diagnostics
assert failure_args[17] == diagnostics
mock_page.locator.return_value.screenshot.assert_not_called()