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

fix(ci): remove macOS tests from CI #6825

Merged
merged 1 commit into from
Jun 6, 2023
Merged

fix(ci): remove macOS tests from CI #6825

merged 1 commit into from
Jun 6, 2023

Conversation

oxarbitrage
Copy link
Contributor

@oxarbitrage oxarbitrage commented Jun 5, 2023

Motivation

We have crashes in every PR after rust upgraded to 1.70 in macOS tests. We don't know what tests are causing it so we are disabling all tests for this platform as they are not supported by Zebra in tier 1.

Fixes the CI error in #6812, but not the underlying bug.

Solution

Remove all macOS tests from CI.

Review

Anyone can review.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

Investigate whats going on and re enable if possible. #6824

@oxarbitrage oxarbitrage requested a review from a team as a code owner June 5, 2023 22:29
@oxarbitrage oxarbitrage requested review from dconnolly and removed request for a team June 5, 2023 22:29
@github-actions github-actions bot added C-bug Category: This is a bug C-removed C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Jun 5, 2023
@oxarbitrage oxarbitrage mentioned this pull request Jun 5, 2023
@teor2345 teor2345 marked this pull request as draft June 5, 2023 22:45
@teor2345
Copy link
Contributor

teor2345 commented Jun 5, 2023

I am a bit surprised, but my PR actually seems to fix the underlying bug:
#6820

The test failures I saw were easy to fix.

So let's leave this one in draft until we see if that one passes CI?

@oxarbitrage
Copy link
Contributor Author

I am a bit surprised, but my PR actually seems to fix the underlying bug: #6820

The test failures I saw were easy to fix.

So let's leave this one in draft until we see if that one passes CI?

That is even better, so yea, lets do that.

@teor2345
Copy link
Contributor

teor2345 commented Jun 6, 2023

Looks like my PR didn't work, let's try this one.

@teor2345 teor2345 marked this pull request as ready for review June 6, 2023 00:45
@teor2345 teor2345 added A-rust Area: Updates to Rust code P-High 🔥 I-crash Zebra crashes (without a panic) I-integration-fail Continuous integration fails, including build and test failures C-testing Category: These are tests and removed C-removed labels Jun 6, 2023
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thanks for this.

I left the issue open because the bug isn't actually fixed, but I changed its priority to low.

mergify bot added a commit that referenced this pull request Jun 6, 2023
mergify bot added a commit that referenced this pull request Jun 6, 2023
@mergify mergify bot merged commit 0c2107a into main Jun 6, 2023
@mergify mergify bot deleted the issue6812 branch June 6, 2023 03:35
oxarbitrage added a commit that referenced this pull request Oct 25, 2023
mergify bot pushed a commit that referenced this pull request Oct 26, 2023
* revert #6825

* remove todo comment from CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug C-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG I-crash Zebra crashes (without a panic) I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants