Help Make Bazel Python Tests More Robust

[Bradltr95]

User description

💥 What does this PR do?

I tried making these tests a bit more robust to save some cycles when reviewing PRs. This addresses issues like these: https://github.com/SeleniumHQ/selenium/actions/runs/15311089476/job/43079844718 which causes contributors/reviewers to rerun tests multiple times.

• Added WebDriverWaits to any dynamic elements to ensure they are loaded before interacting with them.
• Increase some implicit waits to handle slower environments to hopefully prevent as many rebuilds.
• Made use of pytest.fail to output clearer failure reporting when elements arent found within the timeout.
• Added some retry mechanisms such as loops to retry a few times before failing.

---

PR Type

Tests

---

Description

• Added explicit waits and retries to Python Bazel tests

• Improved failure reporting with pytest.fail and clearer messages

• Increased timeouts to handle slower environments

• Enhanced robustness for dynamic element interactions

---

Changes walkthrough 📝

Relevant files

Tests

devtools_tests.py Add waits, retries, and better failure handling to devtools tests py/test/selenium/webdriver/common/devtools_tests.py Added explicit WebDriverWait with increased timeout for console API calls Introduced try/except with pytest.fail for clearer failure reporting Implemented retry loop for assertion robustness Imported TimeoutException for handling timeouts +18/-6    implicit_waits_tests.py Use explicit waits and improved error handling in implicit waits tests py/test/selenium/webdriver/common/implicit_waits_tests.py Replaced direct element access with WebDriverWait and explicit timeouts Used pytest.fail for clearer error messages on timeouts Increased implicit wait durations for reliability Added imports for WebDriverWait, expected_conditions, and TimeoutException +11/-4

---

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 Retry Logic The retry loop implementation might mask actual test failures. Consider if it's better to fail fast with clear error messages rather than retrying assertions that should pass on first attempt. for _ in range(3): try: assert console_api_calls[0].type_ == "log" assert console_api_calls[0].args[0].value == "I love cheese" assert console_api_calls[1].type_ == "error" assert console_api_calls[1].args[0].value == "I love bread" break except AssertionError: if _ == 2: pytest.fail("Assertions failed after retries.") Redundant Waits The test is using both implicit waits and explicit WebDriverWait, which could lead to unexpected timing behaviors. Consider standardizing on one approach. driver.implicitly_wait(3) add.click() try: WebDriverWait(driver, 5).until(EXPECTED.presence_of_element_located((By.ID, "box0"))) except TimeoutException: pytest.fail("Element 'box0' was not found within the timeout.")

[Bradltr95]

@cgoldberg I tried to make some of the tests more robust as you suggested by implementing better waits, retries, logging, etc..

Let me know if you see any issues with this :)

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Add delay between retries Suggestion Impact: The commit implemented exactly what was suggested - adding a 0.5 second sleep between retry attempts in the test code diff: + time.sleep(0.5) # Add a delay between retries The retry loop doesn't include any delay between attempts, which could lead to false failures if the system needs time to stabilize. Add a small sleep between retries to increase test reliability. py/test/selenium/webdriver/common/devtools_tests.py [42-52] # Retry assertions to handle failures. +import time for _ in range(3): try: assert console_api_calls[0].type_ == "log" assert console_api_calls[0].args[0].value == "I love cheese" assert console_api_calls[1].type_ == "error" assert console_api_calls[1].args[0].value == "I love bread" break except AssertionError: if _ == 2: pytest.fail("Assertions failed after retries.") + time.sleep(0.5) # Add delay between retries [Suggestion processed] Suggestion importance[1-10]: 3 __ Why: The suggestion adds unnecessary complexity since the console_api_calls data is already available after the WebDriverWait, making retries with delays redundant for these simple assertions.	Low
Update

[Bradltr95]

Id love feedback from anyone on the bots suggestion:

The retry loop implementation might mask actual test failures. Consider if it's better to fail fast with clear error messages rather than retrying assertions that should pass on first attempt.

Should I remove the retries to prevent false positives?

[Bradltr95]

I took the bots suggestion and added time.sleep(0.5) # Add a delay between retries to the devtools_tests.py tests. Changes have been pushed.

[cgoldberg]

I don't think we have any problems with py/test/selenium/webdriver/common/implicit_waits_tests.py Do we?

I don't think the changes to py/test/selenium/webdriver/common/devtools_tests.py are helpful. I already raised the timeout to 10 secs, and I don't think that is the root cause. I also don't think retrying the assertions does anything useful.

I appreciate the effort, but I am going to close this.

[Bradltr95]

@cgoldberg Yea I think you're right. Even after my implementation the RBE tests remained flaky. Thanks for looking at the PR.