[py] Re-enable remote tests in bazel and fix broken tests

[cgoldberg]

User description

🔗 Related Issues

💥 What does this PR do?

This PR fixes several issue with Python Remote WebDriver tests:

• Fixes bazel //py:test-remote target so it picks up tests in py/test/selenium/webdriver/remote/. Previously, it was skipping them.
• Moved pytest_generate_tests to main conftest.py, allowing all tests to access --driver flag while removing duplicate conftest.py files
• Update the driver fixture so it skips Remote WebDriver tests when called with a local driver
• Added new firefox_options fixture that returns an instance of FirefoxOptions with headless enabled/disabled depending on whether --headless arg was given. This is useful in tests that don't use the main driver fixture to launch the browser
• Some cleanup in the main conftest.py

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)

---

PR Type

Bug fix, Enhancement

---

Description

• Centralize and fix driver parametrization for all tests

  • Move pytest_generate_tests to main conftest.py
  • Remove duplicate conftest.py files in subdirectories

• Fix and re-enable remote WebDriver tests in Bazel

  • Update Bazel config to include remote test directory
  • Adjust test skipping logic for remote/local driver distinction

• Add new headless fixture for consistent headless option access

• Refactor and clean up remote and Firefox test implementations

---

Changes walkthrough 📝

	Relevant files
Enhancement	4 files conftest.py Centralize driver parametrization, add headless fixture, improve skipping logic +22/-4    firefox_sizing_tests.py Use new headless fixture and context manager for driver    +8/-15    custom_element_tests.py Use fixture for custom element driver, cleanup test logic +14/-6    remote_firefox_profile_tests.py Refactor to use headless fixture and context manager, remove custom fixtures +13/-22
Cleanup	4 files conftest.py Remove duplicate driver parametrization logic                        +0/-21    conftest.py Remove duplicate driver parametrization logic                        +0/-21    conftest.py Remove duplicate driver parametrization logic                        +0/-21    conftest.py Remove duplicate driver parametrization logic                        +0/-21
Documentation	1 files remote_connection_tests.py Add docstring, clarify test intent for browser-specific method +2/-0
Tests	1 files remote_custom_locator_tests.py Mark tests as skipped, add pytest import                                  +5/-0
Formatting	1 files remote_downloads_tests.py Minor import reordering                                                                    +1/-0
Bug fix	1 files remote_hub_connection.py Add options to Remote, improve assertion for SSL error      +6/-2
Configuration changes	1 files BUILD.bazel Update Bazel config to include remote tests, remove obsolete conftest.py references +1/-2

---

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 5d6d8c9)

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Resource Cleanup The custom_element_driver fixture in remote_custom_element_tests.py properly resets the web_element_cls, but the driver fixture doesn't reset it after tests. This could cause unexpected behavior if tests run in a specific order. @pytest.fixture(scope="function") def driver(request): kwargs = {} driver_option = getattr(request, "param", "Chrome") # browser can be changed with `--driver=firefox` as an argument or to addopts in pytest.ini driver_class = get_driver_class(driver_option) # skip tests in the 'remote' directory if run with a local driver if request.node.path.parts[-2] == "remote" and driver_class != "Remote": pytest.skip(f"Remote tests can't be run with driver '{driver_option}'") # skip tests that can't run on certain platforms _platform = platform.system() if driver_class == "Safari" and _platform != "Darwin": pytest.skip("Safari tests can only run on an Apple OS") if (driver_class == "Ie") and _platform != "Windows": pytest.skip("IE and EdgeHTML Tests can only run on Windows") if "WebKit" in driver_class and _platform == "Windows": pytest.skip("WebKit tests cannot be run on Windows") # skip tests for drivers that don't support BiDi when --bidi is enabled if request.config.option.bidi: if driver_class in ("Ie", "Safari", "WebKitGTK", "WPEWebKit"): pytest.skip(f"{driver_class} does not support BiDi") # conditionally mark tests as expected to fail based on driver marker = request.node.get_closest_marker(f"xfail_{driver_class.lower()}") if marker is not None: if "run" in marker.kwargs: if marker.kwargs["run"] is False: pytest.skip() yield return if "raises" in marker.kwargs: marker.kwargs.pop("raises") pytest.xfail(**marker.kwargs) def fin(): global driver_instance if driver_instance is not None: driver_instance.quit() driver_instance = None request.addfinalizer(fin) driver_path = request.config.option.executable options = None global driver_instance if driver_instance is None: if driver_class == "Firefox": options = get_options(driver_class, request.config) if platform.system() == "Linux": # There are issues with window size/position when running Firefox # under Wayland, so we use XWayland instead. os.environ["MOZ_ENABLE_WAYLAND"] = "0" if driver_class == "Chrome": options = get_options(driver_class, request.config) if driver_class == "Edge": options = get_options(driver_class, request.config) if driver_class == "WebKitGTK": options = get_options(driver_class, request.config) if driver_class == "WPEWebKit": options = get_options(driver_class, request.config) if driver_class == "Remote": options = get_options("Firefox", request.config) or webdriver.FirefoxOptions() options.set_capability("moz:firefoxOptions", {}) options.enable_downloads = True if driver_path is not None: kwargs["service"] = get_service(driver_class, driver_path) if options is not None: kwargs["options"] = options driver_instance = getattr(webdriver, driver_class)(**kwargs) yield driver_instance # Close the browser after BiDi tests. Those make event subscriptions # and doesn't seems to be stable enough, causing the flakiness of the # subsequent tests. Incomplete Implementation The tests are marked as skipped with "Needs to be updated" but there's no implementation of setting the custom locator converter on the driver. The tests will fail if unskipped without proper implementation. @pytest.mark.skip(reason="Needs to be updated") def test_find_element_with_custom_locator(driver): driver.get("data:text/html,<div custom-attr='example'>Test</div>") element = driver.find_element("custom", "example") assert element is not None assert element.text == "Test" @pytest.mark.skip(reason="Needs to be updated") def test_find_elements_with_custom_locator(driver): driver.get("data:text/html,<div custom-attr='example'>Test1</div><div custom-attr='example'>Test2</div>") elements = driver.find_elements("custom", "example") assert len(elements) == 2

