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

Acceptance test - refactor JsonRpc to use conditionals #204

Merged
merged 15 commits into from
Oct 31, 2018
Merged

Acceptance test - refactor JsonRpc to use conditionals #204

merged 15 commits into from
Oct 31, 2018

Conversation

CjHare
Copy link
Contributor

@CjHare CjHare commented Oct 29, 2018

PR description

The main goal of this PR was re-writing the JSON-RPC acceptance tests to use the conditionals and through restricting access reduce the opportunity to create non-conditional style AT.

Fixed Issue(s)

@CjHare CjHare added acceptance test work in progress Work on this pull request is ongoing labels Oct 29, 2018
@CjHare CjHare removed the work in progress Work on this pull request is ongoing label Oct 30, 2018
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM.

}

@Test
public void shouldSucceedConnectingToNodeWithWsRpcEnabled() {
rpcEnabledNode.verifyWsRpcEnabled();
rpcEnabledNode.jsonRpcByWebSockets();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe useWebSocketsForJsonRpc?

public void verify(final Node node) {
final Throwable thrown = catchThrowable(() -> node.execute(transaction));
assertThat(thrown).isInstanceOf(ClientConnectionException.class);
assertThat(thrown.getMessage()).contains(expectedMessage);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something for the future, but this feels like we could have a ExpectExceptionCondition that delegates to another condition (or maybe just Runnable) and then checks the exception thrown.

import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.eth.EthGetWorkTransaction;

public class ExpectEthGetWorkHasThreeNotNullValues implements Condition {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would this be clearer if it were named SanityCheckEthGetWorkValues? Just to be explicit that the requirement isn't really just that there's three arbitrary, non-null values but that's the most we can actually verify about this data.

@CjHare CjHare merged commit 697171d into PegaSysEng:master Oct 31, 2018
@CjHare CjHare deleted the acceptance-test-refactor-jsonrpc branch October 31, 2018 01:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants