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

ci(unit tests): YOINK unit tests with race detector temporarily #3147

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

ramin
Copy link
Collaborator

@ramin ramin commented Jan 29, 2024

suggesting we YEET the unit test with race temporarily as the job intermittently fails across a handful of tests pretty regularly and we don't require this job to pass for a PR merge anyway.

Lets remove, i'll get it working and also enable for macos-latest which fails ALWAYS on github then we can / replace the normal unit tests with unit tests with race after we get it all running and keep CI green vs just ignoring a red x most of the time

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Reasoning makes sense to me. Let's land an issue about turning race detector back. There were plenty of cases for me where races were found on CI but not locally.

@ramin ramin enabled auto-merge (squash) January 31, 2024 12:46
@ramin ramin merged commit d7c7697 into main Jan 31, 2024
22 of 23 checks passed
@ramin ramin deleted the ci/ramin/remove-race branch January 31, 2024 12:51
renaynay pushed a commit to renaynay/celestia-node that referenced this pull request Feb 15, 2024
…stiaorg#3147)

<!--
Thank you for submitting a pull request!

Please make sure you have reviewed our contributors guide before
submitting your
first PR.

Please ensure you've addressed or included references to any related
issues.

Tips:
- Use keywords like "closes" or "fixes" followed by an issue number to
automatically close related issues when the PR is merged (e.g., "closes
#123" or "fixes #123").
- Describe the changes made in the PR.
- Ensure the PR has one of the required tags (kind:fix, kind:misc,
kind:break!, kind:refactor, kind:feat, kind:deps, kind:docs, kind:ci,
kind:chore, kind:testing)

-->

suggesting we YEET the `unit test with race` temporarily as the job
intermittently fails across a handful of tests pretty regularly and we
don't require this job to pass for a PR merge anyway.

Lets remove, i'll get it working and also enable for `macos-latest`
which fails ALWAYS on github then we can / replace the normal unit tests
with unit tests with race after we get it all running and keep CI green
vs just ignoring a red x most of the time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:ci CI related PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants