Skip to content

Commit

Permalink
[fix] flaky test in proposer (MystenLabs#9809)
Browse files Browse the repository at this point in the history
## Description 

This PR is fixing a flaky test in Narwhal and more specifically when
testing our gRPC Proposer API . Basically the order of the
AuthorityFixtures wasn't aligned with the produced Committee
authorities. That made the `test_rounds_errors` test fail randomly as it
is based on parsing both the Committee and AuthorityFixtures.

Also refactored the test to iterate over the authorities using a common
approach via the `fixture.authorities()`

## Test Plan 

How did you test the new or updated feature?

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
akichidis authored Mar 24, 2023
1 parent abaea8d commit 853c3a6
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
2 changes: 1 addition & 1 deletion narwhal/primary/tests/integration_tests_proposer_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ async fn test_rounds_errors() {
// In this way, the genesis certificate is not run for that authority and is absent when we try to fetch it
let mut builder = CommitteeBuilder::new(Epoch::default());

for authority in committee.authorities() {
for authority in fixture.authorities().map(|a| a.authority()) {
if authority.id() != authority_id {
builder = builder.add_authority(
authority.protocol_key().clone(),
Expand Down
4 changes: 4 additions & 0 deletions narwhal/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,10 @@ impl<R: rand::RngCore + rand::CryptoRng> Builder<R> {
authority.authority = OnceCell::with_value(a.clone());
}

// now order the AuthorityFixtures by the authority id so when we iterate either via the
// committee.authorities() or via the fixture.authorities() we'll get the same order.
authorities.sort_by_key(|a1| a1.authority().id());

CommitteeFixture {
authorities,
committee,
Expand Down

0 comments on commit 853c3a6

Please sign in to comment.