-
Notifications
You must be signed in to change notification settings - Fork 130
Conversation
…p-acceptance-test
@@ -47,6 +49,7 @@ protected AcceptanceTestBase() { | |||
transactions = new Transactions(accounts); | |||
web3 = new Web3(new Web3Transactions()); | |||
pantheon = new PantheonNodeFactory(); | |||
contract = new ContractVerifier(accounts.getPrimaryBenefactor()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: naming this variable as contract
can be misleading. Shouldn't we name it contractVerifier
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, you know that was it's name originally, I'll change it back
*/ | ||
package tech.pegasys.pantheon.tests.acceptance.dsl.condition.eth; | ||
|
||
public class ExpectContractTransactionReciept {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TYPO: ExpectContractTransactionRecIEpt
-> ExpectContractTransactionRecEIpt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well spotted!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Yesterday I discovered that Web3j is calling EthGetTransactionReceipt as part of it's contract deployment code.
Meaning the acceptance test that validates the correctness of the EthGetTransactionReceipt was not as good or 'correct' test as it should be.
The focus of this PR was to change the AT to centre on the state of the transaction receipt and remove the valueless duplicate call (by removing the one in the AT).