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

Add liveness tests (BFT-387) #46

Merged
merged 9 commits into from
Dec 6, 2023
Merged

Add liveness tests (BFT-387) #46

merged 9 commits into from
Dec 6, 2023

Conversation

moshababo
Copy link
Contributor

Adding new protocol liveness tests for when the network becomes idle in various stages:

  • timeout_leader_no_prepares - when leader have no cached prepare messages for the current view
  • timeout_leader_some_prepares - when leader have some cached prepare messages for the current view
  • timeout_leader_in_commit - when leader is in commit phase
  • timeout_replica_in_commit - when replica is in commit phase
  • timeout_leader_some_commits - when leader have some cached commit messages for the current view
  • timeout_leader_in_consecutive_prepare - when leader is in a consecutive prepare phase

@pompon0
Copy link
Collaborator

pompon0 commented Dec 5, 2023

fyi, all these cast() calls are not really necessary. I'll prepare a pr to make those test utilities strongly typed (it will also make the tests easier to read).

@@ -83,8 +135,8 @@ impl UTHarness {
&self.consensus.inner.secret_key
}

pub(crate) fn owner_as_view_leader(&self) -> ViewNumber {
let mut view = self.current_replica_view();
pub(crate) fn owner_as_view_leader_next(&self) -> ViewNumber {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: IMO the "next" suffix seems to suggest at the very least that the returned view will be AFTER the current view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to imply that it's not beyond the next one.
But perhaps there will be a use case where the current is to be skipped, so I made it explicit with a longer name: owner_as_view_leader_current_or_next.

@moshababo moshababo merged commit f25f078 into main Dec 6, 2023
4 checks passed
@moshababo moshababo deleted the liveness_tests branch December 6, 2023 19:59
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.

3 participants