Skip to content
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

[java] Remove WebDriverException When Using Linux ARM64 #15149

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Jan 24, 2025

User description

Description

Per talk in #selenium-tlc slack w/ @p0deje , Linux ARM64 can now be supported by Selenium Manager.

Motivation and Context

Error no longer necessary once arm64 binary is shipped.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement, Tests


Description

  • Removed WebDriverException for Linux ARM64 in Selenium Manager.

  • Updated logic to support Linux ARM64 without exceptions.

  • Adjusted tests to reflect Linux ARM64 support.


Changes walkthrough 📝

Relevant files
Enhancement
SeleniumManager.java
Remove exception and enable Linux ARM64 support                   

java/src/org/openqa/selenium/manager/SeleniumManager.java

  • Removed WebDriverException for Linux ARM64 architecture.
  • Simplified logic to assign the linux folder for Linux systems.
  • +1/-5     
    Tests
    selenium_manager_tests.py
    Adjust tests for Linux ARM64 support                                         

    py/test/selenium/webdriver/common/selenium_manager_tests.py

  • Removed test case for WebDriverException on Linux ARM64.
  • Updated Linux test to validate binary path without exceptions.
  • +4/-8     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Error Handling

    Consider adding a validation or log message when running on Linux ARM64 to inform users about the experimental support status

    } else if (current.is(LINUX)) {
      folder = "linux"

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix missing semicolon in assignment

    Add a semicolon after the 'linux' string assignment to fix the syntax error in the
    folder assignment.

    java/src/org/openqa/selenium/manager/SeleniumManager.java [200]

    -folder = "linux"
    +folder = "linux";
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: Missing semicolon in Java code is a syntax error that would prevent compilation. This is a critical fix needed for the code to work properly.

    10

    @nvborisenko
    Copy link
    Member

    Wow! Really? Does it mean we can execute SM binary on ARM right now?

    @shbenzer
    Copy link
    Contributor Author

    We are now able to compile SM in arm64 https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/, we just have to add it to the bindings. I'm adding this pr in advance so it's ready if/when we decide to ship it.

    @nvborisenko
    Copy link
    Member

    Thanks. PR in advance: please mark it as draft. We don't know yet when/how SM will be shipped.

    @diemol
    Copy link
    Member

    diemol commented Jan 24, 2025

    I think the limitation has not been if we can or not compile it on ARM, but rather if all browser and browser drivers binaries are available for ARM.

    It adds little value if SM is available for ARM but no browser and browser drivers binaries are available.

    Do we know which ones are available yet?

    @shbenzer shbenzer marked this pull request as draft January 24, 2025 20:40
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants