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

[rust] Fix local architecture discovery in Selenium Manager #11611

Merged
merged 2 commits into from
Mar 6, 2023

Conversation

bonigarcia
Copy link
Member

Description

This PR fixes a bug reported in #11517, related to the architecture discovery (e.g., arm64, x86_64) in Selenium Manager.

Motivation and Context

So far, the Selenium Manager logic relied on a Rust standard constant called std::env::consts::ARCH for the architecture discovery. According to its doc, this constant is "A string describing the architecture of the CPU that is currently in use". Nevertheless, when compiled for a x86_64, like in CI (for instance, for macOS, using cargo build --release --target x86_64-apple-darwin), that constant is always x86_64 (even when executed in a macOS M1). The solution I found to fix this issue is to rely on the command uname (in UNIX/Linux) and the environment variable (PROCESSOR_ARCHITECTURE) in Windows for the automatic discovery of the underlying architecture.

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.

@diemol
Copy link
Member

diemol commented Mar 3, 2023

I see issue #11517 is closed, is this still needed?

@bonigarcia
Copy link
Member Author

Yes, this PR still needs to be merged.

@diemol
Copy link
Member

diemol commented Mar 6, 2023

👍
Can you please fix the conflicts so we can merge?

@bonigarcia bonigarcia merged commit d7b0b09 into trunk Mar 6, 2023
@bonigarcia bonigarcia deleted the se_mgr_arch branch March 6, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants