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

Update tests for better Mac M1 compatibility #654

Merged

Conversation

shrekris-anyscale
Copy link
Contributor

Signed-off-by: Shreyas Krishnaswamy [email protected]

Why are these changes needed?

This change updates some test settings and error behavior to improve compatibility with M1 Macbooks. Thanks @simon-mo for the suggested changes!

Related issue number

Copies over some changes from #366.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • This change updates testing configurations.

Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Copy link
Collaborator

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DmitriGekhtman
Copy link
Collaborator

DmitriGekhtman commented Oct 25, 2022

Could you explain the changes in the issue description? (I mean the details of the changes and why these fix M1 issues.)

@shrekris-anyscale
Copy link
Contributor Author

Could you explain the changes in the issue description? (I mean the details of the changes and why these fix M1 issues.)

@simon-mo do you mind explaining the changes and why they're needed?

@simon-mo
Copy link
Collaborator

Sure. The envtests harness is used to run golang operator unit test. To run it, the test harness download pre-built binary from the internet. The current version in the codebase doesn't support m1. The updated override version supports it and is the earliest version that supports m1 binaries.

The comment out section in the test golang code is the fact that the cleanup step always fail and timeout in macOS. @brucez-anyscale and I have confirmed this independently and we should simply skip the harmless error. It doesn't occur on CI anyway.

@DmitriGekhtman DmitriGekhtman merged commit 12c0a90 into ray-project:master Oct 26, 2022
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
This change updates some test settings and error behavior to improve compatibility with M1 Macbooks. Thanks @simon-mo for the suggested changes!

Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants