-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Tests] Added script functions to argument types test suite #157
Conversation
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.
overall lgtm, makes me think we need to improve our testing framework and consolidate all helpers/testing objects/etc.
78b29d4
to
cbe6643
Compare
tests/e2e/transaction/helper.ts
Outdated
const transaction = await aptos.generateTransaction({ | ||
sender: firstAccount.accountAddress.toString(), | ||
data: { | ||
function: `0x${1}::aptos_account::batch_transfer`, |
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.
Lol why like this? If this is doing a pattern match that makes you do something like this, we need to fix it and remove the pattern match
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.
Lol I have no idea why I did this- it's fine with just the string, I updated it
function: `0x${1}::aptos_account::batch_transfer`, | ||
functionArguments: [ | ||
new MoveVector(addressesRemaining), | ||
MoveVector.U64(addressesRemaining.map(() => amountToSend)), |
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.
Why map if it's exactly one?
await aptos.fundAccount({ accountAddress: firstAccount.accountAddress.toString(), amount: FUND_AMOUNT }); | ||
// Fund again because these txns can be expensive | ||
await aptos.fundAccount({ accountAddress: firstAccount.accountAddress.toString(), amount: FUND_AMOUNT }); | ||
const addressesRemaining = accounts.slice(1).map((account) => account.accountAddress); |
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.
Why not just accounts[1]
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.
I'll put a comment explaining it so it's easier to read, but it's sending the coins from account[0]
out to accounts[1..n]
not just accounts[1]
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.
if end
in Array.slice(start?: number, end?: number)
is undefined then it returns the slice from start to the end
…pt args instead of various signers
cbe6643
to
a8f05be
Compare
Description
Updated the test suite to use scripts
The sender/authenticator type is already validated in transaction submission, so in order to keep this test focused, I just added scripts instead of adding all the different signer types.
Test suite
TODO:
Argument types
Number of signers
TODO:
NOTE: Also added a small fix to the script function vector
U8
serialize function. This is only needed forU8
because script functions don't accept any other vector type.Test Plan