test: can change wallet or disconnect#2039
Conversation
WalkthroughThis pull request introduces E2E test infrastructure for wallet operations, adding a new test for switching wallets between extension and console, updating wallet disconnection tests, and extracting shared wallet creation and page handling logic into reusable helper utilities and page objects. Changes
Sequence DiagramsequenceDiagram
participant Test
participant LeapExt
participant ExtPage as Extension Page
participant PopupPage as Popup Window
participant Console
Test->>LeapExt: createWallet(extensionId)
activate LeapExt
LeapExt->>ExtPage: getExtensionPage(context, extensionId)
activate ExtPage
ExtPage-->>LeapExt: extension page
deactivate ExtPage
LeapExt->>ExtPage: clickWalletSelectorDropdown()
LeapExt->>ExtPage: clickCreateNewWalletButton()
LeapExt->>ExtPage: fillWalletName("random-name")
LeapExt->>ExtPage: clickCreateWalletButton()
LeapExt->>PopupPage: waitForPopup()
activate PopupPage
PopupPage-->>LeapExt: popup window
LeapExt->>PopupPage: clickConnectWalletButton()
deactivate PopupPage
PopupPage->>Console: Wallet connected
deactivate LeapExt
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple test infrastructure files with mixed complexity: new helper functions and page object encapsulation require logic verification, while test file updates are straightforward. The breadth of changes across fixtures, helpers, and test files necessitates cross-file coherence checking, though individual pieces follow consistent patterns. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/3085.js(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/5509.js(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(2 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/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/change-wallets.spec.tsapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/pages/FrontPage.tsxpackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/fixture/context-with-extension.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/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/change-wallets.spec.tsapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/pages/FrontPage.tsxapps/deploy-web/tests/ui/fixture/Leap/3085.jspackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/fixture/Leap/5509.jsapps/deploy-web/tests/ui/fixture/context-with-extension.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/change-wallets.spec.tsapps/deploy-web/tests/ui/disconnect-wallet.spec.ts
🧬 Code graph analysis (6)
apps/deploy-web/tests/ui/change-wallets.spec.ts (3)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (2)
test(12-68)expect(70-70)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(7-29)apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
createWallet(35-56)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (2)
test(12-68)expect(70-70)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(7-29)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
apps/deploy-web/src/services/auth/auth/auth.service.ts (1)
logout(31-35)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (2)
apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(12-16)apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
expect(70-70)
apps/deploy-web/tests/ui/fixture/Leap/3085.js (1)
apps/deploy-web/tests/ui/fixture/Leap/5509.js (4)
y(1-1)W(1-1)f(1-1)ee(1-1)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
getExtensionPage(14-26)
🪛 Biome (2.1.2)
apps/deploy-web/tests/ui/pages/FrontPage.tsx
[error] 11-11: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
apps/deploy-web/tests/ui/fixture/Leap/3085.js
[error] 13-13: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ 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). (3)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: Validate local packages
🔇 Additional comments (5)
packages/ui/components/custom/popup.tsx (1)
252-252: LGTM! Accessibility enhancement for custom action buttons.The addition of
aria-labelattributes to left and right action buttons improves screen reader accessibility by ensuring button labels are consistently announced.Also applies to: 261-261
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
75-75: LGTM! Accessibility enhancement for disconnect button.Adding
aria-label="Disconnect Wallet"ensures screen readers properly announce the button's purpose, consistent with similar improvements throughout the PR.apps/deploy-web/tests/ui/fixture/Leap/5509.js (1)
1-1: Minified bundle - skip detailed review.This is a compiled webpack bundle for the Leap extension test fixture. Detailed review of minified code is not actionable.
apps/provider-console/src/components/layout/WalletStatus.tsx (1)
42-42: LGTM! Accessibility enhancement for disconnect menu item.Adding
aria-label="Disconnect Wallet"to the dropdown menu item improves screen reader accessibility, consistent with similar enhancements across wallet-related UI components in this PR.apps/deploy-web/tests/ui/fixture/Leap/3085.js (1)
1-10: Minified bundle - skip detailed review.This is a compiled webpack bundle for the Leap extension test fixture. Detailed review of minified code is not actionable. The static analysis error on line 13 is a false positive due to minification.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
c131e95 to
2f52f13
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
7-7: Remove or use the unusedFrontPageinstance.Instantiating
FrontPagewithout saving or calling it does nothing—drop the line or assign it to a variable you actually use.- new FrontPage(context, page); + const frontPage = new FrontPage(context, page); + // e.g. await frontPage.goto();
15-15: Await Playwright assertions.
expect(...).toBeVisible()returns a Promise; withoutawait, the check can be skipped and failures never surface.- expect(page.getByTestId("connect-wallet-btn")).toBeVisible(); + await expect(page.getByTestId("connect-wallet-btn")).toBeVisible();apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
10-13: Fix self-referential constructor default.
contextisn’t defined in this scope, so the default initializer throws. Require callers to pass the fixture’s context explicitly.- constructor( - readonly context: Context = context, - readonly page: Page - ) {} + constructor( + readonly context: Context, + readonly page: Page + ) {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/3085.js(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/5509.js(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/deploy-web/tests/ui/fixture/wallet-setup.ts
- apps/provider-console/src/components/layout/WalletStatus.tsx
- apps/deploy-web/tests/ui/change-wallets.spec.ts
- apps/deploy-web/tests/ui/fixture/context-with-extension.ts
- apps/deploy-web/tests/ui/fixture/Leap/5509.js
- apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx
🧰 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:
packages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/tests/ui/pages/FrontPage.tsx
**/*.{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:
packages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/tests/ui/fixture/Leap/3085.jsapps/deploy-web/tests/ui/pages/FrontPage.tsx
**/*.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/disconnect-wallet.spec.ts
🧬 Code graph analysis (3)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (2)
test(12-68)expect(70-70)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(7-29)
apps/deploy-web/tests/ui/fixture/Leap/3085.js (1)
apps/deploy-web/tests/ui/fixture/Leap/7370.js (1)
q(1-1)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (2)
apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(12-16)apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
expect(70-70)
🪛 Biome (2.1.2)
apps/deploy-web/tests/ui/fixture/Leap/3085.js
[error] 13-13: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
apps/deploy-web/tests/ui/pages/FrontPage.tsx
[error] 11-11: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (1)
apps/deploy-web/tests/ui/fixture/Leap/3085.js (1)
1-505: Test fixture with bundled webpack code—no functional review needed.This file contains bundled webpack output used as a test fixture for E2E testing of wallet interactions. Since it mirrors existing Leap Cosmos UI logic with formatting-only changes and is generated/compiled code, a detailed code review is not applicable.
Static analysis false positive:
The Biome error flagging line 13 ("variable
yused before declaration") is a false positive. The pattern on line 7:var y = (h => (..., h))(y || {})is valid JavaScript. Due to
varhoisting,yis declared (butundefined) at the top of the scope, soy || {}correctly evaluates to{}. This is a common pattern for compiled TypeScript enums. Please suppress or ignore this Biome warning.Bundled artifacts as test fixtures:
While committing bundled code is generally discouraged, test fixtures are a valid exception when reproducibility is required. However, verify whether your test setup could instead:
- Generate fixtures dynamically during test setup, or
- Source fixtures from the actual built extension bundle
This would reduce repository bloat and ensure fixtures stay synchronized with the source code.
Would you like me to help identify where these fixtures are consumed in the test suite to assess the best approach?
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
2f52f13 to
78446fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
35-56: Critical: Global selector attribute never restored.Setting
selectors.setTestIdAttribute("data-testing-id")on line 38 permanently breaks all subsequent tests that expect the default"data-testid". Additionally, the page handler registered on line 53 may leak if the popup doesn't appear.Wrap the wallet creation in try/finally and ensure cleanup:
export async function createWallet(context: BrowserContext, extensionId: string, walletName: string) { const extPage = await getExtensionPage(context, extensionId); - selectors.setTestIdAttribute("data-testing-id"); - await extPage.locator('button[aria-label="Select Wallet"]').click(); - - const createNewWalletButton = extPage.getByText(/create new wallet/i); - await createNewWalletButton.waitFor({ state: "visible", timeout: 10_000 }); - await createNewWalletButton.click(); - - await extPage.getByTestId("input-enter-wallet-name").fill(walletName); - const pageHandler = async (page: Page) => { - console.log("pageHandler running"); await page.waitForLoadState("domcontentloaded"); await page.getByTestId("btn-connect-wallet").click(); context.off("page", pageHandler); }; context.on("page", pageHandler); - - await extPage.getByTestId("btn-create-wallet").click(); + + try { + selectors.setTestIdAttribute("data-testing-id"); + await extPage.locator('button[aria-label="Select Wallet"]').click(); + + const createNewWalletButton = extPage.getByText(/create new wallet/i); + await createNewWalletButton.waitFor({ state: "visible", timeout: 10_000 }); + await createNewWalletButton.click(); + + await extPage.getByTestId("input-enter-wallet-name").fill(walletName); + await extPage.getByTestId("btn-create-wallet").click(); + } finally { + context.off("page", pageHandler); + selectors.setTestIdAttribute("data-testid"); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/3085.js(1 hunks)apps/deploy-web/tests/ui/fixture/Leap/5509.js(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/deploy-web/tests/ui/fixture/Leap/5509.js
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/provider-console/src/components/layout/WalletStatus.tsx
- apps/deploy-web/tests/ui/fixture/context-with-extension.ts
- apps/deploy-web/tests/ui/pages/FrontPage.tsx
- apps/deploy-web/tests/ui/disconnect-wallet.spec.ts
- packages/ui/components/custom/popup.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/change-wallets.spec.ts
**/*.{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/change-wallets.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/wallet-setup.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/change-wallets.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/fixture/Leap/3085.js
🧬 Code graph analysis (3)
apps/deploy-web/tests/ui/change-wallets.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
test(12-68)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(9-37)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
apps/deploy-web/src/services/auth/auth/auth.service.ts (1)
logout(31-35)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
createWallet(21-27)
🪛 Biome (2.1.2)
apps/deploy-web/tests/ui/fixture/Leap/3085.js
[error] 13-13: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
75-78: LGTM! Good accessibility enhancement.The
aria-labelattribute improves accessibility for screen reader users, making the disconnect action clearer.apps/deploy-web/tests/ui/fixture/Leap/3085.js (1)
1-505: Test fixture bundle—no review needed.This webpack bundle is test fixture data for the Leap extension. The static analysis warning about variable usage on line 13 is a false positive—the self-referencing IIFE pattern
(y || {})is safe in bundled code.apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
14-26: LGTM! Well-structured helper.The
getExtensionPagefunction correctly handles both existing and new extension pages, with appropriate timeout handling.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
78446fd to
df4fb78
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/ui/components/custom/popup.tsx (1)
252-255: Duplicate: Avoid stringifying non-string button labels for aria-label.This issue was previously flagged and remains unaddressed. When
labelis a React.ReactNode (not a string), the template literal\${label}`produces[object Object], which harms accessibility. Only setaria-label` when the label is a string.Apply the previously suggested fix:
- <Button key={`dialog-action-button-${idx}`} {...rest} aria-label={`${label}`}> + <Button + key={`dialog-action-button-${idx}`} + {...rest} + aria-label={typeof label === "string" ? label : undefined} + >Apply the same change for right-side buttons at lines 261-264.
Also applies to: 261-264
🧹 Nitpick comments (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
36-51: Consider cleanup for pageHandler if wallet creation fails.The pageHandler is registered at line 48 but only removes itself when it runs (line 46). If the wallet creation button click fails or the page event never fires, the handler remains registered indefinitely. While this may not cause issues in practice if the test fails and tears down the context, wrapping in a try/finally would be more robust.
Consider this pattern for guaranteed cleanup:
export async function createWallet(context: BrowserContext, extensionId: string, walletName: string) { const extPage = await getExtensionPage(context, extensionId); + const pageHandler = async (page: Page) => { + await page.waitForLoadState("domcontentloaded"); + await clickConnectWalletButton(page); + context.off("page", pageHandler); + }; + context.on("page", pageHandler); + + try { await clickWalletSelectorDropdown(extPage); await clickCreateNewWalletButton(extPage); await fillWalletName(extPage, walletName); - - const pageHandler = async (page: Page) => { - await page.waitForLoadState("domcontentloaded"); - await clickConnectWalletButton(page); - context.off("page", pageHandler); - }; - context.on("page", pageHandler); - await clickCreateWalletButton(extPage); + } catch (error) { + context.off("page", pageHandler); + throw error; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/tests/ui/pages/FrontPage.tsx
- apps/deploy-web/tests/ui/change-wallets.spec.ts
🧰 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/wallet-setup.tsapps/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/testing-helpers.tsapps/deploy-web/tests/ui/disconnect-wallet.spec.tspackages/ui/components/custom/popup.tsx
**/*.{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/wallet-setup.tsapps/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/fixture/testing-helpers.tsapps/deploy-web/tests/ui/disconnect-wallet.spec.tspackages/ui/components/custom/popup.tsx
**/*.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/disconnect-wallet.spec.ts
🧬 Code graph analysis (5)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
getExtensionPage(15-27)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (2)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
createWallet(21-29)apps/deploy-web/tests/ui/fixture/testing-helpers.ts (5)
clickWalletSelectorDropdown(8-10)clickCreateNewWalletButton(16-20)fillWalletName(22-25)clickConnectWalletButton(32-35)clickCreateWalletButton(27-30)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(41-45)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
apps/deploy-web/src/services/auth/auth/auth.service.ts (1)
logout(31-35)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
test(12-68)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(9-39)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (8)
apps/provider-console/src/components/layout/WalletStatus.tsx (1)
42-42: LGTM! Accessibility enhancement for disconnect button.The aria-label improves screen reader support and aligns with similar changes across the codebase.
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
75-75: LGTM! Consistent accessibility enhancement.The aria-label addition matches the pattern applied in WalletStatus.tsx and improves screen reader support.
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (1)
4-10: LGTM! Well-structured disconnect wallet test.The test correctly verifies that the wallet remains disconnected after reload, leveraging the FrontPage page object for clean test organization.
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (2)
10-11: LGTM! Improved consistency with waitForLocator helper.The refactor standardizes the visibility wait pattern used across the test suite.
13-13: Static analysis false positive: RegExp from variable is safe in test context.The static analysis tool flagged the RegExp construction from
networkId. In this test context,networkIdis a controlled test parameter (e.g., "sandbox", "testnet"), not user input, so ReDoS is not a concern. The pattern is simple and safe.apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
52-52: LGTM! Good refactor to eliminate duplication.Replacing the manual extension page lookup with the
getExtensionPagehelper improves maintainability and reduces code duplication.apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
15-27: LGTM! Clean helper for extension page access.The function correctly handles both finding an existing extension page and creating a new one when needed.
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
1-45: LGTM! Well-organized testing utilities.The helper functions provide a clean abstraction layer for wallet UI interactions, with consistent patterns for waiting and clicking elements. The
waitForLocatorutility with a 20-second timeout is appropriate for E2E tests.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
df4fb78 to
ee0c2b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/ui/components/custom/popup.tsx (1)
120-135: Well-implemented button rendering refactor.The
buttonsForSidehelper properly:
- Extracts duplicate button rendering logic for both left and right sides
- Conditionally sets
aria-labelonly whenlabelis a string, avoiding the[object Object]accessibility issue- Uses
useCallbackwith appropriate dependencies to prevent unnecessary re-renders- Handles loading state with the Spinner component
This addresses the past review concern about stringifying non-string labels.
One minor point: The
sideparameter inbuttonsForSidecould be typed more explicitly asActionButtonSideinstead of the string literal union, though the current implementation is functionally correct:- const buttonsForSide = useCallback( - (side: "left" | "right") => { + const buttonsForSide = useCallback( + (side: ActionButtonSide) => {apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
10-10: Unused feeType property.The
feeTypeproperty is defined but never used within this class. If it's intended for future functionality or subclass overrides, consider adding a brief comment explaining its purpose. Otherwise, it can be removed to reduce confusion.If keeping for future use:
+ // Fee type for transaction operations (intended for future use) protected readonly feeType: FeeType = "low";Or remove if not needed:
- protected readonly feeType: FeeType = "low";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/tests/ui/fixture/testing-helpers.ts
- apps/deploy-web/tests/ui/change-wallets.spec.ts
🧰 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/provider-console/src/components/layout/WalletStatus.tsxpackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/pages/FrontPage.tsxapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/actions/selectChainNetwork.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/provider-console/src/components/layout/WalletStatus.tsxpackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/pages/FrontPage.tsxapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/disconnect-wallet.spec.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/deploy-web/tests/ui/actions/selectChainNetwork.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/disconnect-wallet.spec.ts
🧬 Code graph analysis (7)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
getExtensionPage(15-27)
packages/ui/components/custom/popup.tsx (1)
packages/ui/components/spinner.tsx (1)
Spinner(10-39)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (2)
apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(12-16)apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
createWallet(36-51)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
apps/deploy-web/src/services/auth/auth/auth.service.ts (1)
logout(31-35)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
test(12-68)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(9-39)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (2)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
createWallet(21-29)apps/deploy-web/tests/ui/fixture/testing-helpers.ts (5)
clickWalletSelectorDropdown(8-10)clickCreateNewWalletButton(16-20)fillWalletName(22-25)clickConnectWalletButton(32-35)clickCreateWalletButton(27-30)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(41-45)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (9)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
75-75: LGTM! Accessibility improvement.Adding
aria-label="Disconnect Wallet"improves accessibility and enables reliable test automation through semantic selectors.apps/provider-console/src/components/layout/WalletStatus.tsx (1)
42-42: LGTM! Consistent accessibility labeling.The aria-label addition maintains consistency with the Disconnect Wallet button in CustodialWalletPopup.tsx and supports accessible test automation.
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (2)
10-11: Good refactor to centralize wait logic.Using
waitForLocatorimproves consistency across test actions and ensures the element is visible before interaction.
13-13: RegExp safety in test context.The static analysis tool flags the RegExp construction from the
networkIdvariable. However, in this test context wherenetworkIddefaults to "sandbox" and is controlled by test authors, the ReDoS risk is negligible. This is a false positive for test code.apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (1)
4-10: Test structure is acceptable with delegation to page object.The test properly delegates to
FrontPage.disconnectWallet(), which performs the disconnect action and assertions. This follows the page object pattern and keeps the test concise.The 5-minute timeout at line 5 is notably long for a disconnect operation. Consider if this can be reduced based on actual execution time to catch genuine issues faster.
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
15-27: LGTM! Robust extension page helper.The
getExtensionPagefunction properly:
- Reuses existing extension pages when available
- Creates and navigates to the extension URL when needed
- Waits for the page to load
- Handles the optional page event with a timeout and catch
This centralizes extension page management and eliminates duplication.
apps/deploy-web/tests/ui/pages/FrontPage.tsx (3)
8-15: Well-structured page object with clear responsibilities.The FrontPage class follows the page object pattern effectively:
- Encapsulates page navigation and wallet interactions
- Uses accessible locators (getByLabel, getByTestId)
- Properly stores context and page references
- Constructor is clean and straightforward (past issue resolved)
21-29: Solid wallet creation flow with proper assertions.The
createWalletmethod:
- Generates a unique wallet name using faker for test isolation
- Delegates to the wallet-setup helper for the creation flow
- Reloads the page to verify persistence
- Waits for and asserts the wallet name appears correctly
The 20-second timeout is appropriate given the complexity of the wallet creation flow involving extension interactions.
31-38: Disconnect flow properly validates UI state transitions.The
disconnectWalletmethod correctly:
- Triggers the disconnect popup via hover and click (using accessible labels)
- Reloads the page to verify the disconnect persists
- Asserts the connect button is visible, confirming the disconnected state
This aligns with the PR objective to verify wallet disconnection and that it doesn't auto-reconnect after page refresh.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
ee0c2b1 to
9a7dc9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/components/custom/popup.tsx (1)
127-127: Consider renaming the destructured parameter to avoid shadowing.The destructured
sideparameter in the map callback shadows the outer function parameterside. While this doesn't cause a bug (the destructuredsideis not used), it reduces code clarity.Consider using
_or omitting it entirely since it's not used:- .map(({ isLoading, side, label, ...rest }, idx) => ( + .map(({ isLoading, label, ...rest }, idx) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/deploy-web/tests/ui/fixture/context-with-extension.ts
- apps/deploy-web/tests/ui/pages/FrontPage.tsx
- apps/deploy-web/tests/ui/fixture/wallet-setup.ts
- apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx
- apps/deploy-web/tests/ui/disconnect-wallet.spec.ts
- apps/deploy-web/tests/ui/fixture/testing-helpers.ts
- apps/deploy-web/tests/ui/change-wallets.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/provider-console/src/components/layout/WalletStatus.tsxpackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.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/provider-console/src/components/layout/WalletStatus.tsxpackages/ui/components/custom/popup.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.ts
🧬 Code graph analysis (2)
packages/ui/components/custom/popup.tsx (1)
packages/ui/components/spinner.tsx (1)
Spinner(10-39)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(41-45)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (5)
apps/provider-console/src/components/layout/WalletStatus.tsx (1)
42-42: Aria-label is redundant but acceptable for test selectors.The
aria-label="Disconnect Wallet"duplicates the visible text content on line 44. From a pure accessibility standpoint, this is unnecessary since the visible text already provides a descriptive label. However, given the PR's focus on E2E testing (per the linked issue #2007), this is likely intentional as a test selector for Playwright, which is a common and acceptable pattern.apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (2)
4-4: LGTM! Improved test reliability.Using
waitForLocatorto explicitly wait for the element to be visible before clicking is a good practice that reduces test flakiness and improves reliability.Also applies to: 10-11
13-13: Static analysis warning: Regex from variable input.The static analysis tool flags constructing a regex from the
networkIdparameter, warning about potential ReDoS attacks. In this test context with controlled input (defaults to "sandbox"), the risk is minimal. However, if you want to be extra defensive, consider validating thatnetworkIdcontains only safe characters (alphanumeric + hyphens) before constructing the regex.Optional defensive pattern:
export async function selectChainNetwork(page: Page, networkId = "sandbox") { + // Validate networkId to prevent ReDoS + if (!/^[a-zA-Z0-9-]+$/.test(networkId)) { + throw new Error(`Invalid networkId: ${networkId}`); + } + await page.getByRole("link", { name: "App Settings" }).click();Alternatively, escape the
networkIdbefore using it in the regex:- await page.getByLabel(new RegExp(networkId, "i")).click(); + const escapedNetworkId = networkId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + await page.getByLabel(new RegExp(escapedNetworkId, "i")).click();However, given this is test code with controlled inputs, these changes are optional and may be overkill. The current implementation is acceptable for the test context.
packages/ui/components/custom/popup.tsx (2)
128-130: Accessibility fix correctly implemented.The
aria-labelattribute is now correctly set only whenlabelis a string, avoiding the[object Object]issue from the previous review. The loading state with the Spinner is also handled appropriately.
267-268: LGTM! Clean refactoring.The extraction of button rendering into the shared
buttonsForSidehelper eliminates code duplication and makes the component more maintainable.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
9a7dc9f to
836775d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/deploy-web/tests/ui/change-wallets.spec.ts (1)
4-10: Test name doesn't match implementation.The test is named "switching to another wallet" but only creates a single wallet via
frontPage.createWallet(extensionId). To fully satisfy the PR objectives (issue #2007: verify that changing the wallet in the extension is reflected in the console), the test should:
- Create a first wallet
- Create a second wallet with a different name
- Verify the console reflects the second (newly active) wallet
Alternatively, if the test only verifies wallet creation, rename it to match:
-test("switching to another wallet in the extension affects Console", async ({ page, context, extensionId }) => { +test("can create a wallet in the extension", async ({ page, context, extensionId }) => {Note: FrontPage.createWallet already verifies the new wallet name appears in the console, so this test does validate that wallet creation is reflected. However, the PR objective emphasizes switching between existing wallets rather than just creation.
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
36-51: Event handler leak if popup doesn't appear.The
pageHandleris registered at line 48 but only removed within the handler itself (line 46). If the "page" event never fires (e.g., popup blocked, extension behavior changes, or test error), the handler remains registered and could fire unexpectedly in subsequent tests.Wrap the click in a try/finally to guarantee cleanup:
+ let pageHandlerRegistered = false; const pageHandler = async (page: Page) => { await page.waitForLoadState("domcontentloaded"); await clickConnectWalletButton(page); context.off("page", pageHandler); + pageHandlerRegistered = false; }; context.on("page", pageHandler); + pageHandlerRegistered = true; - await clickCreateWalletButton(extPage); + try { + await clickCreateWalletButton(extPage); + } finally { + if (pageHandlerRegistered) { + context.off("page", pageHandler); + } + }Alternatively, add a timeout with explicit cleanup:
const pageHandler = async (page: Page) => { await page.waitForLoadState("domcontentloaded"); await clickConnectWalletButton(page); context.off("page", pageHandler); }; context.on("page", pageHandler); - await clickCreateWalletButton(extPage); + try { + await Promise.race([ + clickCreateWalletButton(extPage).then(() => context.waitForEvent("page", { timeout: 10_000 })), + new Promise((_, reject) => setTimeout(() => reject(new Error("Popup timeout")), 10_000)) + ]); + } finally { + context.off("page", pageHandler); + }
🧹 Nitpick comments (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
3-10: Consider using Locator API for consistency.
getWalletSelectorDropdownandclickWalletSelectorDropdownuse the olderpage.$$(ElementHandle) API. For consistency with the rest of the helpers (which use Locators), consider refactoring to use the Locator API:export const getWalletSelectorDropdown = async (page: Page) => { - const buttons = await page.$$("#popup-layout button"); - return buttons.length ? buttons[0] : null; + const button = page.locator("#popup-layout button").first(); + const count = await button.count(); + return count > 0 ? button : null; };Or simplify
clickWalletSelectorDropdownto use Locator directly:export const clickWalletSelectorDropdown = async (page: Page) => { - return (await getWalletSelectorDropdown(page))?.click(); + const button = page.locator("#popup-layout button").first(); + if (await button.count() > 0) { + await button.click(); + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx
- apps/deploy-web/tests/ui/disconnect-wallet.spec.ts
- packages/ui/components/custom/popup.tsx
- apps/deploy-web/tests/ui/pages/FrontPage.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/change-wallets.spec.ts
**/*.{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/change-wallets.spec.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/fixture/context-with-extension.tsapps/deploy-web/tests/ui/actions/selectChainNetwork.tsapps/deploy-web/tests/ui/fixture/testing-helpers.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/change-wallets.spec.tsapps/deploy-web/tests/ui/fixture/wallet-setup.tsapps/provider-console/src/components/layout/WalletStatus.tsxapps/deploy-web/tests/ui/fixture/context-with-extension.tsapps/deploy-web/tests/ui/actions/selectChainNetwork.tsapps/deploy-web/tests/ui/fixture/testing-helpers.ts
🧬 Code graph analysis (4)
apps/deploy-web/tests/ui/change-wallets.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
test(12-68)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(9-39)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (2)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
createWallet(21-29)apps/deploy-web/tests/ui/fixture/testing-helpers.ts (5)
clickWalletSelectorDropdown(8-10)clickCreateNewWalletButton(16-20)fillWalletName(22-25)clickConnectWalletButton(32-35)clickCreateWalletButton(27-30)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
getExtensionPage(15-27)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(41-45)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (5)
apps/provider-console/src/components/layout/WalletStatus.tsx (1)
42-42: LGTM! Accessibility improvement enables reliable test automation.Adding
aria-label="Disconnect Wallet"to the dropdown item improves accessibility and allows E2E tests to reliably locate and interact with the disconnect control.apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
4-4: Good refactor using the new waitForLocator helper.The two-step pattern—obtaining a locator via
waitForLocatorand then clicking it—improves test reliability by ensuring the element is visible before interaction.Also applies to: 10-11
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
52-52: LGTM! Duplication eliminated by using shared helper.The refactor correctly delegates extension page retrieval to
getExtensionPage, eliminating duplicated lookup logic and improving maintainability.apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
15-27: LGTM! Extension page retrieval logic is solid.The function correctly handles both finding an existing extension page and creating a new one, with appropriate waits for load state and page events.
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
41-45: LGTM! Well-designed helper for reliable test interactions.The
waitForLocatorhelper provides a clean, reusable pattern for ensuring elements are visible before interaction, improving test reliability across the suite.
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
836775d to
c9e7039
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
13-13: Consider escaping regex special characters in test input.While the ReDoS risk is minimal in a test context where inputs are controlled, you could make the code more robust by escaping regex special characters or using a simpler matching approach.
Option 1: Escape special characters in the regex:
- await page.getByLabel(new RegExp(networkId, "i")).click(); + await page.getByLabel(new RegExp(networkId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), "i")).click();Option 2: If exact case-insensitive matching suffices, consider using a static pattern or test-specific label matching approach instead of dynamic regex construction.
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (1)
5-5: Consider reducing the test timeout.A 5-minute timeout is excessive for a simple disconnect workflow (hover → click → reload → assert). Typical UI interactions complete in seconds, and such a long timeout can mask performance issues or hanging operations.
Apply this diff to reduce the timeout to a more reasonable duration:
- test.setTimeout(5 * 60 * 1000); + test.setTimeout(30_000); // 30 secondsapps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
8-10: Remove unused type and property.The
FeeTypetype andfeeTypeproperty are defined but never used within the class or by external consumers. This is dead code that adds unnecessary maintenance burden.Apply this diff to remove the unused declarations:
-export type FeeType = "low" | "medium" | "high"; export class FrontPage { - protected readonly feeType: FeeType = "low"; - constructor(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx(1 hunks)apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/FrontPage.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/provider-console/src/components/layout/WalletStatus.tsx
- packages/ui/components/custom/popup.tsx
- apps/deploy-web/tests/ui/change-wallets.spec.ts
- apps/deploy-web/tests/ui/fixture/context-with-extension.ts
- apps/deploy-web/tests/ui/fixture/wallet-setup.ts
- apps/deploy-web/tests/ui/fixture/testing-helpers.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/disconnect-wallet.spec.ts
**/*.{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/disconnect-wallet.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/pages/FrontPage.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.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/disconnect-wallet.spec.tsapps/deploy-web/src/components/wallet/CustodialWalletPopup.tsxapps/deploy-web/tests/ui/pages/FrontPage.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.ts
🧠 Learnings (1)
📚 Learning: 2025-10-15T16:39:55.311Z
Learnt from: jzsfkzm
PR: akash-network/console#2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.311Z
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/disconnect-wallet.spec.tsapps/deploy-web/tests/ui/pages/FrontPage.tsx
🧬 Code graph analysis (3)
apps/deploy-web/tests/ui/disconnect-wallet.spec.ts (2)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
test(12-68)apps/deploy-web/tests/ui/pages/FrontPage.tsx (1)
FrontPage(9-39)
apps/deploy-web/tests/ui/pages/FrontPage.tsx (2)
apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(12-16)apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
createWallet(36-51)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(37-41)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (2)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
4-4: LGTM! Good practice for test reliability.Adding an explicit visibility wait via
waitForLocatorbefore clicking the "Select Network" button reduces test flakiness and aligns with Playwright best practices for handling timing issues in E2E tests.Also applies to: 9-11
apps/deploy-web/src/components/wallet/CustodialWalletPopup.tsx (1)
75-78: Good accessibility addition!The
aria-label="Disconnect Wallet"attribute enhances test reliability and accessibility. This aligns with the test suite that usesgetByLabel("Disconnect Wallet")to interact with this control.
also fix duplicate code with adding a callback for buttons refs akash-network#2007
jzsfkzm
force-pushed
the
test/2007-change-wallets
branch
from
c9e7039 to
ebec470
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
78-80: Removeanyand the unusedchromedeclaration.This violates our TS guideline (“never use any”). It’s also unused here—just delete it.
-declare global { - const chrome: any; -}
🧹 Nitpick comments (6)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (2)
28-31: Explicitly set headless: false for extension testing.Chromium extensions require headful mode. Make it explicit to avoid environment-dependent defaults.
- const context = await chromium.launchPersistentContext(userDataDir, { - channel: "chromium", - args - }); + const context = await chromium.launchPersistentContext(userDataDir, { + channel: "chromium", + headless: false, + args + });
54-55: Ensure extPage is always closed (use finally).Prevent leaked pages on failures during setup.
- await setupWallet(context, extPage); - await extPage.close(); + try { + await setupWallet(context, extPage); + } finally { + await extPage.close(); + }apps/deploy-web/tests/ui/pages/LeapExt.tsx (2)
23-30: Tighten popup wait (predicate, timeout) and ensure load before clicking.Reduces flakiness and avoids catching unrelated pages.
- const [popup] = await Promise.all([ - this.context.waitForEvent("page"), - wait(100).then(() => { - this.page.reload({ waitUntil: "domcontentloaded" }); - }) - ]); - await clickConnectWalletButton(popup); + const [popup] = await Promise.all([ + this.context.waitForEvent("page", { + timeout: 10_000, + predicate: p => p.url().startsWith(`chrome-extension://${extensionId}`), + }), + wait(100).then(() => { + this.page.reload({ waitUntil: "domcontentloaded" }); + }), + ]); + await popup.waitForLoadState("domcontentloaded"); + await clickConnectWalletButton(popup);
19-22: Optional: seed faker in tests for reproducibility.If tests assert the wallet name, seed faker once in the test suite to keep names stable across runs.
Based on learnings
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (2)
3-10: Avoid brittle CSS and silent no-op; wait for and click a locator.CSS “#popup-layout button” + optional chaining can silently skip clicks. Use a locator with an explicit wait.
-export const getWalletSelectorDropdown = async (page: Page) => { - const buttons = await page.$$("#popup-layout button"); - return buttons.length ? buttons[0] : null; -}; - -export const clickWalletSelectorDropdown = async (page: Page) => { - return (await getWalletSelectorDropdown(page))?.click(); -}; +export const getWalletSelectorDropdown = async (page: Page) => { + // Prefer role or data-testid if available; fallback to first button inside popup-layout. + return page.locator("#popup-layout button").first(); +}; + +export const clickWalletSelectorDropdown = async (page: Page) => { + const dropdown = await waitForLocator(getWalletSelectorDropdown(page)); + await dropdown.click(); +};
12-16: Prefer role-based selector for the “Create new wallet” action.More resilient and accessible than text-only.
-export const clickCreateNewWalletButton = async (page: Page) => { - const createNewWalletButton = page.getByText(/create new wallet/i); - await createNewWalletButton.waitFor({ state: "visible", timeout: 10_000 }); - await createNewWalletButton.click(); -}; +export const clickCreateNewWalletButton = async (page: Page) => { + const button = await waitForLocator(page.getByRole("button", { name: /Create new wallet/i })); + await button.click(); +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts(1 hunks)apps/deploy-web/tests/ui/change-wallets.spec.ts(1 hunks)apps/deploy-web/tests/ui/disconnect-wallet.spec.ts(1 hunks)apps/deploy-web/tests/ui/fixture/context-with-extension.ts(2 hunks)apps/deploy-web/tests/ui/fixture/testing-helpers.ts(1 hunks)apps/deploy-web/tests/ui/fixture/wallet-setup.ts(1 hunks)apps/deploy-web/tests/ui/pages/LeapExt.tsx(1 hunks)apps/provider-console/src/components/layout/WalletStatus.tsx(1 hunks)packages/ui/components/custom/popup.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/deploy-web/tests/ui/disconnect-wallet.spec.ts
- apps/deploy-web/tests/ui/change-wallets.spec.ts
- apps/deploy-web/tests/ui/fixture/wallet-setup.ts
- apps/provider-console/src/components/layout/WalletStatus.tsx
- packages/ui/components/custom/popup.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/testing-helpers.tsapps/deploy-web/tests/ui/pages/LeapExt.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.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/testing-helpers.tsapps/deploy-web/tests/ui/pages/LeapExt.tsxapps/deploy-web/tests/ui/actions/selectChainNetwork.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
PR: akash-network/console#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
PR: akash-network/console#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/pages/LeapExt.tsx
🧬 Code graph analysis (3)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
getExtensionPage(15-27)
apps/deploy-web/tests/ui/pages/LeapExt.tsx (4)
apps/deploy-web/tests/ui/fixture/test-env.config.ts (1)
testEnvConfig(12-16)apps/deploy-web/tests/ui/fixture/wallet-setup.ts (1)
createWallet(36-44)apps/deploy-web/src/utils/timer.ts (1)
wait(26-32)apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
clickConnectWalletButton(28-31)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
waitForLocator(33-37)
🪛 ast-grep (0.39.6)
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(networkId, "i")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (3)
apps/deploy-web/tests/ui/fixture/context-with-extension.ts (1)
59-64: Validate the double connect sequence.We connect, select network, then connect again. If the second connect is no longer necessary, drop it to reduce flakiness.
What scenarios still require the second connect? If none, remove Line 63-64.
apps/deploy-web/tests/ui/actions/selectChainNetwork.ts (1)
10-12: Good wait-before-click pattern.Using waitForLocator reduces flakiness on the “Select Network” control.
apps/deploy-web/tests/ui/fixture/testing-helpers.ts (1)
28-31: LGTM: standardized waitForLocator usage.Consistent locator waits improve stability of connect and follow-up actions.
Also applies to: 33-37
2 participants
closes #2007
Summary by CodeRabbit
Tests
Refactor
Style