Skip to content

Commit fe7f806

Browse files
fix: Enable Playwright migration with graceful Selenium fallback (#35063)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent dce7401 commit fe7f806

5 files changed

Lines changed: 781 additions & 4 deletions

File tree

superset/utils/screenshots.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@
4141

4242
logger = logging.getLogger(__name__)
4343

44+
# Import Playwright availability and install message
45+
try:
46+
from superset.utils.webdriver import (
47+
PLAYWRIGHT_AVAILABLE,
48+
PLAYWRIGHT_INSTALL_MESSAGE,
49+
)
50+
except ImportError:
51+
PLAYWRIGHT_AVAILABLE = False
52+
PLAYWRIGHT_INSTALL_MESSAGE = "Playwright module not found"
53+
54+
4455
DEFAULT_SCREENSHOT_WINDOW_SIZE = 800, 600
4556
DEFAULT_SCREENSHOT_THUMBNAIL_SIZE = 400, 300
4657
DEFAULT_CHART_WINDOW_SIZE = DEFAULT_CHART_THUMBNAIL_SIZE = 800, 600
@@ -169,7 +180,19 @@ def __init__(self, url: str, digest: str | None):
169180
def driver(self, window_size: WindowSize | None = None) -> WebDriver:
170181
window_size = window_size or self.window_size
171182
if feature_flag_manager.is_feature_enabled("PLAYWRIGHT_REPORTS_AND_THUMBNAILS"):
172-
return WebDriverPlaywright(self.driver_type, window_size)
183+
# Try to use Playwright if available (supports WebGL/DeckGL, unlike Cypress)
184+
if PLAYWRIGHT_AVAILABLE:
185+
return WebDriverPlaywright(self.driver_type, window_size)
186+
187+
# Playwright not available, falling back to Selenium
188+
logger.info(
189+
"PLAYWRIGHT_REPORTS_AND_THUMBNAILS enabled but Playwright not "
190+
"installed. Falling back to Selenium (WebGL/Canvas charts may "
191+
"not render correctly). %s",
192+
PLAYWRIGHT_INSTALL_MESSAGE,
193+
)
194+
195+
# Use Selenium as default/fallback
173196
return WebDriverSelenium(self.driver_type, window_size)
174197

175198
def get_screenshot(

superset/utils/webdriver.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@
4545
WindowSize = tuple[int, int]
4646
logger = logging.getLogger(__name__)
4747

48+
# Installation message for missing Playwright (Cypress doesn't work with DeckGL)
49+
PLAYWRIGHT_INSTALL_MESSAGE = (
50+
"To complete the migration from Cypress "
51+
"and enable WebGL/DeckGL screenshot support, install Playwright with: "
52+
"pip install playwright && playwright install chromium"
53+
)
54+
4855
if TYPE_CHECKING:
4956
from typing import Any
5057

@@ -71,6 +78,67 @@
7178
sync_playwright = None
7279

7380

81+
def check_playwright_availability() -> bool:
82+
"""
83+
Lightweight check for Playwright availability.
84+
85+
First checks if browser binary exists, falls back to launch test if needed.
86+
"""
87+
if sync_playwright is None:
88+
return False
89+
90+
try:
91+
with sync_playwright() as p:
92+
# First try lightweight check - just verify executable exists
93+
try:
94+
executable_path = p.chromium.executable_path
95+
if executable_path:
96+
return True
97+
except Exception:
98+
# Fall back to full launch test if executable_path fails
99+
logger.debug(
100+
"Executable path check failed, falling back to launch test"
101+
)
102+
103+
# Fallback: actually launch browser to ensure it works
104+
browser = p.chromium.launch(headless=True)
105+
browser.close()
106+
return True
107+
except Exception as e:
108+
logger.warning(
109+
"Playwright module is installed but browser launch failed. "
110+
"Run 'playwright install chromium' to install browser binaries. "
111+
"Error: %s",
112+
str(e),
113+
)
114+
return False
115+
116+
117+
PLAYWRIGHT_AVAILABLE = check_playwright_availability()
118+
119+
120+
def validate_webdriver_config() -> dict[str, Any]:
121+
"""
122+
Validate webdriver configuration and dependencies.
123+
124+
Used to check migration status from Cypress to Playwright.
125+
Returns a dictionary with the status of available webdrivers
126+
and feature flags.
127+
"""
128+
from superset import feature_flag_manager
129+
130+
return {
131+
"selenium_available": True, # Always available as required dependency
132+
"playwright_available": PLAYWRIGHT_AVAILABLE,
133+
"playwright_feature_enabled": feature_flag_manager.is_feature_enabled(
134+
"PLAYWRIGHT_REPORTS_AND_THUMBNAILS"
135+
),
136+
"recommended_action": (
137+
PLAYWRIGHT_INSTALL_MESSAGE if not PLAYWRIGHT_AVAILABLE else None
138+
),
139+
}
140+
141+
74142
class DashboardStandaloneMode(Enum):
75143
HIDE_NAV = 1
76144
HIDE_NAV_AND_TITLE = 2
@@ -151,6 +219,15 @@ def find_unexpected_errors(page: Page) -> list[str]:
151219
def get_screenshot( # pylint: disable=too-many-locals, too-many-statements # noqa: C901
152220
self, url: str, element_name: str, user: User
153221
) -> bytes | None:
222+
if not PLAYWRIGHT_AVAILABLE:
223+
logger.info(
224+
"Playwright not available - falling back to Selenium. "
225+
"Note: WebGL/Canvas charts may not render correctly with Selenium. "
226+
"%s",
227+
PLAYWRIGHT_INSTALL_MESSAGE,
228+
)
229+
return None
230+
154231
with sync_playwright() as playwright:
155232
browser_args = app.config["WEBDRIVER_OPTION_ARGS"]
156233
browser = playwright.chromium.launch(args=browser_args)

tests/unit_tests/utils/screenshot_test.py

Lines changed: 150 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,16 @@
1717

1818
# pylint: disable=import-outside-toplevel, unused-argument
1919

20-
from unittest.mock import MagicMock
20+
from unittest.mock import MagicMock, patch
2121

2222
import pytest
2323
from pytest_mock import MockerFixture
2424

2525
from superset.utils.hashing import md5_sha_from_dict
2626
from superset.utils.screenshots import (
2727
BaseScreenshot,
28+
ChartScreenshot,
29+
DashboardScreenshot,
2830
ScreenshotCachePayload,
2931
ScreenshotCachePayloadType,
3032
)
@@ -239,3 +241,150 @@ def test_get_image_multiple_reads(self):
239241

240242
# Should be different BytesIO instances
241243
assert result1 is not result2
244+
245+
246+
class TestBaseScreenshotDriverFallback:
247+
"""Test BaseScreenshot.driver() fallback logic for Playwright migration."""
248+
249+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", True)
250+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
251+
def test_driver_returns_playwright_when_feature_enabled_and_available(
252+
self, mock_feature_flag, screenshot_obj
253+
):
254+
"""Test driver() returns WebDriverPlaywright when enabled and available."""
255+
mock_feature_flag.return_value = True
256+
257+
driver = screenshot_obj.driver()
258+
259+
assert driver.__class__.__name__ == "WebDriverPlaywright"
260+
mock_feature_flag.assert_called_once_with("PLAYWRIGHT_REPORTS_AND_THUMBNAILS")
261+
262+
@patch("superset.utils.screenshots.logger")
263+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", False)
264+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
265+
def test_driver_falls_back_to_selenium_when_playwright_unavailable(
266+
self, mock_feature_flag, mock_logger, screenshot_obj
267+
):
268+
"""Test driver() falls back to Selenium when Playwright unavailable."""
269+
mock_feature_flag.return_value = True
270+
271+
driver = screenshot_obj.driver()
272+
273+
assert driver.__class__.__name__ == "WebDriverSelenium"
274+
# Should log the fallback message
275+
mock_logger.info.assert_called_once()
276+
log_call = mock_logger.info.call_args[0][0]
277+
assert (
278+
"PLAYWRIGHT_REPORTS_AND_THUMBNAILS enabled but Playwright not installed"
279+
in log_call
280+
)
281+
assert "Falling back to Selenium" in log_call
282+
assert "WebGL/Canvas charts may not render correctly" in log_call
283+
284+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
285+
def test_driver_uses_selenium_when_feature_flag_disabled(
286+
self, mock_feature_flag, screenshot_obj
287+
):
288+
"""Test driver() uses Selenium when feature flag disabled."""
289+
mock_feature_flag.return_value = False
290+
291+
driver = screenshot_obj.driver()
292+
293+
assert driver.__class__.__name__ == "WebDriverSelenium"
294+
mock_feature_flag.assert_called_once_with("PLAYWRIGHT_REPORTS_AND_THUMBNAILS")
295+
296+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", True)
297+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
298+
def test_driver_passes_window_size_to_playwright(
299+
self, mock_feature_flag, screenshot_obj
300+
):
301+
"""Test driver() passes window_size parameter to WebDriverPlaywright."""
302+
mock_feature_flag.return_value = True
303+
custom_window_size = (1200, 800)
304+
305+
driver = screenshot_obj.driver(window_size=custom_window_size)
306+
307+
assert driver._window == custom_window_size
308+
assert driver.__class__.__name__ == "WebDriverPlaywright"
309+
310+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
311+
def test_driver_passes_window_size_to_selenium(
312+
self, mock_feature_flag, screenshot_obj
313+
):
314+
"""Test driver() passes window_size parameter to WebDriverSelenium."""
315+
mock_feature_flag.return_value = False
316+
custom_window_size = (1200, 800)
317+
318+
driver = screenshot_obj.driver(window_size=custom_window_size)
319+
320+
assert driver._window == custom_window_size
321+
assert driver.__class__.__name__ == "WebDriverSelenium"
322+
323+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", True)
324+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
325+
def test_driver_uses_default_window_size_when_none_provided(
326+
self, mock_feature_flag, screenshot_obj
327+
):
328+
"""Test driver() uses screenshot object's window_size when none provided."""
329+
mock_feature_flag.return_value = True
330+
331+
driver = screenshot_obj.driver()
332+
333+
assert driver._window == screenshot_obj.window_size
334+
assert driver.__class__.__name__ == "WebDriverPlaywright"
335+
336+
337+
class TestScreenshotSubclassesDriverBehavior:
338+
"""Test ChartScreenshot and DashboardScreenshot inherit driver behavior."""
339+
340+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", True)
341+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
342+
def test_chart_screenshot_uses_playwright_when_enabled(self, mock_feature_flag):
343+
"""Test ChartScreenshot uses Playwright when feature enabled."""
344+
mock_feature_flag.return_value = True
345+
346+
chart_screenshot = ChartScreenshot("http://example.com/chart", "digest")
347+
driver = chart_screenshot.driver()
348+
349+
assert driver.__class__.__name__ == "WebDriverPlaywright"
350+
assert driver._window == chart_screenshot.window_size
351+
352+
@patch("superset.utils.screenshots.logger")
353+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", False)
354+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
355+
def test_dashboard_screenshot_falls_back_to_selenium(
356+
self, mock_feature_flag, mock_logger
357+
):
358+
"""Test DashboardScreenshot falls back to Selenium if no Playwright."""
359+
mock_feature_flag.return_value = True
360+
361+
dashboard_screenshot = DashboardScreenshot(
362+
"http://example.com/dashboard", "digest"
363+
)
364+
driver = dashboard_screenshot.driver()
365+
366+
assert driver.__class__.__name__ == "WebDriverSelenium"
367+
assert driver._window == dashboard_screenshot.window_size
368+
369+
# Should log the fallback message
370+
mock_logger.info.assert_called_once()
371+
372+
@patch("superset.utils.screenshots.PLAYWRIGHT_AVAILABLE", True)
373+
@patch("superset.extensions.feature_flag_manager.is_feature_enabled")
374+
def test_custom_window_size_passed_to_driver(self, mock_feature_flag):
375+
"""Test custom window size is passed correctly to driver."""
376+
mock_feature_flag.return_value = True
377+
custom_window_size = (1920, 1080)
378+
custom_thumb_size = (960, 540)
379+
380+
chart_screenshot = ChartScreenshot(
381+
"http://example.com/chart",
382+
"digest",
383+
window_size=custom_window_size,
384+
thumb_size=custom_thumb_size,
385+
)
386+
387+
driver = chart_screenshot.driver()
388+
389+
assert driver._window == custom_window_size
390+
assert chart_screenshot.thumb_size == custom_thumb_size

0 commit comments

Comments
 (0)