fix: fixing e2e test for authorizations#2176
Conversation
WalkthroughThe PR refactors UI test wallet setup infrastructure by removing BrowserContext dependency from wallet helpers, introducing environment-based second wallet import capability via new Changes
Sequence Diagram(s)sequenceDiagram
participant Test as authorize-spending.spec.ts
participant Setup as wallet-setup.ts
participant Leap as Leap Extension
Test->>Setup: setupWallet(extPage)
Setup->>Setup: importWalletToLeap(page)
Setup->>Leap: Import primary wallet
Leap-->>Setup: wallet imported
Setup->>Test: ✓ ready
Test->>Setup: importSecondWallet(extPage, mnemonic)
Setup->>Setup: importSecondWalletToLeap(page, mnemonic)
Setup->>Leap: Validate & import secondary wallet
Leap-->>Setup: wallet imported
Setup->>Setup: Reload page & top up
Setup->>Test: ✓ second wallet ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2176 +/- ##
==========================================
- Coverage 47.61% 47.28% -0.34%
==========================================
Files 1036 1026 -10
Lines 29356 29008 -348
Branches 7589 7549 -40
==========================================
- Hits 13978 13716 -262
+ Misses 14979 14893 -86
Partials 399 399
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
171-206: Consider adding balance confirmation wait after faucet request.The
topUpWalletfunction makes a faucet request but doesn't wait for the transaction to be confirmed on-chain. This could cause the "Insufficient funds" error mentioned in the PR description when tests attempt transactions before the funds arrive.Add a polling mechanism to wait for balance update:
if (response.status >= 300) { console.error(`Unexpected faucet response status: ${response.status}`); console.error("Faucet response:", await response.text()); + return; } + + // Wait for balance to update (poll for up to 60 seconds) + for (let i = 0; i < 30; i++) { + await wait(2000); + const newBalance = await getBalance(accounts[0].address); + if (newBalance > balance) { + console.log(`Wallet topped up successfully. New balance: ${newBalance} uakt`); + return; + } + } + console.warn("Faucet request succeeded but balance did not update within 60 seconds"); } catch (error) {You'll need to import
waitat the top if not already imported:import { setTimeout as wait } from "timers/promises";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/deploy-web/tests/ui/authorize-spending.spec.ts(2 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(1 hunks)apps/deploy-web/tests/ui/fixture/test-env.config.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/tests/ui/fixture/context-with-extension.tsapps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/authorize-spending.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/tests/ui/fixture/context-with-extension.tsapps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/authorize-spending.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/tests/ui/authorize-spending.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/deploy-web/tests/ui/fixture/context-with-extension.tsapps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/authorize-spending.spec.ts
🧬 Code graph analysis (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
setupWallet(29-34)
apps/deploy-web/tests/ui/authorize-spending.spec.ts (3)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (2)
getExtensionPage(15-27)importSecondWallet(36-40)apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
clickCopyAddressButton(31-39)apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(11-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (7)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
56-56: LGTM! Signature update aligns with refactored wallet setup.The updated call correctly matches the new
setupWallet(page: Page)signature in wallet-setup.ts.apps/deploy-web/tests/ui/fixture/wallet-setup.ts (4)
23-23: LGTM! Increased timeout improves reliability.The timeout increase from 5,000ms to 10,000ms for waiting for the page event provides better resilience for slower test environments.
29-34: LGTM! Simplified signature removes unnecessary parameter.The refactored
setupWalletsignature correctly removes theBrowserContextparameter that was not being used, making the API cleaner.
36-40: New second wallet import function looks good, but note timing consideration.The
importSecondWalletfunction properly encapsulates the second wallet import flow. However, be aware thattopUpWalletdoesn't wait for on-chain confirmation of the faucet transaction, which may contribute to the "Insufficient funds" error mentioned in the PR description.See the comment on authorize-spending.spec.ts lines 67-69 for details on the timing issue.
141-169: New helper function is well-implemented with proper validation.The
importSecondWalletToLeapfunction correctly:
- Validates the mnemonic is set and has 12 words
- Navigates the wallet import UI flow
- Returns a wallet instance for further operations
Note that after this function executes, the Leap extension will have the second wallet as the active wallet. This is intentional for the authorization test flow where the second wallet grants authorization to the first wallet.
apps/deploy-web/tests/ui/authorize-spending.spec.ts (1)
67-69: Potential timing issue: faucet top-up may not complete before authorization.The flow imports a second wallet and calls
topUpWallet, but thetopUpWalletfunction (in wallet-setup.ts) only makes the faucet HTTP request without waiting for on-chain confirmation. The authorization transaction on line 74-75 may execute before the funds arrive, causing the "Insufficient funds" error mentioned in the PR description.Consider adding a wait/retry mechanism after the faucet request in
topUpWalletto ensure funds are available:// In wallet-setup.ts topUpWallet function, after the faucet request: // Wait for balance to update (with timeout) for (let i = 0; i < 30; i++) { await wait(2000); // Wait 2 seconds const newBalance = await getBalance(accounts[0].address); if (newBalance > balance) { return; // Funds arrived } } console.warn("Faucet funds may not have arrived yet");Alternatively, run a script to verify the current faucet behavior:
#!/bin/bash # Check faucet endpoint and recent transaction times curl -s "$(node -e "console.log(require('@akashnetwork/net').netConfig.getFaucetUrl('sandbox'))")" || echo "Faucet URL not accessible"⛔ Skipped due to learnings
Learnt from: jzsfkzm Repo: akash-network/console PR: 2039 File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10 Timestamp: 2025-10-15T16:39:55.348Z Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.Learnt from: stalniy Repo: akash-network/console PR: 2138 File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382 Timestamp: 2025-10-31T11:26:42.138Z Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe: ```typescript private async getAddress() { if (!this.address) { this.address = await this.wallet.getFirstAddress(); } return this.address; } ``` Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
6-6: MakeAUTHORIZING_WALLET_MNEMONICoptional since only one test uses it.The field is only used in
authorize-spending.spec.ts(line 69), but defining it as required (z.string()) will fail validation for all test runs that don't set this environment variable. Tests that don't perform authorization won't have this variable available.Change to one of:
z.string().optional()- allow tests without authorization to runz.string().default("...")- if a safe default test mnemonic exists
stalniy
force-pushed
the
bugfixes/authorization-e2e
branch
from
0f7ff47 to
b1e39b1
Compare
stalniy
force-pushed
the
bugfixes/authorization-e2e
branch
from
b1e39b1 to
c5b6d73
Compare
2 participants
@stalniy I changed how authorization e2e works, now it would import a second wallet by mnemonic, and this wallet would be authorizing the test wallet to spend.
Second wallet is represented by AUTHORIZING_WALLET_MNEMONIC in the env vars, I used this mnemonic for testing purposes:
useful custom fit trophy hub proud quit grape mutual curtain ceiling foilHowever, when I switch to sandbox, authorizing starts not working (thus the whole e2e test is breaking), with
Insufficent fundserror, despite the fact the wallet is topped up before use. Any ideas why this would happen?Summary by CodeRabbit