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/ updated mempool sync protocol tests #3607

Merged
merged 5 commits into from
Mar 21, 2023
Merged

Conversation

pavitthrap
Copy link
Contributor

@pavitthrap pavitthrap commented Mar 9, 2023

Description

When working on tests for the microblock tip sync protocol, I realized that ibd was set to true in the call to TestPeer::step(). Since ibd=true, the mempool (and microblock) sync protocol were effectively skipped over in the unit test. To avoid this problem, I am passing ibd as a parameter to step_with_ibd.

The mempool sync tests are now fast (because the protocol now runs correctly), so I was also able to remove the timeout. I thus also removed the #[ignore] above the tests.

@pavitthrap pavitthrap requested a review from jcnelson March 9, 2023 18:15
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #3607 (441490e) into develop (c825e1e) will decrease coverage by 2.41%.
The diff coverage is 0.68%.

@@             Coverage Diff             @@
##           develop    #3607      +/-   ##
===========================================
- Coverage    31.08%   28.68%   -2.41%     
===========================================
  Files          298      298              
  Lines       276125   276115      -10     
===========================================
- Hits         85844    79191    -6653     
- Misses      190281   196924    +6643     
Impacted Files Coverage Δ
src/net/mod.rs 18.04% <0.00%> (-0.42%) ⬇️
src/net/p2p.rs 53.34% <0.70%> (+1.37%) ⬆️

... and 99 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@diwakergupta
Copy link
Member

The mempool sync tests are now fast (because the protocol now runs correctly), so I was also able to remove the timeout. I thus also removed the #[ignore] above the tests.

👏🏽 👏🏽 Thanks Pavi!!

Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

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

LGTM!

@jcnelson jcnelson merged commit c622b89 into develop Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants