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

ABI Interaction integration tests with app calls #148

Merged
merged 15 commits into from
Nov 15, 2021

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented Nov 5, 2021

This PR introduces some Cucumber integration tests for interacting with ARC-0004 compliant apps using the AtomicTransactionComposer in the SDKs.

PyTeal contract: gist

This change is composed of:

  1. Payment transaction unit tests
  2. Composer unit tests that build and sign the transaction, and compares the signed transactions to a golden
  3. Composer integration tests that build, clone, sign, and executes the transactions and compares the logged results to the expected values.

Closes #146

Todo:

  • Standardize how return logs are handled (Hash the word return)

@algochoi algochoi marked this pull request as ready for review November 8, 2021 16:37
@algochoi algochoi self-assigned this Nov 8, 2021
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

This appears to test AtomicTransactionComposer in a more indirect way than what I was expecting. I was imagining we would have new steps for each action of the composer and mix these in with some of the existing transaction construction steps. Something like:

Given a new atomic transaction composer
And suggested transaction parameters fee <fee>, flat-fee "<flat-fee>", first-valid <first-valid>, last-valid <last-valid>, genesis-hash "<genesis-hash>", genesis-id "<genesis-id>"
When I build a transaction ...
And I make a transaction signer for account mnemonic "<mnemonic>"
And I add the transaction and signer to the composer
And I gather signatures with the composer
Then the signatures should equal "<expected>"

Though admittedly we only have general txn construction steps for app and keyreg txns at the moment. If needed I can help flesh these out for the other txn types.

I may have lead you astray by suggesting we reuse steps as much as possible. In general we should do that, but it doesn't appear to apply to this test very much.

@algochoi
Copy link
Contributor Author

algochoi commented Nov 9, 2021

I only added a PaymentTxn test case for now since the current tests only require payments -- I can create a ticket for creating the remaining transaction types in the cucumber tests.

Copy link
Contributor Author

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Split up the tests so the unit tests check for the signed transaction goldens and the integration tests check for the app return values.

One tricky thing is how we build up transaction and app arguments for the method calls, and I've just chosen to hard-code it on the Cucumber side rather than the SDK side. It would be nicer if the transactions can be "parameterized" better, but I'm not sure if there is a better way right now.

@algochoi algochoi requested a review from jasonpaulos November 9, 2021 21:24
@algochoi
Copy link
Contributor Author

I updated the tests and tested them against the Python SDK. This commit contains the relevant diff for implementing the test steps in the Python SDK: algorand/py-algorand-sdk@e96b740

@algochoi algochoi requested a review from jasonpaulos November 15, 2021 21:26
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looks good!

@algochoi algochoi merged commit f7b3cec into master Nov 15, 2021
@algochoi algochoi deleted the algochoi/abi-integration-tests branch November 15, 2021 22:46
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.

Add tests for ABI interaction - AtomicTransactionComposer
2 participants