[py] Add clean_options fixture and remove all Python tests from .skipped-tests

[cgoldberg]

User description

💥 What does this PR do?

This PR fixes all the Python tests so they can run without error on Engflow (RBE CI job). This includes adding a new clean_options fixture to pass options to the clean_driver fixture.

This PR also removes all of the Python tests from ~/.skipped-tests. since they should all pass now.

🔄 Types of changes

• Internal test infrastructure

---

PR Type

Tests

---

Description

• Remove Python tests from .skipped-tests (all now pass)

• Add clean_options fixture for driver options

• Refactor test files to use clean_options fixture

• Simplify and clean up test setup in Python WebDriver tests

---

Changes walkthrough 📝

Relevant files

Tests

7 files .skipped-tests Remove Python test exclusions from skip list                          +0/-5      conftest.py Add clean_options fixture and refactor driver/service fixtures +14/-19  chrome_launcher_tests.py Use clean_options fixture in Chrome launcher tests              +6/-6      chrome_service_tests.py Use clean_options fixture in Chrome service tests                +13/-15  proxy_tests.py Use clean_options fixture in Chrome proxy tests                    +3/-7      edge_launcher_tests.py Use clean_options fixture in Edge launcher tests                  +6/-6      edge_service_tests.py Use clean_options fixture in Edge service tests                    +13/-15

---

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 🔍

(Review updated until commit 98d5d3f)

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 ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	✅ Fix duplicate variable assignment Suggestion Impact: The commit addressed the issue by completely restructuring the options initialization. Instead of having duplicate assignment, the code now initializes options once at the beginning and removes all the redundant 'if not options:' checks throughout the code. code diff: - options = None + + options = getattr(webdriver, f"{driver_class}Options")() if browser_path or browser_args: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() if driver_class == "WebKitGTK": options.overlay_scrollbars_enabled = False if browser_path is not None: @@ -227,16 +226,12 @@ options.add_argument(arg) if headless: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() if driver_class == "Chrome" or driver_class == "Edge": options.add_argument("--headless=new") if driver_class == "Firefox": options.add_argument("-headless") if bidi: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() options.web_socket_url = True There's a duplicate variable assignment with options = options =. This creates a redundant assignment that doesn't cause functional issues but is unnecessary and confusing. Remove the duplication. py/conftest.py [217] -options = options = getattr(webdriver, f"{driver_class}Options")() +options = getattr(webdriver, f"{driver_class}Options")() [Suggestion has been applied] Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies and fixes a redundant variable assignment (options = options =), improving code clarity.	Low
Update

[cgoldberg]

This still needs some work.

The tests fail on RBE when using the clean_driver fixture. That needs to be updated so it uses the correct driver/browser location like the driver fixture does. Then the tests should pass.

[cgoldberg]

All tests are now passing except for a JS one unrelated to this PR.

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to f8b5eda

Category	Suggestion	Impact
Possible issue	Add error handling Add error handling for missing driver configuration similar to what's in the clean_driver fixture. Without proper error handling, the fixture will fail with an unclear error when no driver is specified. py/conftest.py [346-349] @pytest.fixture(scope="function") def clean_service(request): - driver_class = get_driver_class(request.config.option.drivers[0]) + try: + driver_class = get_driver_class(request.config.option.drivers[0]) + except (AttributeError, TypeError): + raise Exception("This test requires a --driver to be specified") yield get_service(driver_class, request.config.option.executable) Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the new clean_service fixture lacks the error handling present in the clean_driver fixture (and previously present in the old clean_service fixture). Adding the try...except block improves robustness and provides clearer error messages when a driver is not specified, aligning it with the clean_driver fixture's behavior.	Medium
More

---

Previous suggestions

✅ Suggestions

Category	Suggestion	Impact
General	✅ Fix duplicate variable assignment Suggestion Impact: The commit addressed the issue by completely restructuring the options initialization. Instead of having duplicate assignment, the code now initializes options once at the beginning and removes all the redundant 'if not options:' checks throughout the code. code diff: - options = None + + options = getattr(webdriver, f"{driver_class}Options")() if browser_path or browser_args: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() if driver_class == "WebKitGTK": options.overlay_scrollbars_enabled = False if browser_path is not None: @@ -227,16 +226,12 @@ options.add_argument(arg) if headless: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() if driver_class == "Chrome" or driver_class == "Edge": options.add_argument("--headless=new") if driver_class == "Firefox": options.add_argument("-headless") if bidi: - if not options: - options = getattr(webdriver, f"{driver_class}Options")() options.web_socket_url = True There's a duplicate variable assignment with options = options =. This creates a redundant assignment that doesn't cause functional issues but is unnecessary and confusing. Remove the duplication. py/conftest.py [217] -options = options = getattr(webdriver, f"{driver_class}Options")() +options = getattr(webdriver, f"{driver_class}Options")() [Suggestion has been applied] Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies and fixes a redundant variable assignment (options = options =), improving code clarity.	Low