From 5fdfc777b292138afc15cb4b5c927ad429f186cc Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Thu, 28 Aug 2025 15:59:36 -0300 Subject: [PATCH 1/3] [py] Remove redundant driver_instance from conftest.py Currently, the driver lifetime is handled by two global variables: - `driver_instance`, the actual Selenium API driver. - `selenium_driver`, the wrapper `conftest.Driver`, which "owns" the former. Also, conftest.py could quit the driver from multiple places: - `Driver.stop_driver` finalizer, used for bidi and xfail tests - `stop_driver` Session-scope fixture finalizer - `pytest_exception_interact` to handle exceptions The last two paths called `driver_instance.quit()` directly but did not tell `selenium_driver` that the driver had exited. This could potentially raise `urllib.exception.MaxRetryError`, as `Driver.stop_driver` would try to send the `quit()` command to an already destroyed driver service. For example, this happened rather frequently on WebKit import of selenium 4.34.2 tests, whenever an exception happened, as we have enabled the `bidi` flag when calling pytest. To address this issue, this commit removes the global variable `driver_instance`, keeping only `selenium_driver` as global, and routes all teardown paths through `Driver.stop_driver()`. --- py/conftest.py | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/py/conftest.py b/py/conftest.py index 37f76fb1731b4..6bc6f83f28019 100644 --- a/py/conftest.py +++ b/py/conftest.py @@ -101,7 +101,6 @@ def pytest_generate_tests(metafunc): metafunc.parametrize("driver", metafunc.config.option.drivers, indirect=True) -driver_instance = None selenium_driver = None @@ -276,7 +275,8 @@ def service(self): @property def driver(self): - self._driver = self._initialize_driver() + if self._driver is None: + self._driver = self._initialize_driver() return self._driver @property @@ -297,21 +297,15 @@ def _initialize_driver(self): kwargs["service"] = self.service return getattr(webdriver, self.driver_class)(**kwargs) - @property def stop_driver(self): - def fin(): - global driver_instance - if self._driver is not None: - self._driver.quit() - self._driver = None - driver_instance = None - - return fin + driver_to_stop = self._driver + self._driver = None + if driver_to_stop is not None: + driver_to_stop.quit() @pytest.fixture(scope="function") def driver(request): - global driver_instance global selenium_driver driver_class = getattr(request, "param", "Chrome").lower() @@ -345,38 +339,36 @@ def driver(request): request.addfinalizer(selenium_driver.stop_driver) - if driver_instance is None: - driver_instance = selenium_driver.driver - - yield driver_instance # Close the browser after BiDi tests. Those make event subscriptions # and doesn't seems to be stable enough, causing the flakiness of the # subsequent tests. # Remove this when BiDi implementation and API is stable. - if selenium_driver.bidi: + if selenium_driver is not None and selenium_driver.bidi: request.addfinalizer(selenium_driver.stop_driver) + yield selenium_driver.driver + if request.node.get_closest_marker("no_driver_after_test"): - driver_instance = None + selenium_driver = None @pytest.fixture(scope="session", autouse=True) def stop_driver(request): def fin(): - global driver_instance - if driver_instance is not None: - driver_instance.quit() - driver_instance = None + global selenium_driver + if selenium_driver is not None: + selenium_driver.stop_driver() + selenium_driver = None request.addfinalizer(fin) def pytest_exception_interact(node, call, report): if report.failed: - global driver_instance - if driver_instance is not None: - driver_instance.quit() - driver_instance = None + global selenium_driver + if selenium_driver is not None: + selenium_driver.stop_driver() + selenium_driver = None @pytest.fixture From a61dff19cd0dec11ec8778f5cde38a408a30d133 Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Fri, 29 Aug 2025 10:50:44 -0300 Subject: [PATCH 2/3] Properly stop driver before nullifying As suggested by the review bot --- py/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/py/conftest.py b/py/conftest.py index 6bc6f83f28019..d15a52ddc04bc 100644 --- a/py/conftest.py +++ b/py/conftest.py @@ -349,6 +349,8 @@ def driver(request): yield selenium_driver.driver if request.node.get_closest_marker("no_driver_after_test"): + if selenium_driver is not None: + selenium_driver.stop_driver() selenium_driver = None From c0d8f5fa05391e7e8079226502dbfde09de1bbf7 Mon Sep 17 00:00:00 2001 From: Lauro Moura Date: Fri, 29 Aug 2025 23:56:39 -0300 Subject: [PATCH 3/3] [py] Ignore exception in no_driver_after_test fixture The `no_driver_after_test` indicates that tests are expected to quit the driver themselves, leaving `selenium_driver` with a stale driver instance. This commit keeps the `stop_driver()` call as a safety fallback, but ignores WebDriverExceptions since they're expected in the call when the test properly quits the driver. --- py/conftest.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/py/conftest.py b/py/conftest.py index d15a52ddc04bc..aeff257648150 100644 --- a/py/conftest.py +++ b/py/conftest.py @@ -23,6 +23,7 @@ import pytest from selenium import webdriver +from selenium.common.exceptions import WebDriverException from selenium.webdriver.remote.server import Server from test.selenium.webdriver.common.network import get_lan_ip from test.selenium.webdriver.common.webserver import SimpleWebServer @@ -350,8 +351,13 @@ def driver(request): if request.node.get_closest_marker("no_driver_after_test"): if selenium_driver is not None: - selenium_driver.stop_driver() - selenium_driver = None + try: + selenium_driver.stop_driver() + except WebDriverException: + pass + except Exception: + raise + selenium_driver = None @pytest.fixture(scope="session", autouse=True)