[rust] Fix Windows arm64 logic for Selenium Manager and prepare CI for running the test suite#16046
[rust] Fix Windows arm64 logic for Selenium Manager and prepare CI for running the test suite#16046dennisameling wants to merge 14 commits into
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
When this happens, all the bindings will need to be updated to use the correct version of Selenium Manager when running on ARM. Currently, we ship all versions of Selenium Manager on all platforms. When runninng on Windows, the bindings just use |
|
So the tests in CI have failed. It's trying to pass MSVC build arguments like Interestingly, doing I'm not sure why the |
|
I'll see if someone with better knowledge of Rust and Bazel than me can take a look. btw, feel free to join other developers in chat via Slack/IRC/Matrix. There are usually people monitoring the #selenium and #selenium-tlc channels: |
a026be4 to
f7e53c3
Compare
|
@cgoldberg GitHub actions had an outage yesterday, causing the jobs to have been skipped. Mind kicking off the pipeline one more time? Thanks! |
|
1 test failed in CI, but it's totally unrelated to these changes.. I'll kick it off again just to see if we get a fully green run. |
| walkdir = "2.5.0" | ||
| debpkg = "0.6.0" | ||
| anyhow = { version = "1.0.100", default-features = false, features = ["backtrace", "std"] } | ||
| apple-flat-package = "0.20.0" |
There was a problem hiding this comment.
The apple-flat-package package transitively pulls in ring via apple-xar → cryptographic-message-syntax → ring which causes the Bazel tests to fail on Windows arm64. The related issue is briansmith/ring#2215. It passes MSVC build flags to the clang compiler which it was forced to use on Windows arm64.
There was a problem hiding this comment.
ring just merged in briansmith/ring#2803, which should fix the build on Windows arm64. They haven't released a new version yet, but have been merging a lot of PRs in the last few weeks, so I'd expect a new release relatively soon.
| [target.'cfg(all(windows, target_arch = "aarch64"))'.dependencies] | ||
| reqwest = { version = "0.12.23", default-features = false, features = ["native-tls"] } | ||
|
|
||
| [target.'cfg(not(all(windows, target_arch = "aarch64")))'.dependencies] | ||
| reqwest = { version = "0.12.23", default-features = false, features = ["rustls-tls"] } |
There was a problem hiding this comment.
Same problem with the ring dependency here; using rustls-tls pulls in ring and causes the Bazel tests to fail
There was a problem hiding this comment.
ring just merged in briansmith/ring#2803, which should fix the build on Windows arm64. They haven't released a new version yet, but have been merging a lot of PRs in the last few weeks, so I'd expect a new release relatively soon.
| supported_platform_triples = [ | ||
| "aarch64-apple-darwin", | ||
| "aarch64-pc-windows-msvc", | ||
| "aarch64-unknown-linux-gnu", | ||
| "x86_64-apple-darwin", | ||
| "x86_64-pc-windows-msvc", | ||
| "x86_64-unknown-linux-gnu", | ||
| ], |
There was a problem hiding this comment.
The default value here is ["aarch64-apple-darwin", "aarch64-unknown-linux-gnu", "wasm32-unknown-unknown", "wasm32-wasip1", "x86_64-pc-windows-msvc", "x86_64-unknown-linux-gnu", "x86_64-unknown-nixos-gnu"], so I had to explicitly add aarch64-pc-windows-msvc, otherwise Bazel would happily skip the build and tests on Windows arm64. That caused the tests to "succeed" in CI, but in reality they were just skipped. Docs: https://bazelbuild.github.io/rules_rust/crate_universe_bzlmod.html#from_cargo
|
Just ran some tests locally on a Windows arm64 VM: Note that Chrome for Testing doesn't support Windows arm64 yet (as opposed to the main Chrome browser that users would typically download), so it falls back to the x64 version that can run under emulation on Windows arm64: |
|
Hi @dennisameling and @cgoldberg |
User description
🔗 Related Issues
#15801
💥 What does this PR do?
This PR fixes two issues that were preventing the Rust test suite to fully pass on Windows on ARM. While running the tests locally on an arm64 machine, I ran into these errors:
and
It turned out that the arch names for Windows arm64 were incorrect in the Rust code. Confirmed that these are valid URLs:
I want to emphasize that Windows ARM64 can run x64 binaries under emulation. So even though e.g. Chrome doesn't have a native Chromedriver yet, Windows ARM64 can run the x64 version under emulation.
This means that the following browsers/drivers will work on Windows ARM64:
I can confirm that the Rust test suite is passing locally on my Windows arm64 machine.
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
PR Type
Bug fix
Description
Fix Windows ARM64 platform labels in Selenium Manager
Correct Firefox ARM64 URL path from
win-aarch64towin64-aarch64Fix Electron ARM64 platform label from
win32-arm64-x64towin32-arm64Add Windows ARM64 CI testing support
Changes diagram
Changes walkthrough 📝
electron.rs
Correct Electron ARM64 platform labelrust/src/electron.rs
win32-arm64-x64towin32-arm64firefox.rs
Correct Firefox ARM64 platform labelrust/src/firefox.rs
win-aarch64towin64-aarch64ci-rust.yml
Enable Windows ARM64 CI testing.github/workflows/ci-rust.yml
windows-11-armto CI test matrix