Skip to content

[rust] Test Selenium Manager on Linux arm64 #16044

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[rust] Test Selenium Manager on Linux arm64 #16044

wants to merge 2 commits into from

Conversation

@dennisameling
Copy link

@dennisameling dennisameling commented Jul 12, 2025 •
edited by qodo-merge-pro bot
Loading

User description

🔗 Related Issues

#15801

💥 What does this PR do?

This PR ensures that the Selenium Manager test suite passes on Linux arm64, and enables CI tests for this platform through GitHub's recently released Linux arm64 runners.

🔧 Implementation Notes

I'm new to Rust, but wanted Selenium Manager to explicitly fail if users try to run it for Chrome or Edge on Linux arm64, since it's not supported. Previously, the code would silently download linux64 binaries which are for Linux x64, and cause segfaults on Linux arm64.

The setup I went for at least ensures that Firefox/Geckodriver on Linux arm64 will work and are tested properly as well.

💡 Additional Considerations

The docs at https://www.selenium.dev/documentation/selenium_manager/#alternative-architectures will need to be updated if the team is open to merging this PR. I'm happy to open a separate PR for that if desired.

🔄 Types of changes

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

PR Type

Enhancement

Description

  • Add Linux ARM64 support for Selenium Manager

  • Enable CI testing on GitHub's ARM64 runners

  • Restrict Chrome/Edge to Firefox-only on ARM64

  • Update test suite for ARM64 compatibility

Changes diagram

flowchart LR
  A["Linux ARM64 Detection"] --> B["Browser Support Check"]
  B --> C["Firefox: Supported"]
  B --> D["Chrome/Edge: Error"]
  E["Test Suite Updates"] --> F["ARM64 Conditional Logic"]
  G["CI Configuration"] --> H["GitHub ARM64 Runners"]
Loading

Changes walkthrough 📝

Relevant files
Error handling
2 files
chrome.rs
Add ARM64 unsupported error for Chrome                                     
+2/-0     
edge.rs
Add ARM64 unsupported error for Edge                                         
+3/-0     
Tests
11 files
browser_download_tests.rs
Skip non-Firefox tests on ARM64                                                   
+23/-16 
browser_tests.rs
Skip non-Firefox browser tests on ARM64                                   
+9/-0     
cache_tests.rs
Disable cache tests on ARM64                                                         
+1/-0     
config_tests.rs
Skip non-Firefox config tests on ARM64                                     
+5/-0     
exec_driver_tests.rs
Skip non-Firefox driver tests on ARM64                                     
+5/-0     
mirror_tests.rs
Use Firefox for ARM64 mirror tests                                             
+2/-2     
offline_tests.rs
Use Firefox for ARM64 offline tests                                           
+10/-5   
output_tests.rs
Use Firefox for ARM64 output tests                                             
+28/-13 
proxy_tests.rs
Use Firefox for ARM64 proxy tests                                               
+10/-4   
stable_browser_tests.rs
Skip non-Firefox stable tests on ARM64                                     
+6/-0     
webview_tests.rs
Disable webview tests on ARM64                                                     
+1/-0     
Enhancement
1 files
common.rs
Add ARM64 detection helper function                                           
+6/-0     
Configuration changes
1 files
ci-rust.yml
Add Ubuntu ARM64 runner to CI                                                       
+1/-0     
Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @dennisameling
    [rust] Enable tests in GitHub Actions on Linux arm64
    de5e3b2 
    Now that GitHub-hosted Linux arm64 runners are available,
we can start using them to test the Selenium Manager code.
    @CLAassistant
    Copy link

    CLAassistant commented Jul 12, 2025 •
    edited
    Loading

    CLA assistant check
    All committers have signed the CLA.

    @selenium-ci selenium-ci added B-build Includes scripting, bazel and CI integrations C-rust Rust code is mostly Selenium Manager B-manager Selenium Manager labels Jul 12, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 12, 2025 •
    edited
    Loading

    PR Reviewer Guide 🔍

    (Review updated until commit a88d4e1)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    15801 - Partially compliant

    Compliant requirements:

    • Enable CI builds for ARM64 systems (Linux arm64 runner added)

    Non-compliant requirements:

    • Add official support for Windows ARM64 (WoA) platform (only Linux arm64 is addressed)
    • Distribute native Selenium binaries for ARM64 (Windows ARM64 binaries not provided)

    Requires further human verification:

    • Verify that the Linux arm64 CI runner actually works and produces correct results
    • Confirm that Firefox/Geckodriver functionality works properly on Linux arm64
    • Test that the error messages for unsupported browsers are user-friendly

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

    Wrong Error Message

    The error message mentions "Google Chrome" instead of "Microsoft Edge" when rejecting Linux arm64 for Edge browser

     
        return Err(anyhow!("Linux arm64 is not supported yet by Google Chrome. Please try another browser."));
} else {
    Logic Error

    The conditional logic for skipping Edge on Windows appears inverted and may cause incorrect test behavior

     
    if browser.eq("edge") && OS.eq("windows") {
  return
} else if OS.eq("linux") && ARCH.eq("aarch64") && !browser.eq("firefox") {
  return
}

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 12, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Latest suggestions up to a88d4e1
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Fix incorrect browser name in error

    The error message mentions "Google Chrome" but this is in the Edge manager. The
    error message should reference Microsoft Edge instead of Google Chrome to avoid
    confusion.

    rust/src/edge.rs [293]

    -return Err(anyhow!("Linux arm64 is not supported yet by Google Chrome. Please try another browser."));
+return Err(anyhow!("Linux arm64 is not supported yet by Microsoft Edge. Please try another browser."));
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a copy-paste error in a user-facing error message, improving clarity and correctness.

    		Medium
    • More

    Previous suggestions

    Suggestions up to commit 19d72d6
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Replace panic with error result

    Using panic! for unsupported platform combinations is too aggressive and will
    crash the entire application. Consider returning a proper error result instead
    to allow graceful error handling.

    rust/src/chrome.rs [386-387]

     } else if LINUX.is(os) && ARM64.is(arch) {
-    panic!("Linux arm64 is not supported yet by Google Chrome. Please try another browser.")
+    return Err(anyhow!("Linux arm64 is not supported yet by Google Chrome. Please try another browser."));
    Suggestion importance[1-10]: 8

    __

    Why: Using panic! causes the program to crash, whereas returning a Result::Err allows for graceful error handling by the caller, which is a significant improvement in robustness.

    		Medium

    @dennisameling
    [rust] Selenium Manager: Update tests to work with Linux arm64
    a88d4e1 
    Currently, only Firefox and Geckodriver have official support
for Linux arm64. This commit ensures that the Selenium Manager
test suite passes on this platform, by skipping tests on non-
Firefox browsers.
    @dennisameling dennisameling force-pushed the add-selenium-manager-linux-arm64 branch from 19d72d6 to a88d4e1 Compare July 12, 2025 13:40
    @dennisameling dennisameling reopened this Jul 12, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    B-build Includes scripting, bazel and CI integrations B-manager Selenium Manager C-rust Rust code is mostly Selenium Manager Review effort 3/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @dennisameling @CLAassistant @selenium-ci