[py] Fixed Flaky Bidi Test

[shbenzer]

User description

//py:common-edge-bidi-test/selenium/webdriver/common/bidi_tests.py::test_collect_log_mutations was flaky - fixed test by removing line not reliant on bidi functionality, but the html itself.

Description

removed unnecessary line in bidi_tests.py::test_collect_log_mutations

Motivation and Context

A flaky test in CI - it was driving me nuts

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Fixed a flaky test in bidi_tests.py by removing an unnecessary WebDriverWait condition that was not reliant on bidi functionality.
• Simplified the test_collect_log_mutations function to improve test reliability.

---

Changes walkthrough 📝

	Relevant files
Bug fix	bidi_tests.py Remove redundant WebDriverWait condition in test                  py/test/selenium/webdriver/common/bidi_tests.py Removed an unnecessary line in the test_collect_log_mutations function. Simplified the test by eliminating redundant WebDriverWait condition. +0/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[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 Test Reliability Verify that removing the WebDriverWait condition doesn't affect the test's reliability or its ability to catch potential issues.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Use a more specific wait condition to directly check for the style attribute change Consider using a more specific wait condition that directly checks for the style attribute change, rather than relying on the CSS property. This can make the test more robust and potentially faster. py/test/selenium/webdriver/common/bidi_tests.py [76-78] WebDriverWait(driver, 10).until( - lambda d: d.find_element(By.ID, "revealed").value_of_css_property("display") != "none" + EC.attribute_to_be( + (By.ID, "revealed"), + "style", + "" + ) ) Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion improves the robustness and efficiency of the test by directly checking the style attribute change, which is more precise than relying on a CSS property. This change aligns with the test's purpose and can potentially make it faster.	8
Possible issue	✅ Ensure that the wait condition is met before capturing and asserting the event Suggestion Impact: The commit moved the assertions inside the async context manager, aligning with the suggestion to ensure the wait condition is met before capturing and asserting the event. code diff: assert event["attribute_name"] == "style" assert event["current_value"] == "" assert event["old_value"] == "display:none;" Consider moving the WebDriverWait logic into the async context manager to ensure that the wait condition is met before the event is captured and asserted. py/test/selenium/webdriver/common/bidi_tests.py [73-82] async with log.mutation_events() as event: pages.load("dynamic.html") driver.find_element(By.ID, "reveal").click() WebDriverWait(driver, 10).until( lambda d: d.find_element(By.ID, "revealed").value_of_css_property("display") != "none" ) -assert event["attribute_name"] == "style" -assert event["current_value"] == "" -assert event["old_value"] == "display:none;" + assert event["attribute_name"] == "style" + assert event["current_value"] == "" + assert event["old_value"] == "display:none;" Apply this suggestion Suggestion importance[1-10]: 7 Why: Moving the WebDriverWait logic inside the async context manager ensures that the condition is satisfied before capturing and asserting the event, which can prevent race conditions and improve test reliability.	7

💡 Need additional feedback ? start a PR chat

[VietND96]

Thank you, @shbenzer!