-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[rust] Test Selenium Manager on Linux arm64 #16045
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
base: trunk
Are you sure you want to change the base?
[rust] Test Selenium Manager on Linux arm64 #16045
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||
|
Thanks. I'll leave code review for someone who knows rust. But I'm a little confused...
I thought we weren't building Selenium Manager on ARM. Were you running the linux64 binaries through QEMU or something? Does this PR enable building Selenium Manager for arm64, or just handle the situation when trying to run the linux64 binaries on arm64? If the former, don't we need to update the various bindings and packaging to accommodate this? ... or is this just a step towards enabling Selenium on ARM64? |
|
Sorry for the confusion here.
Indeed. There have been no Selenium Manager builds for ARM. This PR is a step towards enabling Selenium on ARM64. The reason I mentioned This PR will at least ensure that users of Linux arm64 can use Firefox, which has official binaries for this platform 👍🏼 whenever Edge and Chrome start publishing native binaries too, they can be added through a follow-up PR. |
|
OK... that makes sense, but it's probably more appropriate to make these changes if/when we start building SM for ARM since these code paths won't be tested or used until then. |
|
With regards to testing, that's why I added https://github.com/SeleniumHQ/selenium/pull/16045/files#diff-5162db340096b281e8c77ef016f0595e9b7a63e9cb48605a43b685301b4b5ee0R38 so that it will start running the tests on native Linux arm64 runners. GitHub released those in Public Preview in January of this year, and quite some open source projects have started using them already. I was hoping - by just adding the testing part to CI for now - that the team can at least be confident that the code will work on Linux arm64, and leave the actual builds/publishing for a follow-up PR. But happy to look into that part as well if helpful. |
|
@bonigarcia, do we need to explicitly build a SM binary for ARM? |
|
The code looks good to me (thanks a lot for contributing, @dennisameling!). I don't have an ARM machine to test it. Let's see what CI says about it. @diemol Yes, we would need to build SM for ARM in Linux. And then, we would need to distribute the new binary together with the other three. Also, the bindings would need to discover the system architecture (in Linux) to use the X64 or ARM SM binary. |
|
Given that all the tests are passing, I just added the necessary logic to publish builds for Linux arm64 as well. I see that some preparations have already been done to start using them in the Python binding. Is there anything I can help with to push this over the finish line? Thanks! 😊 |
|
Thanks! I was the one who did the work to prepare the Python bindings. I want to test building and running Selenium Manager and the Python bindings locally for Linux arm64. Since my only machine is x86, I am going to get a Raspberry Pi next weekend (I'm not even sure if that's the right ARM architecture?) Hopefully I can get things building and running, and that will give us more confidence to move forward with building for ARM in CI. Your changes are very helpful and I will update this issue in a week or so once I've had time to play with it. |
Nice! The Raspberry Pi 3, 4 and 5 are all arm64, so you should be good there. I've been testing natively on an Ubuntu arm64 VM in Parallels on Mac, and can confirm that at least the Rust part is working as expected 👍🏼 |
|
I bought a raspberry pi, and everything was surprisingly easy to setup. I was able to build selenium-manager on Raspberry Pi OS (Debian Stable) without problems using our Python source package and |
| prerelease: false | ||
| files: | | ||
| artifacts/selenium-manager-linux/selenium-manager-linux | ||
| artifacts/selenium-manager-linux-arm64/selenium-manager-linux-arm64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realizing I forgot to add artifacts/selenium-manager-linux-arm64-debug/selenium-manager-linux-arm64-debug.tar to this list, so let's keep that in mind as we continue working on this PR.
Now that GitHub-hosted Linux arm64 runners are available, we can start using them to test the Selenium Manager code.
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.
34cbe19 to
eaa4a43
Compare
|
@cgoldberg is there anything I can do to help push this over the finish line? Happy to update/change things if needed. Thanks! 😊 |
|
I should have some time to get back to it soon and I'll let you know if there's anything else you can do. I appreciate your patience 🙂 |
|
@cgoldberg quick reminder for this one. Anything I can do to help? Thanks! 🙏🏼 |
|
Thanks for your patience on this. There hasn't been much interest from the other maintainers in shipping ARM binaries for Selenium Manager. It's mostly due to percieved low interest/useage on ARM platforms (I don't necesarrily agree), lack of driver/browser support on ARM, and some concerns about inflating our package sizes (we include binaries for all platforms in universal packages).
Edit: I guess it's better to leave the CI as-is.. just because we build the binaries doesn't mean we need to include them as part of packages yet. Disregard my suggestion above and let's just try to get the issues in my followup comments fixed. |
|
I checked out your branch, merged trunk, and fixed the merge conflicts and formatting issues. I committed those changes and pushed them to your branch. Can you make sure my changes look OK? |
|
Right now I am getting 5 failures when i run I'm running on Debian ARM64 13 (Raspberry Pi OS) with the latest rust toolchain. I am building/testing with cargo, not with Bazel, so I don't know if that does anything different? (Bazel is giving me issues on ARM) The failures all look like: I also built selenium-manager from trunk branch, and I get the same failures: ... so it's not something your code introduced, but it is something we need to investigate and fix. Do you have any idea what those failures mean, and can you reproduce them? |
afe492f to
b2fa08d
Compare
|
The tests that fail for me locally pass on CI. Maybe I'm missing something by not using Bazel? A different test failed on CI: https://github.com/SeleniumHQ/selenium/actions/runs/19880823913/job/56979362597?pr=16045#step:19:811 |
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
linux64binaries 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
🔄 Types of changes
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
Changes walkthrough 📝
2 files
Add ARM64 unsupported error for ChromeAdd ARM64 unsupported error for Edge11 files
Skip non-Firefox tests on ARM64Skip non-Firefox browser tests on ARM64Disable cache tests on ARM64Skip non-Firefox config tests on ARM64Skip non-Firefox driver tests on ARM64Use Firefox for ARM64 mirror testsUse Firefox for ARM64 offline testsUse Firefox for ARM64 output testsUse Firefox for ARM64 proxy testsSkip non-Firefox stable tests on ARM64Disable webview tests on ARM641 files
Add ARM64 detection helper function1 files
Add Ubuntu ARM64 runner to CI