Skip to content

Conversation

navin772
Copy link
Member

@navin772 navin772 commented Aug 1, 2025

User description

🔗 Related Issues

https://issues.chromium.org/issues/425801332

💥 What does this PR do?

Enable FedCM tests for chrome

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Tests


Description

  • Remove Chrome xfail marker for FedCM tests

  • Enable FedCM testing for Chrome browser


Diagram Walkthrough

flowchart LR
  A["FedCM Tests"] -- "Remove xfail marker" --> B["Chrome Support Enabled"]
Loading

File Walkthrough

Relevant files
Tests
fedcm_tests.py
Remove Chrome xfail for FedCM tests                                           

py/test/selenium/webdriver/common/fedcm_tests.py

  • Remove @pytest.mark.xfail_chrome decorator
  • Enable FedCM tests to run on Chrome browser
+0/-1     

@selenium-ci selenium-ci added the C-py Python Bindings label Aug 1, 2025
@navin772
Copy link
Member Author

navin772 commented Aug 1, 2025

As per https://issues.chromium.org/issues/425801332 this should be resolved once Chrome 140 is out. Manual test against chrome canary verifies this.

@cgoldberg
Copy link
Member

Nice. We might want to setup CI so we can test against beta releases of Chrome (I think Ruby bindings do this). I'm not sure if it would add too much noise... but it would be cool to see breaking changes and fixes in upcoming releases so we can be ready when they hit the stable channel.

@navin772
Copy link
Member Author

navin772 commented Aug 4, 2025

We already have http archives for linux_beta_chrome and its driver. I guess they were added when Ruby added chrome beta support.
I am working on adding chrome beta bazel targets for python in CI-RBE.

Currently, the bazel targets look like - //py:common-chrome-test/selenium/webdriver/common/fedcm_tests.py.
I will configure the beta ones to look like - //py:common-chrome-beta-test/selenium/webdriver/common/fedcm_tests.py.

I made some changes, I will open a draft PR to see if it works as intended.

@navin772 navin772 marked this pull request as ready for review September 15, 2025 08:08
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Investigate and resolve "ConnectFailure" with multiple ChromeDriver instances.
  • Ensure consistent behavior across multiple instantiations.
  • Provide reproducible steps or mitigations.

Requires further human verification:

  • N/A

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Ensure click() triggers JavaScript in link href for Firefox.
  • Provide test coverage verifying alert fires on click.

Requires further human verification:

  • N/A
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Test Matrix

Removing the Chrome xfail will run these tests on Chrome; ensure CI environments set required FedCM flags/origins and that remote runs do not implicitly include Chrome under the xfail_remote marker logic.

@pytest.mark.xfail_safari(reason="FedCM not supported")
@pytest.mark.xfail_firefox(reason="FedCM not supported")
@pytest.mark.xfail_ie(reason="FedCM not supported")
@pytest.mark.xfail_remote(reason="FedCM not supported, since remote uses Firefox")
class TestFedCM:

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Learned
best practice
Add defensive capability checks

Add a runtime capability check to skip tests when FedCM isn't actually available
to prevent flaky failures across environments. Use pytest.skip in a setup
fixture or at test start.

py/test/selenium/webdriver/common/fedcm_tests.py [23-26]

 @pytest.mark.xfail_safari(reason="FedCM not supported")
 @pytest.mark.xfail_firefox(reason="FedCM not supported")
 @pytest.mark.xfail_ie(reason="FedCM not supported")
 @pytest.mark.xfail_remote(reason="FedCM not supported, since remote uses Firefox")
 
+def _ensure_fedcm_available(driver):
+    # Skip if FedCM capability/feature flag is missing
+    caps = getattr(driver, "capabilities", {}) or {}
+    browser_name = (caps.get("browserName") or "").lower()
+    fedcm_supported = caps.get("fedcmSupported") or caps.get("se:fedcmSupported")
+    if browser_name == "chrome" and fedcm_supported is False:
+        pytest.skip("FedCM not enabled in Chrome configuration")
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add precise validation and defensive checks around parsing and collections; when enabling tests on new platforms, ensure assumptions are guarded to avoid flaky failures.

Low
  • More

@navin772 navin772 changed the title [py]: test enabling fedcm tests for chrome [py]: enable fedcm tests for chrome Sep 15, 2025
Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@navin772 navin772 merged commit d702027 into SeleniumHQ:trunk Sep 15, 2025
17 checks passed
@navin772 navin772 deleted the test-fedcm-chrome branch September 15, 2025 13:14
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.

3 participants