Skip to content

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

Merged
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:py-add-chrome-beta
Aug 5, 2025
Merged

[py]: enable chrome beta tests for CI-RBE#16125
navin772 merged 3 commits intoSeleniumHQ:trunkfrom
navin772:py-add-chrome-beta

Conversation

@navin772
Copy link
Member

@navin772 navin772 commented Aug 4, 2025 •
edited by qodo-code-review bot
Loading

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 •
edited
Loading

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
@qodo-code-review
Copy link
Contributor

qodo-code-review 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 []),

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Aug 4, 2025 •
edited
Loading

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
p0deje
p0deje approved these changes Aug 5, 2025
View reviewed changes
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
Member

cgoldberg commented Aug 5, 2025

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

Reviewers

@p0deje p0deje p0deje approved these changes

@cgoldberg cgoldberg cgoldberg approved these changes

Assignees

No one assigned

Labels

B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@navin772 @cgoldberg @p0deje @selenium-ci