[py] Remove redundant driver_instance from conftest.py

[lauromoura]

User description

🔗 Problem description

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.

💥 What does this PR do?

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

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)

---

PR Type

Bug fix, Other

---

Description

• Remove redundant driver_instance global variable from conftest.py

• Route all driver teardown paths through Driver.stop_driver() method

• Fix potential MaxRetryError when driver quit called multiple times

• Improve driver lifecycle management consistency

---

Diagram Walkthrough

flowchart LR
  A["Multiple teardown paths"] --> B["driver_instance.quit()"]
  A --> C["selenium_driver.stop_driver()"]
  B --> D["Potential MaxRetryError"]
  E["Single teardown path"] --> F["selenium_driver.stop_driver()"]
  F --> G["Clean driver shutdown"]

Loading

File Walkthrough

	Relevant files
Bug fix	conftest.py Consolidate driver teardown logic                                                py/conftest.py Remove global driver_instance variable declaration Convert stop_driver from property to method in Driver class Update all teardown paths to use selenium_driver.stop_driver() Add null check for selenium_driver in BiDi test cleanup +18/-26

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The driver property now lazily initializes only when accessed; ensure no callers rely on side effects of eager initialization (e.g., options/service setup timing) and that multiple accesses during setup don't create race conditions under parallel pytest. @property def driver(self): if self._driver is None: self._driver = self._initialize_driver() return self._driver Fixture Finalizers Multiple addfinalizer calls to stop the same driver can occur (xfail path, BiDi cleanup, session fin, exception hook). Verify idempotency across all paths and that setting selenium_driver to None doesn’t break later finalizers referencing it. request.addfinalizer(selenium_driver.stop_driver) # 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 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"): selenium_driver = None @pytest.fixture(scope="session", autouse=True) def stop_driver(request): def fin(): 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 selenium_driver if selenium_driver is not None: selenium_driver.stop_driver() selenium_driver = None Global State Reset After yielding the driver, the code sets selenium_driver to None when marker no_driver_after_test is present; confirm subsequent tests re-create Driver correctly and that no lingering references are used by other fixtures like pages. yield selenium_driver.driver if request.node.get_closest_marker("no_driver_after_test"): selenium_driver = None

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Properly stop driver before nullifying Suggestion Impact: The commit added a check to stop the driver (selenium_driver.stop_driver()) before nullifying selenium_driver when the "no_driver_after_test" marker is present. code diff: if request.node.get_closest_marker("no_driver_after_test"): + if selenium_driver is not None: + selenium_driver.stop_driver() selenium_driver = None Setting the global variable to None without properly stopping the driver can lead to resource leaks. The driver should be stopped before nullifying the reference to ensure proper cleanup. py/conftest.py [344-345] if request.node.get_closest_marker("no_driver_after_test"): + if selenium_driver is not None: + selenium_driver.stop_driver() selenium_driver = None [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that nullifying selenium_driver without stopping it first is fragile and can lead to resource leaks, making the code more robust and consistent with other cleanup logic in the file.	Medium
	Handle exceptions during driver cleanup The method should handle exceptions during driver quit to prevent test failures. Add a try-except block around the quit() call to catch and log any exceptions that might occur during cleanup. py/conftest.py [293-297] def stop_driver(self): driver_to_stop = self._driver self._driver = None if driver_to_stop is not None: - driver_to_stop.quit() + try: + driver_to_stop.quit() + except Exception: + pass # Ignore exceptions during cleanup Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly proposes to wrap driver_to_stop.quit() in a try-except block, which is a good practice for cleanup logic to prevent test suite failures from exceptions during teardown.	Low
Update

[cgoldberg]

Thanks @lauromoura
Can you add the "Properly stop driver before nullifying" suggestion from the review bot? #16271 (comment)

[cgoldberg]

This is causing a failure in CI.

https://github.com/SeleniumHQ/selenium/actions/runs/17329416928/job/49202234767?pr=16271

[lauromoura]

This is causing a failure in CI.

https://github.com/SeleniumHQ/selenium/actions/runs/17329416928/job/49202234767?pr=16271

Checking it. Thanks for the heads-up.

[lauromoura]

@cgoldberg CI fixed :)

[cgoldberg]

LGTM. Thanks @lauromoura