test: re-enable secure send e2e test #4507
Conversation
| @@ -1,31 +0,0 @@ | |||
| #!/usr/bin/env sh | |||
There was a problem hiding this comment.
we have a fund_e2e_accounts.ts file, and i think this one is not used anymore (couldn't find references to it)
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4507 +/- ##
=======================================
Coverage 85.27% 85.27%
=======================================
Files 715 715
Lines 26663 26663
Branches 3610 3610
=======================================
Hits 22738 22738
Misses 3865 3865
Partials 60 60 Continue to review full report in Codecov by Sentry.
|
e2e/scripts/fund-e2e-accounts.ts
Outdated
| const [balanceE2ETestWallet, balanceSecureSendWallet] = await Promise.all([ | ||
| getBalance(E2E_TEST_WALLET), | ||
| getBalance(E2E_TEST_WALLET_SECURE_SEND), | ||
| ]) | ||
| const receivingBalance = { | ||
| ...balanceE2ETestWallet, | ||
| ...balanceSecureSendWallet, | ||
| } |
There was a problem hiding this comment.
Isn't this gonna mix/overwrite balances between both accounts?
Where do we actually send to E2E_TEST_WALLET_SECURE_SEND?
There was a problem hiding this comment.
lol oops, this logic didn't make sense. i've updated the script, sorry about that
e2e/src/utils/utils.js
Outdated
| await waitForElementId('PhoneVerificationSkipHeader') | ||
| // Skip | ||
| await element(by.id('PhoneVerificationSkipHeader')).tap() | ||
| } catch {} |
There was a problem hiding this comment.
Would be nice to console.log something here, maybe Ignored error while waiting for phone verification screen, account likely already verified or something.
| await element(by.id('ValidateRecipientAccount/TextInput')).replaceText( | ||
| SAMPLE_WALLET_ADDRESS_VERIFIED_2 | ||
| ) |
There was a problem hiding this comment.
Why doesn't it need just the last 4 digits?
Oh, it's doing this lookup from the verified account itself.
Since we can't lookup numbers without being verified.
We'd need 2 other verified accounts, to another number to test that case.
There was a problem hiding this comment.
if i understand correctly, the secure send screen either asks for the full address or the last 4 digits but this is depending on how "different" the possible addresses are. unfortunately we've mapped to 2 wallets and they both have a "0" in the last 4 digits of the address, so we've got the full text input.
idk if it is too important to test both the full text input and the 4 digits, i have 90% confidence that both flows are working if i know one of them is working. we can try to remap to another address, but this is Tom's burner number and he's out all week so would need to wait till he's back
There was a problem hiding this comment.
Yes the current test is fine for now.
For posterity, the check for uniqueness of the last 4 digits is done as a whole, but also includes the user's own address:
wallet/src/identity/secureSend.ts
Lines 59 to 62 in 2b28b69
It's not the fact that there are chars in common between them that causes PARTIAL vs FULL.
1 build had no size change
Celo (test) 1.72.0 (137)
|
| switch (balance) { | ||
| // Ensure that the faucet has enough balance for each refill tokens | ||
| for (const [tokenSymbol, tokenBalance] of Object.entries(faucetTokenBalances)) { | ||
| if (!REFILL_TOKENS.includes(tokenSymbol)) { |
There was a problem hiding this comment.
not strictly necessary, but without this we'd proceed for tokens like cREAL and TT only to hit the default case in the switch statement (no transaction created)
| ) { | ||
| const balance = (await getBalance(address)) ?? {} | ||
| for (const [tokenSymbol, tokenBalance] of Object.entries(balance)) { | ||
| if (tokenSymbols.includes(tokenSymbol) && tokenBalance < minBalance) { |
There was a problem hiding this comment.
use tokenSymbols to prevent throwing an error for tokens that we don't care about
Description
This PR:
Test plan
n/a
Related issues
Backwards compatibility
Y