Skip to content

[py]: enable chrome beta tests for CI-RBE #16125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2025

Conversation

navin772
Copy link
Member

@navin772 navin772 commented Aug 4, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Enabled tests for chrome beta as per discussion - #16118 (comment)

🔧 Implementation Notes

Ruby already added http archives for chrome beta and chromedriver - #15874

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Enable Chrome Beta browser testing in Python CI-RBE

  • Add Chrome Beta configuration to Bazel build system

  • Skip DevTools tests for Chrome Beta compatibility

  • Create dedicated test suite for Chrome Beta


Diagram Walkthrough

flowchart LR
  A["browsers.bzl"] --> B["Add chrome_beta_data import"]
  A --> C["Define chrome_beta_args"]
  A --> D["Add chrome-beta browser config"]
  E["BUILD.bazel"] --> F["Skip devtools_tests.py for chrome-beta"]
  E --> G["Create test-chrome-beta suite"]
  D --> H["Chrome Beta Testing Enabled"]
  G --> H
Loading

File Walkthrough

Relevant files
Enhancement
browsers.bzl
Add Chrome Beta browser configuration                                       

py/private/browsers.bzl

  • Import chrome_beta_data from common browsers configuration
  • Add chrome_beta_args with Linux and macOS Chrome Beta binary paths
  • Define new "chrome-beta" browser configuration in BROWSERS dict
+20/-0   
BUILD.bazel
Configure Chrome Beta test suites                                               

py/BUILD.bazel

  • Skip devtools_tests.py for chrome-beta in common test suites
  • Apply same exclusion to BiDi test suites for chrome-beta
  • Create dedicated test-chrome-beta test suite for Chrome Beta specific
    tests
+27/-2   

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Aug 4, 2025
@navin772
Copy link
Member Author

navin772 commented Aug 4, 2025

We should disable devtools_tests.py for chrome beta runs since we generate the pdl files for latest 3 stable releases only (correct me if I am wrong!).

@navin772 navin772 marked this pull request as ready for review August 4, 2025 14:54
Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

5678 - Not compliant

Non-compliant requirements:

• Fix ChromeDriver connection failure error that occurs after first instance
• Resolve "ConnectFailure (Connection refused)" error for subsequent ChromeDriver instances
• Ensure proper ChromeDriver instantiation for Ubuntu 16.04.4 with Chrome 65.0.3325.181 and ChromeDriver 2.35

1234 - Not compliant

Non-compliant requirements:

• Fix JavaScript execution in link href on click() for Firefox
• Ensure click() triggers JavaScript in href attribute for Firefox 42.0
• Restore functionality that worked in Selenium 2.47.1 but broke in 2.48.0/2.48.2

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logic Complexity

The conditional exclusion logic for devtools_tests.py uses inline conditionals that could be simplified for better readability and maintainability

exclude = BIDI_TESTS + ["test/selenium/webdriver/common/print_pdf_tests.py"] +
          (["test/selenium/webdriver/common/devtools_tests.py"] if browser == "chrome-beta" else []),

Copy link
Contributor

qodo-merge-pro bot commented Aug 4, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Extract duplicated conditional exclusion logic

The conditional exclusion logic is duplicated across multiple test suites.
Consider extracting this into a function or variable to improve maintainability
and reduce code duplication.

py/BUILD.bazel [444-445]

-exclude = BIDI_TESTS + ["test/selenium/webdriver/common/print_pdf_tests.py"] +
-          (["test/selenium/webdriver/common/devtools_tests.py"] if browser == "chrome-beta" else []),
+exclude = BIDI_TESTS + ["test/selenium/webdriver/common/print_pdf_tests.py"] + 
+          _get_chrome_beta_exclusions(browser),
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies duplicated conditional logic for test exclusions across two test suites and proposes a valid refactoring to improve code maintainability.

Low
  • Update

@navin772 navin772 requested review from cgoldberg and p0deje August 4, 2025 14:56
Copy link
Member

@p0deje p0deje left a comment

Choose a reason for hiding this comment

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

LGTM

@cgoldberg
Copy link
Contributor

We should disable devtools_tests.py for chrome beta runs

We should disable it everywhere :)

That test fails like 80% of the time and is our only consistently flaky test. I tried to fix it once but was unsuccessful.

@navin772
Copy link
Member Author

navin772 commented Aug 5, 2025

We should disable it everywhere :)

I agree, it's a lot flaky. Let's add it too .skipped-tests in the other PR I will open to test this.
I am not sure whether .skipped-tests is honored by CI running on GH runners.

@navin772 navin772 merged commit e7416fe into SeleniumHQ:trunk Aug 5, 2025
4 checks passed
@navin772 navin772 deleted the py-add-chrome-beta branch August 5, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants