tests: add send coins test #3679
Conversation
bznein
force-pushed
the
basicSendFlow
branch
from
9c2f4cc to
d1102ee
Compare
bznein
force-pushed
the
basicSendFlow
branch
11 times, most recently
from
eda42fa to
d54dca5
Compare
bznein
force-pushed
the
basicSendFlow
branch
3 times, most recently
from
e8668b1 to
a82ec48
Compare
|
@thisconnect ready for review :) |
thisconnect
left a comment
thisconnect
left a comment
There was a problem hiding this comment.
very nice, added some comments, but overall LGTM
| await page.getByRole('button', { name: 'Verify address on BitBox' }).click(); | ||
| const addressLocator = page.locator('[data-testid="receive-address"]'); | ||
| recvAdd = await addressLocator.inputValue(); | ||
| console.log(`Receive address: ${recvAdd}`); |
There was a problem hiding this comment.
shouldn't there be an expect() call at the end?
maybe it could just test that the recvAdd starts with "brct1" (for regtest addresses but I am not sure).
There was a problem hiding this comment.
shouldn't there be an expect() call at the end?
I guess we could technically do it, even though the step is mostly only trying to grab the address and we are reusing it later
maybe it could just test that the recvAdd starts with "brct1" (for regtest addresses but I am not sure).
I'll do that!
| } | ||
| }}> | ||
| <div className={styles.txContent}> | ||
| <div className={styles.txContent} data-testid="transaction" tx-type={type}> |
There was a problem hiding this comment.
Although it technically works without, I think we should keep using data-* prefix for custom attributes unless there is good reason not to.
| <div className={styles.txContent} data-testid="transaction" tx-type={type}> | |
| <div className={styles.txContent} data-testid="transaction" data-tx-type={type}> |
| await page.getByRole('link', { name: 'Settings' }).click(); | ||
| await page.getByRole('link', { name: 'Manage Accounts' }).click(); | ||
| await page.getByRole('button', { name: 'Add account' }).click(); | ||
| await page.getByRole('button', { name: 'Add account' }).click(); |
There was a problem hiding this comment.
On a multi device you would have to choose the coin type first. So I guess this is a btc-only simulated device?
Just thinking out loud. Is the edition btconly/multi something we should be able to configure for each test in the future?
There was a problem hiding this comment.
This test is performed in regtest, which only supports BTC.
To test a multi behaviour, we'd need to use testnet with either a software keystore or a simulator, which are both "multi"
There was a problem hiding this comment.
I didn't mean to suggest to test altcoins here, but just wanted to note that if the device was a multi it would have 1 extra step here.
| await page.getByRole('link', { name: 'Manage Accounts' }).click(); | ||
| await page.getByRole('button', { name: 'Add account' }).click(); | ||
| await page.getByRole('button', { name: 'Add account' }).click(); | ||
| await page.getByRole('button', { name: 'Done' }).click(); |
There was a problem hiding this comment.
At the end of adding a 2nd account there should be a text: "Account added" or "<accountname> has now been added to your accounts."
Maybe something you could test with expect here?
frontends/web/tests/send.test.ts
Outdated
| await test.step('Verify there are no transactions yet', async () => { | ||
| await page.goto('/#/account-summary'); | ||
| mineBlocks(12); | ||
| await page.locator('td[data-label="Account name"]').nth(0).click(); |
There was a problem hiding this comment.
Currently this and other selectors target td element but in the future we might refactor or redesign.
With that in mind it would be better to drop the element td from the selector? wdyt?
| await test.step('Send RBTC to second account receive address', async () => { | ||
| await page.goto('/#/account-summary'); | ||
| await page.getByRole('link', { name: 'Bitcoin Regtest Bitcoin' }).click(); | ||
| console.log('Sending RBTC to second account'); |
There was a problem hiding this comment.
in the run-tests log I can only see, the following
Receive address: bcrt1qn00wa7j8v2ssq7y7774qua7xly02she0ukcygc
Receive address: bcrt1qh8r2rx5myc0pjhkjyx70hug2s4dpj6fufej4jg
Sending RBTC to second account
https://github.com/BitBoxSwiss/bitbox-wallet-app/actions/runs/19498423962/job/55806495741
maybe also add a console log 'Sending RBTC to first account' when sending the first time? Then the log info would be a bit more usefuls.
thisconnect
left a comment
thisconnect
left a comment
There was a problem hiding this comment.
LGTM with some suggestion and comment/question
bznein
force-pushed
the
basicSendFlow
branch
from
a82ec48 to
a73d990
Compare
bznein
force-pushed
the
basicSendFlow
branch
from
a73d990 to
3b3e027
Compare
2 participants
Note: based on #3666
This is a test that:
Before asking for reviews, here is a check list of the most common things you might need to consider: