[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()`.

Author: lauromoura

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