Skip to content

test(sync): Improve test_sync_rotate to prevent false positives #816

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

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

msbrogli
Copy link
Member

@msbrogli msbrogli commented Oct 17, 2023

Background

The test_sync_rotate() had an assert with 0.0877% change of failing. As every CI job executes it 6 times, it had a change of 0.52% of failing, i.e., 1 out of 200 CI executions.

Acceptance Criteria

  1. Modify test_sync_rotate() to reduce the chance of failing to 0.000483%, which reduces the chance of a CI failing from 0.52% to 0.00289797% (1 out of 35,000 CI executions).

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@msbrogli msbrogli requested a review from jansegre as a code owner October 17, 2023 23:56
@msbrogli msbrogli changed the base branch from master to test/fix-flaky-test_many_miners_since_beginning October 17, 2023 23:57
@msbrogli msbrogli requested a review from glevco October 18, 2023 00:05
@msbrogli msbrogli self-assigned this Oct 18, 2023
@msbrogli msbrogli added the tests label Oct 18, 2023
glevco
glevco previously approved these changes Oct 18, 2023
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (e305645) 84.65% compared to head (a9b2a4c) 84.60%.
Report is 2 commits behind head on master.

❗ Current head a9b2a4c differs from pull request most recent head 3c9bac8. Consider uploading reports for the commit 3c9bac8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #816      +/-   ##
==========================================
- Coverage   84.65%   84.60%   -0.06%     
==========================================
  Files         266      266              
  Lines       22123    22131       +8     
  Branches     3379     3380       +1     
==========================================
- Hits        18728    18723       -5     
- Misses       2734     2745      +11     
- Partials      661      663       +2     
Files Coverage Δ
hathor/simulator/fake_connection.py 81.34% <44.44%> (-3.52%) ⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jansegre
jansegre previously approved these changes Oct 18, 2023
@msbrogli msbrogli force-pushed the test/fix-flaky-test_many_miners_since_beginning branch 2 times, most recently from 96167d7 to 6b039c1 Compare October 18, 2023 15:33
Base automatically changed from test/fix-flaky-test_many_miners_since_beginning to master October 18, 2023 15:36
@msbrogli msbrogli dismissed stale reviews from jansegre and glevco October 18, 2023 15:36

The base branch was changed.

@msbrogli msbrogli force-pushed the test/fix-flaky-test_sync_enabled branch from a9b2a4c to 3c9bac8 Compare October 18, 2023 15:37
@msbrogli msbrogli merged commit 3c9bac8 into master Oct 18, 2023
@msbrogli msbrogli deleted the test/fix-flaky-test_sync_enabled branch October 18, 2023 15:38
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants