From 9bbf97b60d2201231b3221a7dc3e306f9f56b82d Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 4 Aug 2026 18:25:36 +0000 Subject: [PATCH] feat(reports): structured chart-holder diagnostics parity for capture logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- superset/utils/report_execution.py | 59 ++++++ superset/utils/screenshot_utils.py | 173 ++++++++++++++++-- superset/utils/webdriver.py | 138 ++++++++++---- .../unit_tests/utils/test_report_execution.py | 43 +++++ .../unit_tests/utils/test_screenshot_utils.py | 29 +-- tests/unit_tests/utils/webdriver_test.py | 12 +- 6 files changed, 379 insertions(+), 75 deletions(-) diff --git a/superset/utils/report_execution.py b/superset/utils/report_execution.py index 690dfcaa454..ba4445db631 100644 --- a/superset/utils/report_execution.py +++ b/superset/utils/report_execution.py @@ -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,9 @@ from uuid import UUID logger = logging.getLogger(__name__) +TERMINAL_CHART_HOLDER_STATES = frozenset({"rendered", "empty", "error", "virtualized"}) +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 +239,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, diff --git a/superset/utils/screenshot_utils.py b/superset/utils/screenshot_utils.py index 121fc6b0443..4ab72f1ae0f 100644 --- a/superset/utils/screenshot_utils.py +++ b/superset/utils/screenshot_utils.py @@ -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,60 @@ def take_tiled_screenshot( # noqa: C901 raise else: tile_elapsed = time.monotonic() - tile_wait_start + if report_execution_context: + holder_states = page.evaluate(FIND_CHART_HOLDER_STATES_JS) + 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, + ) + 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 tile=%s/%s%s; " + "capture readiness is satisfied, but the tile " + "contains terminal chart errors", + url, + report_execution_context.expected_chart_count, + diagnostics.rendered_holders, + diagnostics.empty_holders, + diagnostics.error_holders, + CHART_HOLDER_SEMANTIC_POLICY, + i + 1, + num_tiles, + context_suffix, + ) logger.debug( "Tile %s/%s chart holders ready after %.2fs " "(effective_wait=%.2fs)%s", @@ -760,23 +865,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, diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index 18944937636..57b40c5162c 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -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, diff --git a/tests/unit_tests/utils/test_report_execution.py b/tests/unit_tests/utils/test_report_execution.py index 6c99d53482d..99c4cc530d6 100644 --- a/tests/unit_tests/utils/test_report_execution.py +++ b/tests/unit_tests/utils/test_report_execution.py @@ -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,48 @@ 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 + assert diagnostics.ready_holders == 5 + assert diagnostics.rendered_holders == 2 + assert diagnostics.empty_holders == 1 + assert diagnostics.error_holders == 1 + assert diagnostics.virtualized_holders == 1 + assert diagnostics.unready_holders == 0 + 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( diff --git a/tests/unit_tests/utils/test_screenshot_utils.py b/tests/unit_tests/utils/test_screenshot_utils.py index cb772e45d78..f1486b01ab4 100644 --- a/tests/unit_tests/utils/test_screenshot_utils.py +++ b/tests/unit_tests/utils/test_screenshot_utils.py @@ -559,21 +559,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 +602,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). diff --git a/tests/unit_tests/utils/webdriver_test.py b/tests/unit_tests/utils/webdriver_test.py index bc245a0ec36..0c06c279e4c 100644 --- a/tests/unit_tests/utils/webdriver_test.py +++ b/tests/unit_tests/utils/webdriver_test.py @@ -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()