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

AT DSL - Removing WaitCondition, consistently applying Condition instead #1513

Merged
merged 8 commits into from
Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@ public void setUp() throws Exception {
@Test
public void shouldBeAbleToGetValidatorsForBlockNumber() {
cluster.verify(clique.validatorsAtBlockEqual("0x0", minerNode1));
minerNode1.waitUntil(wait.chainHeadIsAtLeast(1));
minerNode1.verify(blockchain.minimumHeight(1));

minerNode1.execute(cliqueTransactions.createAddProposal(minerNode2));
cluster.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
cluster.verify(blockchain.minimumHeightProgression(minerNode1, 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

progression sounds a bit odd here. Is there a consistent phrasing or tense used or meant to be used for the dsl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggested alternative?

Copy link
Contributor Author

@CjHare CjHare Jun 3, 2019

Choose a reason for hiding this comment

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

blockchain.reachesHeight() ?

cluster.verify(clique.validatorsAtBlockEqual("0x2", minerNode1, minerNode2));
cluster.verify(clique.validatorsAtBlockEqual(LATEST, minerNode1, minerNode2));
}

@Test
public void shouldBeAbleToGetValidatorsForBlockHash() {
cluster.verify(clique.validatorsAtBlockHashFromBlockNumberEqual(minerNode1, 0, minerNode1));
minerNode1.waitUntil(wait.chainHeadIsAtLeast(1));
minerNode1.verify(blockchain.minimumHeight(1));

minerNode1.execute(cliqueTransactions.createAddProposal(minerNode2));
cluster.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
cluster.verify(blockchain.minimumHeightProgression(minerNode1, 1));
cluster.verify(
clique.validatorsAtBlockHashFromBlockNumberEqual(minerNode1, 2, minerNode1, minerNode2));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
package tech.pegasys.pantheon.tests.acceptance.clique;

import tech.pegasys.pantheon.tests.acceptance.dsl.AcceptanceTestBase;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique.ExpectNonceVote.CLIQUE_NONCE_VOTE;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode;
import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitCondition;

import java.io.IOException;

Expand All @@ -35,10 +35,10 @@ public void shouldAddValidators() throws IOException {
cluster.start(minerNode1, minerNode2, minerNode3);

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2));
final WaitCondition cliqueValidatorsChanged = wait.cliqueValidatorsChanged(minerNode1);
final Condition cliqueValidatorsChanged = clique.awaitSignerSetChange(minerNode1);
minerNode1.execute(cliqueTransactions.createAddProposal(minerNode3));
minerNode2.execute(cliqueTransactions.createAddProposal(minerNode3));
cluster.waitUntil(cliqueValidatorsChanged);
cluster.verify(cliqueValidatorsChanged);
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be removed - if we're ditching the "waiting" concept, then no need to ensure miner1 has changed - just ensure all the cluster have in the next line (and thus line 38 can probably go too).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2, minerNode3));
}

Expand All @@ -54,10 +54,10 @@ public void shouldRemoveValidators() throws IOException {
cluster.start(minerNode1, minerNode2, minerNode3);

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2, minerNode3));
final WaitCondition cliqueValidatorsChanged = wait.cliqueValidatorsChanged(minerNode1);
final Condition cliqueValidatorsChanged = clique.awaitSignerSetChange(minerNode1);
minerNode1.execute(cliqueTransactions.createRemoveProposal(minerNode3));
minerNode2.execute(cliqueTransactions.createRemoveProposal(minerNode3));
cluster.waitUntil(cliqueValidatorsChanged);
cluster.verify(cliqueValidatorsChanged);
cluster.verify(clique.validatorsEqual(minerNode1, minerNode2));
}

Expand All @@ -74,7 +74,7 @@ public void shouldNotAddValidatorWhenInsufficientVotes() throws IOException {

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2));
minerNode1.execute(cliqueTransactions.createAddProposal(minerNode3));
minerNode1.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
minerNode1.verify(blockchain.minimumHeightProgression(minerNode1, 1));
cluster.verify(clique.validatorsEqual(minerNode1, minerNode2));
}

Expand All @@ -87,7 +87,7 @@ public void shouldNotRemoveValidatorWhenInsufficientVotes() throws IOException {

cluster.verify(clique.validatorsEqual(minerNode1, minerNode2, minerNode3));
minerNode1.execute(cliqueTransactions.createRemoveProposal(minerNode3));
minerNode1.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
minerNode1.verify(blockchain.minimumHeightProgression(minerNode1, 1));
cluster.verify(clique.validatorsEqual(minerNode1, minerNode2, minerNode3));
}

