Previously discussed on Matrix, with a variety of very vibecoded fixes put up in #61122.
See https://wpt.fyi/results/webdriver/tests/classic?run_id=5322123978735616&run_id=5776929038008320 (747f5ee...1903b4e).
The tl;dr is com.apple.WebDriver.HTTPService crashes during the tests added in b0dcd3d which causes safaridriver to then exit — however, we never recover from this.
The PytestExecutor never checks if the browser has crashed, merely setting the status to CRASH if OSError or socket.timeout is raised, which is extremely unlikely given pytest should have caught any exceptions itself, and because we keep getting OK harness statuses merely with every subtest returning ERROR: setup error: ConnectionRefusedError: [Errno 61] Connection refused (because, y'know, safaridriver isn't running) we don't fall into the TestRunnerManager restart for CRASH, EXTERNAL-TIMEOUT, INTERNAL-ERROR.
Digging into this shows quite a lot of things that could be improved:
Previously discussed on Matrix, with a variety of very vibecoded fixes put up in #61122.
See https://wpt.fyi/results/webdriver/tests/classic?run_id=5322123978735616&run_id=5776929038008320 (747f5ee...1903b4e).
The tl;dr is
com.apple.WebDriver.HTTPServicecrashes during the tests added in b0dcd3d which causessafaridriverto then exit — however, we never recover from this.The
PytestExecutornever checks if the browser has crashed, merely setting the status to CRASH ifOSErrororsocket.timeoutis raised, which is extremely unlikely givenpytestshould have caught any exceptions itself, and because we keep gettingOKharness statuses merely with every subtest returningERROR:setup error: ConnectionRefusedError: [Errno 61] Connection refused(because, y'know,safaridriverisn't running) we don't fall into theTestRunnerManagerrestart for CRASH, EXTERNAL-TIMEOUT, INTERNAL-ERROR.Digging into this shows quite a lot of things that could be improved:
TimedRunnerandPytestRun(there's a lot of duplicate logic there, though it wouldn't directly fix this!).PytestExecutorcheck whether the browser is still alive regardless of result (because unlike every other executor, it can still get an OK harness result regardless of whether the browser is actually running).TestRunnerManagercheck whether the browser is running itself, along with checking the various harness statuses (this will have a per cost, adding a bunch of redundant checks, but guarantees that an executor bug can't cause this cascading failure).TestRunnerManagerrestart when any subtest has ERROR as a result, not the harness-level INTERNAL-ERROR (but then maybe we also should for harness-level ERROR too, which seems unnecessary).Browserobject to theExecutorvia amultiprocessing.Manager, avoiding the currentExecutorBrowserintroduced way back in 9a9d62f, and would allow us to provide the ability to restart the browser from within the pytest fixtures.