[titusfortner]

Does it need a separate conftest? It'll be easy to overlook it if there are multiple.

[cgoldberg]

Does it need a separate conftest?

Good call... it doesn't. I just moved the function it contained into the main conftest, which allowed me to removed it, along with 3 other duplicate conftests.

I just updated the PR description too. This ended up larger than I was expecting, but it cleans up a lot of junk.

I haven't tested it all yet, so we'll see if CI passes :)

[cgoldberg]

@navin772

Can you take a look at py/test/selenium/webdriver/remote/remote_custom_locator_tests.py? (either in this PR or in trunk once it gets merged).

There are 2 tests that you added in #14587. They fail for me locally, so I have them marked to skip (they weren't getting run in CI before this PR). I don't really understand what they do and don't know if they are failing because of a real issue or they are just broken tests.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix path handling compatibility The code is using request.node.path.parts which assumes path is a pathlib.Path object, but in some pytest versions it might be a string. Use pathlib.Path(request.node.path).parts to ensure compatibility. py/conftest.py [126-128] # skip tests in the 'remote' directory if run with a local driver -if request.node.path.parts[-2] == "remote" and driver_class != "Remote": +from pathlib import Path +if Path(request.node.path).parts[-2] == "remote" and driver_class != "Remote": pytest.skip(f"Remote tests can't be run with driver '{driver_option}'") Apply this suggestion Suggestion importance[1-10]: 8 __ Why: This suggestion addresses a potential compatibility issue with pytest path handling. Using Path(request.node.path).parts ensures the code works correctly across different pytest versions where path might be a string instead of a Path object, preventing potential runtime errors.	Medium
	Make SSL error check robust The assertion is too specific about the error message. SSL error messages can vary between platforms and urllib3 versions. Use a more general assertion that checks for SSL verification failure. py/test/selenium/webdriver/remote/remote_hub_connection.py [31-32] assert site in str(excinfo.value) -assert "certificate is not valid" in str(excinfo.value).lower() +# Check for any SSL verification failure message +assert any(msg in str(excinfo.value).lower() for msg in ["ssl", "certificate", "verify"]) Apply this suggestion Suggestion importance[1-10]: 6 __ Why: The suggestion improves test reliability by making the SSL error assertion more flexible. Since SSL error messages can vary between platforms and library versions, using a more general check prevents test failures due to minor message variations.	Low
General	Ensure proper fixture cleanup The fixture doesn't properly restore the original web element class if an exception occurs during the test. Use try/finally to ensure proper cleanup regardless of test outcome. py/test/selenium/webdriver/remote/custom_element_tests.py [30-34] @pytest.fixture() def custom_element_driver(driver): - driver._web_element_cls = MyCustomElement - yield driver - driver._web_element_cls = WebElement + original_cls = driver._web_element_cls + try: + driver._web_element_cls = MyCustomElement + yield driver + finally: + driver._web_element_cls = original_cls Apply this suggestion Suggestion importance[1-10]: 7 __ Why: This suggestion improves test robustness by properly saving and restoring the original web element class in a try/finally block, ensuring cleanup happens even if an exception occurs during test execution, preventing test state leakage.	Medium
Update

[navin772]

Hi @cgoldberg, the tests are failing because of a misconfiguration, the remote webdriver expects the CustomLocatorConverter() class to be passed as a param for the tests to pass.
Something like this:

driver = webdriver.Remote(command_executor="http://localhost:4444", options=options, locator_converter=CustomLocatorConverter())

Do you know how can I add the above only for my test file?

[cgoldberg]

@navin772 I'll fix it in this branch. Thanks!

[cgoldberg]

@navin772 thanks.. that worked.

Do you know how can I add the above only for my test file?

I added a small fixture to the test file so it doesn't use the main driver fixture. It's not great, but it solves this problem.

[navin772]

@cgoldberg thanks for the changes, this PR looks good, just run the formatter once.

[titusfortner]

which allowed me to removed it, along with 3 other duplicate conftests

Right, I had no idea there were multiple of these, so thanks!

[cgoldberg]

The only failure in CI is not related to this PR.