Skip to content

Conversation

cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Oct 3, 2025

User description

💥 What does this PR do?

This PR removes some old xfail test markers related to see if they pass on GHA.

🔄 Types of changes

  • Build/Test

PR Type

Tests


Description

  • Remove outdated Travis CI xfail markers from Python tests

  • Clean up test markers for Firefox, Chrome, and remote drivers

  • Update alert and interaction test configurations


Diagram Walkthrough

flowchart LR
  A["Travis CI xfail markers"] -- "remove" --> B["Clean test files"]
  B --> C["Updated test configurations"]
Loading

File Walkthrough

Relevant files
Tests
alerts_tests.py
Remove Travis CI alert test markers                                           

py/test/selenium/webdriver/common/alerts_tests.py

  • Remove @pytest.mark.xfail_firefox and @pytest.mark.xfail_remote
    markers with "Fails on travis" reason
  • Keep Safari xfail markers intact
  • Clean up two alert test functions
+0/-4     
w3c_interaction_tests.py
Remove Travis CI interaction test markers                               

py/test/selenium/webdriver/common/w3c_interaction_tests.py

  • Remove @pytest.mark.xfail_remote and @pytest.mark.xfail_chrome markers
    with "Fails on Travis" reason
  • Keep Firefox and Safari xfail markers
  • Update double click test configuration
+0/-2     
window_tests.py
Remove Travis CI window test markers                                         

py/test/selenium/webdriver/common/window_tests.py

  • Remove commented Travis CI related xfail markers and skipif conditions
  • Clean up window maximize, fullscreen, and minimize test comments
  • Remove environment-based test skipping logic
+0/-7     

@selenium-ci selenium-ci added the C-py Python Bindings label Oct 3, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 3, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

qodo-merge-pro bot commented Oct 3, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Uncomment test to re-enable it

Uncomment the test_should_maximize_the_window test to re-enable it, as the PR's
changes removed Travis-specific xfail markers but left the test itself commented
out.

py/test/selenium/webdriver/common/window_tests.py [23-32]

-# @pytest.mark.xfail_ie
-# def test_should_maximize_the_window(driver):
-#     resize_timeout = 5
-#     wait = WebDriverWait(driver, resize_timeout)
-#     size_before = driver.get_window_size()
-#     driver.maximize_window()
-#     wait.until(lambda dr: dr.get_window_size()['width'] > size_before['width'])
-#     size_after = driver.get_window_size()
-#     assert size_after['width'] > size_before['width']
-#     assert size_after['height'] > size_before['height']
+@pytest.mark.xfail_ie
+def test_should_maximize_the_window(driver):
+    resize_timeout = 5
+    wait = WebDriverWait(driver, resize_timeout)
+    size_before = driver.get_window_size()
+    driver.maximize_window()
+    wait.until(lambda dr: dr.get_window_size()['width'] > size_before['width'])
+    size_after = driver.get_window_size()
+    assert size_after['width'] > size_before['width']
+    assert size_after['height'] > size_before['height']

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that a test remains commented out after its Travis-specific markers were removed, making the PR's change ineffective for this test. Uncommenting the test is a logical next step to re-enable it, which aligns with the PR's intent.

Medium
Learned
best practice
Verify page load before actions

Assert the test page loaded successfully to fail fast with a clear reason if
navigation failed.

py/test/selenium/webdriver/common/w3c_interaction_tests.py [149-151]

 def test_double_click(driver, pages):
     """Copied from org.openqa.selenium.interactions.TestBasicMouseInterface."""
     pages.load("javascriptPage.html")
+    assert "javascript" in driver.current_url, "Failed to load javascriptPage.html"

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Validate inputs and states early with precise checks to prevent logic errors.

Low
  • Update

@cgoldberg cgoldberg merged commit 2a8261d into SeleniumHQ:trunk Oct 4, 2025
16 checks passed
@cgoldberg cgoldberg deleted the py-remove-old-test-markers branch October 4, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants