[py] Remove support for GLOBAL_DEFAULT_TIMEOUT environment variable

[cgoldberg]

User description

🔗 Related Issues

Fixes #15604
Supersedes #15628

💥 What does this PR do?

This PR remove support for GLOBAL_DEFAULT_TIMEOUT environment variable (which is currently not working anyway).

I also added a test to verify that you can set the HTTP client timeout on webdriver.Remote. This doesn't currently work for other drivers (see: #15672)

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix, Tests

---

Description

• Remove support for GLOBAL_DEFAULT_TIMEOUT environment variable

• Update timeout logic to use socket default only

• Add test verifying HTTP client timeout in Remote WebDriver

• Improve test skipping logic for remote driver tests

---

Changes walkthrough 📝

	Relevant files
Bug fix	client_config.py Remove GLOBAL_DEFAULT_TIMEOUT and simplify timeout logic  py/selenium/webdriver/remote/client_config.py Remove logic for GLOBAL_DEFAULT_TIMEOUT environment variable Set timeout to socket default if not specified +2/-9      remote_connection.py Remove GLOBAL_DEFAULT_TIMEOUT from RemoteConnection            py/selenium/webdriver/remote/remote_connection.py Remove GLOBAL_DEFAULT_TIMEOUT environment variable usage Use socket default timeout for _timeout +1/-5
Tests	remote_connection_tests.py Add test for Remote WebDriver HTTP timeout                              py/test/selenium/webdriver/remote/remote_connection_tests.py Add test for HTTP client timeout in Remote WebDriver Import necessary modules for new test +24/-0
Enhancement	conftest.py Improve remote test skipping in firefox_options fixture    py/conftest.py Enhance firefox_options fixture to skip remote tests with local drivers Raise exception if --driver not specified +7/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to aeedba1

Category	Suggestion	Impact
Possible issue	Handle missing index error The exception handling is too broad. If request.config.option.drivers exists but is empty, it will raise an IndexError, but your code only catches AttributeError and TypeError. This could lead to unexpected errors. py/conftest.py [402-405] try: driver_option = request.config.option.drivers[0] -except (AttributeError, TypeError): +except (AttributeError, TypeError, IndexError): raise Exception("This test requires a --driver to be specified") Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential bug where an IndexError could occur if request.config.option.drivers exists but is empty. Adding IndexError to the exception handling would make the code more robust and prevent unexpected crashes.	Medium
General	Improve timeout test reliability The test assumes that ReadTimeoutError will always be raised, but this might not be reliable. The timeout behavior could vary based on network conditions or server response times, making the test flaky. py/test/selenium/webdriver/remote/remote_connection_tests.py [38-52] def test_remote_webdriver_with_http_timeout(firefox_options, webserver): """This test starts a remote webdriver with an http client timeout set less than the implicit wait timeout, and verifies the http timeout is triggered first when waiting for an element. """ http_timeout = 6 wait_timeout = 8 server_addr = f"http://{webserver.host}:{webserver.port}" client_config = ClientConfig(remote_server_addr=server_addr, timeout=http_timeout) assert client_config.timeout == http_timeout with webdriver.Remote(options=firefox_options, client_config=client_config) as driver: driver.get(f"{server_addr}/simpleTest.html") driver.implicitly_wait(wait_timeout) + start_time = time.time() with pytest.raises(ReadTimeoutError): driver.find_element(By.ID, "no_element_to_be_found") + elapsed_time = time.time() - start_time + # Verify timeout occurred closer to http_timeout than wait_timeout + assert elapsed_time < wait_timeout, f"Test took {elapsed_time}s, expected < {wait_timeout}s" Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion adds time measurement and verification to ensure the test is actually failing due to the HTTP timeout rather than the implicit wait timeout. This makes the test more reliable and verifiable, providing better validation of the timeout behavior being tested.	Medium
Learned best practice	Improve error message clarity Improve error message clarity by providing more specific details about what went wrong and how to resolve it. The current error message doesn't explain what the --driver option is or how to use it. py/conftest.py [402-405] try: driver_option = request.config.option.drivers[0] except (AttributeError, TypeError): - raise Exception("This test requires a --driver to be specified") + raise Exception("This test requires a --driver option to be specified. Use '--driver=firefox' or another supported driver name when running the test.") Apply this suggestion Suggestion importance[1-10]: 6	Low
Update

---

Previous suggestions

✅ Suggestions up to commit b4cd3f0

Category	Suggestion	Impact
General	✅ Ensure proper resource cleanup Suggestion Impact: The suggestion recommended using a try-finally block to ensure driver cleanup. The commit implemented this concept but used a more elegant solution with a context manager (using 'with' statement) which automatically handles resource cleanup, achieving the same goal of ensuring the driver is properly closed. code diff: + with webdriver.Remote(options=firefox_options, client_config=client_config) as driver: + driver.get(f"{server_addr}/simpleTest.html") + driver.implicitly_wait(wait_timeout) + with pytest.raises(ReadTimeoutError): + driver.find_element(By.ID, "no_element_to_be_found") Ensure the driver is properly closed even if the test fails by using a try-finally block. Currently, if the test fails before reaching driver.quit(), the driver won't be properly cleaned up. py/test/selenium/webdriver/remote/remote_connection_tests.py [38-53] def test_remote_webdriver_with_http_timeout(firefox_options, webserver): """This test starts a remote webdriver with an http client timeout set less than the implicit wait timeout, and verifies the http timeout is triggered first when waiting for an element. """ http_timeout = 6 wait_timeout = 8 server_addr = f"http://{webserver.host}:{webserver.port}" client_config = ClientConfig(remote_server_addr=server_addr, timeout=http_timeout) assert client_config.timeout == http_timeout driver = webdriver.Remote(options=firefox_options, client_config=client_config) - driver.get(f"{server_addr}/simpleTest.html") - driver.implicitly_wait(wait_timeout) - with pytest.raises(ReadTimeoutError): - driver.find_element(By.ID, "no_element_to_be_found") - driver.quit() + try: + driver.get(f"{server_addr}/simpleTest.html") + driver.implicitly_wait(wait_timeout) + with pytest.raises(ReadTimeoutError): + driver.find_element(By.ID, "no_element_to_be_found") + finally: + driver.quit() [Suggestion has been applied] Suggestion importance[1-10]: 8 __ Why: This suggestion addresses a potential resource leak by ensuring the driver is properly closed even if the test fails. This is an important improvement for test reliability and system resource management.	Medium
	Improve error message clarity Use a more specific exception message that explains what the user needs to do. The current message doesn't provide enough guidance on how to fix the issue. py/conftest.py [402-405] try: driver_option = request.config.option.drivers[0] except (AttributeError, TypeError): - raise Exception("This test requires a --driver to be specified") + raise Exception("This test requires a --driver to be specified. Run with '--driver=firefox' or another supported driver.") Suggestion importance[1-10]: 5 __ Why: The suggestion improves the error message by providing specific guidance on how to fix the issue, making it more user-friendly. This is a moderate improvement to usability but doesn't fix a critical bug.	Low