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(test): simplify resource conflict test to avoid CI failures #3537

Merged
merged 2 commits into from
Feb 15, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Feb 15, 2022

Motivation

Previously, the resource conflict tests would wait 10 seconds for the process to launch. But I think this might be causing test failures.

Now the test waits until the process has actually used the conflicting resource.

Solution

Closes #3489.

Review

Anyone can review this PR.

It's causing 1-3 test failures per day, so I'm marking it as high priority.

Reviewer Checklist

  • Test Changes Make Sense
  • Tests Pass

Previously, the test would wait 10 seconds for the process to launch.
Now it waits until the process has used the conflicting resource.
@teor2345 teor2345 requested a review from oxarbitrage February 15, 2022 00:43
@teor2345 teor2345 self-assigned this Feb 15, 2022
@teor2345
Copy link
Contributor Author

The deny.toml CI failed due to temporary alpine linux Docker network errors.
We can restart them after a few minutes, or just ignore them for this PR.

(This PR doesn't change any dependencies, so there's nothing new for them to check.)

@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #3537 (71e7240) into main (499ae89) will increase coverage by 2.10%.
The diff coverage is 80.01%.

@@            Coverage Diff             @@
##             main    #3537      +/-   ##
==========================================
+ Coverage   78.34%   80.44%   +2.10%     
==========================================
  Files         267      274       +7     
  Lines       31526    32229     +703     
==========================================
+ Hits        24698    25926    +1228     
+ Misses       6828     6303     -525     

Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

Looks good and more reliable

@mergify mergify bot merged commit 08828c1 into main Feb 15, 2022
@mergify mergify bot deleted the fix-conflict-test-ci branch February 15, 2022 15:30
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.

zebra_zcash_listener_conflict test occasionally fails
2 participants