Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Change AT discard RPC tests to be more reliable by checking discard using proposals #1411

Merged
merged 5 commits into from
May 10, 2019
Merged

Change AT discard RPC tests to be more reliable by checking discard using proposals #1411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9789fc9
Change AT discard RPC tests to be more reliable by checking discard u…
jframe May 8, 2019
7e542c5
only need single initial validator
jframe May 9, 2019
d471e36
check both add and remove with discard
jframe May 9, 2019
aa02a7a
Merge branch 'master' into discard_at_use_proposals
jframe May 9, 2019
b880599
Merge branch 'master' into discard_at_use_proposals
jframe May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 19 additions & 22 deletions ...st/java/tech/pegasys/pantheon/tests/acceptance/clique/CliqueDiscardRpcAcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,24 @@ public class CliqueDiscardRpcAcceptanceTest extends AcceptanceTestBase {

@Test
public void shouldDiscardVotes() throws IOException {
final String[] initialValidators = {"miner1", "miner2"};
final PantheonNode minerNode1 =
pantheon.createCliqueNodeWithValidators("miner1", initialValidators);
final PantheonNode minerNode2 =
pantheon.createCliqueNodeWithValidators("miner2", initialValidators);
final PantheonNode minerNode3 =
pantheon.createCliqueNodeWithValidators("miner3", initialValidators);
cluster.start(minerNode1, minerNode2, minerNode3);

minerNode1.execute(cliqueTransactions.createRemoveProposal(minerNode2));
minerNode2.execute(cliqueTransactions.createRemoveProposal(minerNode2));
minerNode1.execute(cliqueTransactions.createAddProposal(minerNode3));
minerNode2.execute(cliqueTransactions.createAddProposal(minerNode3));
minerNode1.execute(cliqueTransactions.createDiscardProposal(minerNode2));
minerNode1.execute(cliqueTransactions.createDiscardProposal(minerNode3));

minerNode1.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 2));

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2));
minerNode1.verify(clique.noProposals());
minerNode2.verify(
clique.proposalsEqual().removeProposal(minerNode2).addProposal(minerNode3).build());
final String[] validators = {"validator1", "validator3"};
final PantheonNode validator1 =
pantheon.createCliqueNodeWithValidators("validator1", validators);
final PantheonNode validator2 =
pantheon.createCliqueNodeWithValidators("validator2", validators);
final PantheonNode validator3 =
pantheon.createCliqueNodeWithValidators("validator3", validators);
cluster.start(validator1, validator2, validator3);

validator1.execute(cliqueTransactions.createAddProposal(validator2));
validator1.execute(cliqueTransactions.createRemoveProposal(validator3));
validator1.verify(
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a race condition? ...as you can't be entire sure of the ordering of the proposals (assuming verify checks ordering)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually builds a map of address, boolean so the order doesn't matter.

clique.proposalsEqual().addProposal(validator2).removeProposal(validator3).build());

validator1.execute(cliqueTransactions.createDiscardProposal(validator2));
validator1.verify(clique.proposalsEqual().removeProposal(validator3).build());

validator1.execute(cliqueTransactions.createDiscardProposal(validator3));
cluster.verify(clique.noProposals());
}
}
21 changes: 8 additions & 13 deletions ...c/test/java/tech/pegasys/pantheon/tests/acceptance/ibft/IbftDiscardRpcAcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,21 @@ public class IbftDiscardRpcAcceptanceTest extends AcceptanceTestBase {

@Test
public void shouldDiscardVotes() throws IOException {
final String[] validators = {"validator1", "validator2"};
final String[] validators = {"validator1", "validator3"};
final PantheonNode validator1 = pantheon.createIbftNodeWithValidators("validator1", validators);
final PantheonNode validator2 = pantheon.createIbftNodeWithValidators("validator2", validators);
final PantheonNode validator3 = pantheon.createIbftNodeWithValidators("validator3", validators);
cluster.start(validator1, validator2, validator3);

validator1.execute(ibftTransactions.createRemoveProposal(validator2));
validator1.execute(ibftTransactions.createAddProposal(validator3));

validator2.execute(ibftTransactions.createRemoveProposal(validator2));
validator2.execute(ibftTransactions.createAddProposal(validator3));
validator1.execute(ibftTransactions.createAddProposal(validator2));
validator1.execute(ibftTransactions.createRemoveProposal(validator3));
validator1.verify(
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto; race condition around request ordering?

ibft.pendingVotesEqual().addProposal(validator2).removeProposal(validator3).build());

validator1.execute(ibftTransactions.createDiscardProposal(validator2));
validator1.execute(ibftTransactions.createDiscardProposal(validator3));
validator1.verify(ibft.pendingVotesEqual().removeProposal(validator3).build());

validator1.waitUntil(wait.chainHeadHasProgressedByAtLeast(validator1, 2));

cluster.verify(ibft.validatorsEqual(validator1, validator2));
validator1.verify(ibft.noProposals());
validator2.verify(
ibft.pendingVotesEqual().removeProposal(validator2).addProposal(validator3).build());
validator1.execute(ibftTransactions.createDiscardProposal(validator3));
cluster.verify(ibft.noProposals());
}
}