diff --git a/scripts/check_dash_browser_smoke.py b/scripts/check_dash_browser_smoke.py index 97cdef5..4e17594 100644 --- a/scripts/check_dash_browser_smoke.py +++ b/scripts/check_dash_browser_smoke.py @@ -179,7 +179,11 @@ def _check_route( interaction_failures = (download_ok is False) or (provenance_ok is False) graph_failure = graph_count < ROUTE_MIN_GRAPHS.get(_route_key(route), 0) return BrowserSmokeResult( - ok=not missing_text and not serious_console and not failed_requests and not interaction_failures and not graph_failure, + ok=not missing_text + and not serious_console + and not failed_requests + and not interaction_failures + and not graph_failure, route=route, viewport=viewport_name, missing_text=missing_text, @@ -215,7 +219,9 @@ def run_browser_smoke( try: print(f"checking {viewport_name} {route}", flush=True) results.append( - _check_route(page, base_url, route, viewport_name, viewport, timeout_seconds, screenshot_dir) + _check_route( + page, base_url, route, viewport_name, viewport, timeout_seconds, screenshot_dir + ) ) finally: page.close() @@ -262,6 +268,7 @@ def main(argv: list[str] | None = None) -> int: process.wait(timeout=10) except subprocess.TimeoutExpired: process.kill() + process.wait() ok = all(result.ok for result in results) for result in results: diff --git a/tests/scripts/test_check_dash_browser_smoke.py b/tests/scripts/test_check_dash_browser_smoke.py new file mode 100644 index 0000000..7692f33 --- /dev/null +++ b/tests/scripts/test_check_dash_browser_smoke.py @@ -0,0 +1,39 @@ +import os +import subprocess +import sys +from unittest.mock import MagicMock, patch + +# Add the root directory to sys.path so we can import scripts +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../.."))) + +from scripts.check_dash_browser_smoke import main + + +def test_dash_browser_smoke_timeout_error(): + """ + Tests that when the server process times out during the wait() call, + the TimeoutExpired exception is caught, process.kill() is called, and then process.wait() is called. + """ + with ( + patch("scripts.check_dash_browser_smoke._free_port", return_value=12345), + patch("scripts.check_dash_browser_smoke._start_server") as mock_start_server, + patch("scripts.check_dash_browser_smoke._wait_for_server"), + patch("scripts.check_dash_browser_smoke.run_browser_smoke", return_value=[]), + ): + # Create a mock process + mock_process = MagicMock(spec=subprocess.Popen) + mock_start_server.return_value = mock_process + + # Make the first call to wait() raise a TimeoutExpired exception + mock_process.wait.side_effect = [subprocess.TimeoutExpired(cmd=["fake_cmd"], timeout=5), None] + + main([]) + + # The mock process should be terminated first + mock_process.terminate.assert_called_once() + + # After terminate, it should have called kill() due to TimeoutExpired + mock_process.kill.assert_called_once() + + # We also check that wait() was called again after kill() + assert mock_process.wait.call_count == 2