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): use FuturesOrdered in fallback_verification test #4867

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

conradoplg
Copy link
Collaborator

Motivation

The fallback_verification test assumed futures would be completed in order, but used FuturesUnordered

Specifications

Designs

Solution

Use FuturesOrdered instead.

Closes #4845

Review

I couldn't replicate the issue locally, so I can't be sure if it really fixes it, but it seems to be the proper fix.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

@conradoplg conradoplg requested a review from a team as a code owner August 2, 2022 13:39
@conradoplg conradoplg requested review from oxarbitrage and removed request for a team August 2, 2022 13:39
@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #4867 (4d2b15e) into main (0f01c9f) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4867      +/-   ##
==========================================
- Coverage   78.86%   78.76%   -0.11%     
==========================================
  Files         305      305              
  Lines       38780    38780              
==========================================
- Hits        30583    30544      -39     
- Misses       8197     8236      +39     

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.

Looks good, seems like an easy fix!

@teor2345 teor2345 added C-bug Category: This is a bug P-Critical 🚑 I-integration-fail Continuous integration fails, including build and test failures labels Aug 2, 2022
@mergify mergify bot merged commit c7f681d into main Aug 2, 2022
@mergify mergify bot deleted the ed25519-test-ordered branch August 2, 2022 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix unstable fallback_verification test
2 participants