-
Notifications
You must be signed in to change notification settings - Fork 44
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
Use local network with tests #26
Conversation
This is so great, huge props again to @banool !! |
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!
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.
My comments are all about making sure this code is maintainable. Need to follow up with it.
tests/e2e/api/account.test.ts
Outdated
const accountCoinData = await aptos.getAccountCoinsData({ | ||
accountAddress: senderAccount.accountAddress.toString(), | ||
}); | ||
expect(accountCoinData[0].amount).toBe(100000000); |
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.
Should move to a constant, maybe even a default fund amount, at least for testing
// to help with indexer latency | ||
await sleep(1000); |
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.
:(
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.
Summarizing what I said in Slack, it'd be better to look at the txn version from the committed txn output and then wait for the indexer to catch up to that version by querying the processor status. That'd have the least latency and be fully reliable.
const config = new AptosConfig({ network: Network.LOCAL }); | ||
const aptos = new Aptos(config); | ||
const alice = Account.generate({ scheme: SigningScheme.Ed25519 }); | ||
await aptos.fundAccount({ accountAddress: alice.accountAddress.toString(), amount: 1000000000 }); |
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.
We should probably at a minimum put this duplicated code in a function
f9712f0
to
6cc9243
Compare
6cc9243
to
6ecd477
Compare
Description
Change tall tests to use local network.
@banool do we need docker in CI?
There is a latency chance when querying indexer right after we submit a transaction, therefore adding
sleep(100)
- can think about a different approach for our test framework we want to have or in a following PR.Test Plan
Related Links