Expand All @@ -103,12 +103,12 @@ public void shouldIncludeVoteInBlockHeader() throws IOException {
cluster.start(minerNode1, minerNode2, minerNode3);

minerNode1.execute(cliqueTransactions.createAddProposal(minerNode3));
minerNode1.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
minerNode1.verify(blockchain.minimumHeightProgression(minerNode1, 1));
minerNode1.verify(blockchain.beneficiaryEquals(minerNode3));
minerNode1.verify(clique.nonceVoteEquals(CLIQUE_NONCE_VOTE.AUTH));

minerNode1.execute(cliqueTransactions.createRemoveProposal(minerNode2));
minerNode1.waitUntil(wait.chainHeadHasProgressedByAtLeast(minerNode1, 1));
minerNode1.verify(blockchain.minimumHeightProgression(minerNode1, 1));
minerNode1.verify(blockchain.beneficiaryEquals(minerNode2));
minerNode1.verify(clique.nonceVoteEquals(CLIQUE_NONCE_VOTE.DROP));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.net.NetTransactions;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.perm.PermissioningTransactions;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.web3.Web3Transactions;
import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitConditions;

import org.junit.After;

Expand All @@ -60,7 +59,6 @@ public class AcceptanceTestBase {
protected final PermissioningTransactions permissioningTransactions;
protected final ContractTransactions contractTransactions;
protected final Web3 web3;
protected final WaitConditions wait;

protected AcceptanceTestBase() {
ethTransactions = new EthTransactions();
Expand All @@ -83,7 +81,6 @@ protected AcceptanceTestBase() {
web3 = new Web3(new Web3Transactions());
pantheon = new PantheonNodeFactory();
contractVerifier = new ContractVerifier(accounts.getPrimaryBenefactor());
wait = new WaitConditions(ethTransactions, cliqueTransactions, ibftTwoTransactions);
permissionedNodeBuilder = new PermissionedNodeBuilder();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@

import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.blockchain.ExpectBeneficiary;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.blockchain.ExpectBlockNumberAbove;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.blockchain.ExpectMinimumBlockNumber;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.eth.EthTransactions;

import java.math.BigInteger;

public class Blockchain {

private final EthTransactions eth;
Expand All @@ -28,10 +31,32 @@ public Blockchain(final EthTransactions eth) {
}

public Condition blockNumberMustBeLatest(final Node node) {
return new ExpectMinimumBlockNumber(eth, node.execute(eth.blockNumber()));
return new ExpectMinimumBlockNumber(eth, currentHeight(node));
}

public Condition beneficiaryEquals(final PantheonNode node) {
return new ExpectBeneficiary(eth, node);
}

public Condition minimumHeight(final long blockNumber) {
return new ExpectBlockNumberAbove(eth, BigInteger.valueOf(blockNumber));
}

public Condition minimumHeightProgression(
final PantheonNode node, final int blocksAheadOfLatest) {
return new ExpectBlockNumberAbove(eth, futureHeight(node, blocksAheadOfLatest));
}

public Condition minimumHeightProgression(
final PantheonNode node, final int blocksAheadOfLatest, final int timeout) {
return new ExpectBlockNumberAbove(eth, futureHeight(node, blocksAheadOfLatest), timeout);
}

private BigInteger futureHeight(final Node node, final int blocksAheadOfLatest) {
return currentHeight(node).add(BigInteger.valueOf(blocksAheadOfLatest));
}

private BigInteger currentHeight(final Node node) {
return node.execute(eth.blockNumber());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,31 @@
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition;
package tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique;

import static org.assertj.core.api.Assertions.assertThat;
import static tech.pegasys.pantheon.tests.acceptance.dsl.WaitUtils.waitFor;
import static tech.pegasys.pantheon.tests.acceptance.dsl.transaction.clique.CliqueTransactions.LATEST;

import tech.pegasys.pantheon.ethereum.core.Address;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.clique.CliqueTransactions;

import java.util.List;

public class WaitUntilSignersChanged implements WaitCondition {
public class AwaitSignerSetChange implements Condition {

private final CliqueTransactions clique;
private final List<Address> initialSigners;

public WaitUntilSignersChanged(
final List<Address> initialSigners, final CliqueTransactions clique) {
public AwaitSignerSetChange(final List<Address> initialSigners, final CliqueTransactions clique) {
this.initialSigners = initialSigners;
this.clique = clique;
}

@Override
public void waitUntil(final Node node) {
public void verify(final Node node) {
waitFor(
60,
() ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,31 @@
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition;
package tech.pegasys.pantheon.tests.acceptance.dsl.condition.ibft2;

import static org.assertj.core.api.Assertions.assertThat;
import static tech.pegasys.pantheon.tests.acceptance.dsl.WaitUtils.waitFor;
import static tech.pegasys.pantheon.tests.acceptance.dsl.transaction.clique.CliqueTransactions.LATEST;

import tech.pegasys.pantheon.ethereum.core.Address;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.ibft2.Ibft2Transactions;

import java.util.List;

public class WaitUntilValidatorsChanged implements WaitCondition {
public class AwaitValidatorSetChange implements Condition {

private final Ibft2Transactions ibft;
private final List<Address> initialSigners;

public WaitUntilValidatorsChanged(
final List<Address> initialSigners, final Ibft2Transactions ibft) {
public AwaitValidatorSetChange(final List<Address> initialSigners, final Ibft2Transactions ibft) {
this.initialSigners = initialSigners;
this.ibft = ibft;
}

@Override
public void waitUntil(final Node node) {
public void verify(final Node node) {
waitFor(
60,
() ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@

import static java.util.Collections.emptyList;
import static tech.pegasys.pantheon.ethereum.core.Hash.fromHexString;
import static tech.pegasys.pantheon.tests.acceptance.dsl.transaction.clique.CliqueTransactions.LATEST;

import tech.pegasys.pantheon.config.CliqueConfigOptions;
import tech.pegasys.pantheon.config.GenesisConfigFile;
import tech.pegasys.pantheon.ethereum.core.Address;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.blockchain.ExpectBlockNotCreated;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique.AwaitSignerSetChange;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique.ExpectNonceVote;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique.ExpectNonceVote.CLIQUE_NONCE_VOTE;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.clique.ExpectProposals;
Expand Down Expand Up @@ -50,7 +52,7 @@ public Clique(final EthTransactions eth, final CliqueTransactions clique) {
this.clique = clique;
}

public ExpectValidators validatorsEqual(final PantheonNode... validators) {
public Condition validatorsEqual(final PantheonNode... validators) {
return new ExpectValidators(clique, validatorAddresses(validators));
}

Expand Down Expand Up @@ -86,6 +88,10 @@ public Condition noNewBlockCreated(final PantheonNode node) {
return new ExpectBlockNotCreated(eth, blockPeriodWait, blockPeriodWait);
}

public Condition awaitSignerSetChange(final Node node) {
return new AwaitSignerSetChange(node.execute(clique.createGetSigners(LATEST)), clique);
}

private int cliqueBlockPeriod(final PantheonNode node) {
final String config = node.getGenesisConfigProvider().create(emptyList()).get();
final GenesisConfigFile genesisConfigFile = GenesisConfigFile.fromConfig(config);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
*/
package tech.pegasys.pantheon.tests.acceptance.dsl.jsonrpc;

import static tech.pegasys.pantheon.tests.acceptance.dsl.transaction.clique.CliqueTransactions.LATEST;

import tech.pegasys.pantheon.ethereum.core.Address;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.ibft2.AwaitValidatorSetChange;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.ibft2.ExpectProposals;
import tech.pegasys.pantheon.tests.acceptance.dsl.condition.ibft2.ExpectValidators;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.ibft2.Ibft2Transactions;

Expand Down Expand Up @@ -53,6 +57,10 @@ private Address[] validatorAddresses(final PantheonNode[] validators) {
return Arrays.stream(validators).map(PantheonNode::getAddress).sorted().toArray(Address[]::new);
}

public Condition awaitValidatorSetChange(final Node node) {
return new AwaitValidatorSetChange(node.execute(ibftTwo.createGetValidators(LATEST)), ibftTwo);
}

public Condition noProposals() {
return new ExpectProposals(ibftTwo, ImmutableMap.of());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@

import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.Transaction;
import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitCondition;

public interface Node {

<T> T execute(Transaction<T> transaction);

void verify(final Condition expected);

void waitUntil(final WaitCondition condition);
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.login.LoginRequestFactory;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.net.CustomNetJsonRpcRequestFactory;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.perm.PermissioningJsonRpcRequestFactory;
import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitCondition;

import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -569,9 +568,4 @@ public <T> T execute(final Transaction<T> transaction) {
public void verify(final Condition expected) {
expected.verify(this);
}

@Override
public void waitUntil(final WaitCondition expected) {
expected.waitUntil(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNodeRunner;
import tech.pegasys.pantheon.tests.acceptance.dsl.node.RunnableNode;
import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitCondition;

import java.net.URI;
import java.util.Arrays;
Expand Down Expand Up @@ -168,10 +167,4 @@ public void verifyOnActiveNodes(final Condition condition) {
.filter(node -> pantheonNodeRunner.isActive(node.getName()))
.forEach(condition::verify);
}

public void waitUntil(final WaitCondition condition) {
for (final Node node : nodes.values()) {
node.waitUntil(condition);
}
}
}

This file was deleted.

Loading