chore: restore e2e tests #6751
chore: restore e2e tests #6751
Conversation
|
@crutch12 is attempting to deploy a commit to the cow Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a root E2E Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Cypress test
participant B as Browser / App
participant E as window.ethereum (injected)
participant M as Mock middleware chain
Note over T,B: cy.visit(..., onBeforeLoad) installs stubs & mocks
T->>B: cy.visit(onBeforeLoad)
B->>E: App invokes ethereum.send(method, params)
E->>M: Stubbed send routes call to middleware chain
alt middleware handles call (balance/allowance/multicall)
M-->>E: Return mocked RPC response
else not handled
M-->>E: Forward to original provider / real send
end
E-->>B: RPC response (mocked or real)
B-->>T: UI updates (balances/allowances)
T->>B: cy.unlockCrossChainSwap() (click `#unlock-cross-chain-swap-btn`)
B-->>T: UI enters unlocked state
T->>B: Click `#do-trade-button` / `#approve-trade-button`
B-->>T: Confirmation UI appears
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 4
🧹 Nitpick comments (7)
.env (1)
1-8: Add a trailing blank line.The file is missing a blank line at the end, which is flagged by the dotenv-linter.
🔎 Proposed fix
CYPRESS_INTEGRATION_TEST_PRIVATE_KEY= CYPRESS_INTEGRATION_TESTS_INFURA_KEY= +apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
22-22: Minor: Trailing comma in error message.The error message string now includes a trailing comma, which is unconventional but harmless. Consider removing it for cleaner output.
🔎 Proposed fix
- `PROVIDER_URL is empty, NETWORK_URL=${NETWORK_URL}, INTEGRATION_TESTS_INFURA_KEY=${INTEGRATION_TESTS_INFURA_KEY}`, + `PROVIDER_URL is empty, NETWORK_URL=${NETWORK_URL}, INTEGRATION_TESTS_INFURA_KEY=${INTEGRATION_TESTS_INFURA_KEY}`apps/cowswap-frontend-e2e/cypress.config.ts (1)
34-34: Consider if 30s page load timeout is sufficient for E2E tests.The
pageLoadTimeoutis set to 30 seconds (reduced from Cypress's default of 60 seconds). While this may work for fast development environments, E2E tests running in CI or slower environments might experience intermittent failures due to genuine slow page loads.apps/cowswap-frontend-e2e/src/e2e/fee.test.ts (1)
98-100: Replace hardcodedwait(1000)with proper element waiting.Using
cy.wait(1000)introduces flakiness and unnecessary delays. Instead, wait for specific UI elements or conditions to be ready.Example: wait for output token to be visible
cy.unlockCrossChainSwap() cy.swapSelectInput(USDC) -cy.wait(1000) +cy.get('#output-currency-input .open-currency-select-button').should('be.visible') cy.swapSelectOutput(COW)Also applies to: 145-147
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (1)
102-102: Replace hardcodedcy.wait(1000)with explicit element waiting.Similar to the fee tests, this hardcoded delay introduces flakiness. Wait for the specific UI element to be ready instead.
Example refactor
acceptFeesExceedWarning() -cy.wait(1000) -cy.get('#classic-eth-flow-banner').should('contain', 'Wrap your ETH and use the classic WETH experience').click() +cy.get('#classic-eth-flow-banner', { timeout: 5000 }).should('be.visible').should('contain', 'Wrap your ETH and use the classic WETH experience').click()apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (2)
13-51: Consider adding explicit return types.The function correctly handles both callback and promise forms of
ethereum.send. However, the return type andFunctiontype on line 15 could be more specific for better type safety.🔎 Optional refactor for type safety
-// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-function-return-type -function parseSendArgs(send: typeof injected.send, ...args: any[]) { +function parseSendArgs(send: typeof injected.send, ...args: any[]): { + method: string | undefined + params: any[] | undefined + getOriginalResult: () => Promise<BytesLike> + returnResult: (value: unknown) => Promise<unknown> | void +} { const isCallbackForm = typeof args[0] === 'object' && typeof args[1] === 'function' - let callback: Function | undefined + let callback: ((value: unknown) => void) | undefined
94-94: Redundant property assignment.Setting both
item[1]anditem.returnDataassigns to the same property twice sinceitem[1]anditem.returnDatarefer to the same value in the tuple.🔎 Simplify to single assignment
- item[1] = item.returnData = returnData + item.returnData = returnData
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.envREADME.mdapps/cowswap-frontend-e2e/cypress.config.tsapps/cowswap-frontend-e2e/src/e2e/fee.test.tsapps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.tsapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend-e2e/src/e2e/lists.test.tsapps/cowswap-frontend-e2e/src/e2e/search.test.tsapps/cowswap-frontend-e2e/src/e2e/send.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/commands.tsapps/cowswap-frontend-e2e/src/support/e2e.tsapps/cowswap-frontend-e2e/src/support/ethereum.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.tsapps/cowswap-frontend-e2e/tsconfig.jsonapps/cowswap-frontend/.envapps/cowswap-frontend/src/legacy/components/ErrorBoundary/index.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsxapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend/src/serviceWorkerRegistration.ts
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/.env
- apps/cowswap-frontend-e2e/src/e2e/send.test.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/search.test.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.tsapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/e2e/lists.test.tsapps/cowswap-frontend-e2e/src/e2e/fee.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/commands.tsapps/cowswap-frontend-e2e/src/e2e/fee.test.ts
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/token.test.ts
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/commands.ts
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx
📚 Learning: 2025-11-19T10:18:23.717Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6537
File: apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx:33-35
Timestamp: 2025-11-19T10:18:23.717Z
Learning: In apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx, when there is no partner fee (amount is null/undefined, bps is missing, or amount equals 0), FreeFeeRow is rendered with withTimelineDot={false} hardcoded. This is intentional design—free fee rows should not show the timeline dot regardless of what the parent component passes, as they have a distinct visual treatment from actual fee rows.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend-e2e/src/e2e/fee.test.ts
📚 Learning: 2025-05-26T12:39:29.009Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 5715
File: libs/common-const/src/tokens.ts:539-555
Timestamp: 2025-05-26T12:39:29.009Z
Learning: The team accepts using NATIVE_CURRENCY_ADDRESS as a temporary placeholder for COW token contract addresses on new networks (Polygon, Avalanche) until actual COW contracts are deployed.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-06-25T07:28:32.570Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-02-21T13:00:04.809Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/hooks/useSwapWidgetActions.ts:37-37
Timestamp: 2025-02-21T13:00:04.809Z
Learning: Recipient address validation in the swap widget is handled in a different layer, not in the onChangeRecipient callback.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/fee.test.ts
📚 Learning: 2025-12-03T19:23:35.789Z
Learnt from: kernelwhisperer
Repo: cowprotocol/cowswap PR: 6610
File: apps/cowswap-frontend/project.json:38-39
Timestamp: 2025-12-03T19:23:35.789Z
Learning: In the cowswap-frontend project configuration (apps/cowswap-frontend/project.json), the Vite dev server option `"host": true` is set to support Windows + WSL development environments, where binding to all network interfaces is required for the Windows host to access the dev server running in WSL.
Applied to files:
apps/cowswap-frontend-e2e/cypress.config.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
🧬 Code graph analysis (6)
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (2)
libs/common-const/src/tokens.ts (1)
USDC(540-557)apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (4)
mockSendCall(140-151)handleTokenBalance(104-106)handleTokenAllowance(108-110)handleNativeBalance(136-138)
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx (1)
libs/ui/src/pure/Button/index.tsx (1)
ButtonError(232-238)
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (2)
send(47-118)injected(124-124)
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts (1)
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (2)
mockSendCall(140-151)handleNativeBalance(136-138)
apps/cowswap-frontend-e2e/src/e2e/fee.test.ts (1)
libs/common-const/src/tokens.ts (1)
USDC(540-557)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
injected(124-124)
🪛 dotenv-linter (4.0.0)
.env
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 8-8: [UnorderedKey] The CYPRESS_INTEGRATION_TESTS_INFURA_KEY key should go before the CYPRESS_INTEGRATION_TEST_PRIVATE_KEY key
(UnorderedKey)
⏰ 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: Setup
- GitHub Check: Cypress
🔇 Additional comments (31)
apps/cowswap-frontend/src/legacy/components/ErrorBoundary/index.tsx (3)
62-64: LGTM! Defensive runtime guard for service worker availability.This check prevents errors in environments where service workers are unavailable (e.g., E2E tests, certain browsers). The error is properly caught by the catch block at lines 97-99.
103-107: LGTM! Explicit return type annotations improve type safety.The explicit
: voidand: React.ReactNodeannotations correctly reflect React's lifecycle method signatures and improve TypeScript type safety.
147-152: LGTM! Explicit return type annotations are correct.The
: React.ReactNodereturn type for the HOC and: voidfor the error handler correctly reflect their behavior and improve TypeScript type safety.apps/cowswap-frontend/src/serviceWorkerRegistration.ts (1)
26-26: LGTM! Explicit return type annotations improve type safety.Adding explicit
: voidreturn types toregisterValidSW,checkValidServiceWorker,register, andunregisteris good TypeScript practice. The early return guard at line 96 is also a sensible defensive check.Also applies to: 70-70, 95-96, 132-132
README.md (1)
116-120: LGTM!The documentation updates correctly reflect the new Cypress-prefixed environment variables and provide clearer instructions for E2E test setup.
apps/cowswap-frontend-e2e/src/e2e/lists.test.ts (1)
6-7: LGTM!Re-enabling the test with the proper prerequisite (unlockCrossChainSwap) is appropriate. This aligns with the PR's objective to restore E2E tests.
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx (1)
54-59: LGTM!Adding the id attribute provides a stable selector for E2E tests. The naming is clear and follows good practices for test automation.
apps/cowswap-frontend-e2e/src/support/e2e.ts (3)
48-56: LGTM!Disabling the service worker via
window:before:loadis the correct approach to prevent interference with test execution, as noted in the comment about Safari SDK and window.load event issues.
58-69: LGTM!Blocking analytics and tracking URLs during E2E tests prevents external network dependencies and speeds up test execution.
71-89: LGTM!Moving initialization to
window:before:loadwithinbeforeEachensures proper setup timing. UsingCypress.config('baseUrl')for the origin header is more flexible than a hardcoded value.apps/cowswap-frontend-e2e/src/e2e/search.test.ts (2)
1-3: LGTM!Adding an explicit return type improves code clarity and type safety.
10-86: LGTM!Re-enabling the test suite with consistent prerequisites (unlockCrossChainSwap) across all tests is appropriate and aligns with the PR's objective to restore E2E tests.
apps/cowswap-frontend-e2e/src/e2e/token.test.ts (2)
3-6: Good approach to speed up tests by mocking slow endpoints.The intercept for
safe.global/tx-servicereturning a 404 effectively bypasses a slow-loading dependency, allowing the token search tests to run without the long wait times.
14-30: LGTM! Token search tests are now active and well-structured.The tests properly verify token search by name, address, and case-insensitive address. Each test independently visits the page for good isolation.
apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts (2)
1-7: Good addition of explicit return types.Adding
Cypress.Chainablereturn types improves TypeScript support and makes the helper functions' contracts clearer.
18-38: LGTM! Test properly verifies fiat amount scaling.The test includes the necessary unlock step and correctly verifies that fiat amounts scale proportionally with input amounts.
apps/cowswap-frontend-e2e/src/support/commands.ts (2)
69-74: LGTM! Well-structured Cypress command addition.The
unlockCrossChainSwapcommand is properly documented, includes visibility checks before clicking, and follows Cypress best practices. The command is correctly registered and typed.Also applies to: 185-187, 214-214
108-108: Good improvement for case-insensitive token matching.Converting the token address to lowercase ensures the selector works regardless of address casing variations in the DOM.
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts (2)
1-1: Good addition of mocking utilities for reliable E2E tests.Importing and using
mockSendCallwithhandleNativeBalanceallows tests to run with predictable blockchain state, reducing flakiness from actual network conditions.Also applies to: 79-89
11-116: LGTM! Comprehensive swap test coverage with proper setup.The test suite now includes:
- Proper per-test isolation with individual
cy.visitcalls- Cross-chain swap unlocking before interactions
- Multiple input validation scenarios (empty, valid, zero, invalid)
- Native token swap with mocked balances
- Recipient mode verification
The updated selector
#do-trade-buttonaligns with the UI changes mentioned in the PR.apps/cowswap-frontend-e2e/src/e2e/limit-orders.test.ts (2)
5-23: Good addition of explicit return types for helper functions.Adding
voidandCypress.Chainablereturn types improves type safety and code clarity.
25-76: LGTM! Comprehensive limit order URL parameter testing.The test suite is now active and includes thorough coverage of URL parameter handling:
- Individual
sellAmountandbuyAmountparameters- Parameter validation when tokens are missing
- Parameter precedence rules
The skipped confirmation test has a clear TODO marker for future work.
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (2)
5-10: Excellent use of mocking for reliable swap tests.The tests properly mock blockchain interactions using
mockSendCallwith:
- Token balances via
handleTokenBalance- Token allowances via
handleTokenAllowance- Native ETH balance via
handleNativeBalanceThis ensures tests run consistently without depending on actual blockchain state.
Also applies to: 34-49, 61-71, 84-94
31-142: LGTM! Comprehensive swap test suite with proper E2E coverage.The test suite now includes:
- WETH/USDC swaps with mocked balances and allowances
- Native ETH swaps with proper balance mocking
- ETH wrapping flow with banner interactions
- Extensive URL parameter validation
Tests are well-isolated and include proper unlock steps. The selector updates to
#do-trade-buttonalign with the UI changes.apps/cowswap-frontend-e2e/cypress.config.ts (1)
3-5: All three Vite plugins (vite-plugin-babel-macros,vite-plugin-node-polyfills,vite-tsconfig-paths) are properly declared in package.json devDependencies and are correctly imported in this configuration.apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (6)
1-11: LGTM: Imports and setup are appropriate.The ESLint suppressions are acceptable for test utilities, and the direct import of
getMulticallContractto avoid bundling overhead is well-documented.
104-110: LGTM: Token balance and allowance handlers are correct.The ERC-20 method selectors (
0x70a08231forbalanceOf,0xdd62ed3eforallowance) are accurate, and the encoding is appropriate.
136-138: LGTM: Native balance handler wrapper is correct.The function appropriately encodes the balance value and delegates to
handleNativeBalanceCall.
140-151: LGTM: Middleware chain implementation is solid.The middleware pattern correctly chains handlers and falls back to the original
sendmethod when no middleware matches. The use ofcy.stubis appropriate for Cypress tests.
54-54: LGTM: MockMiddleware type is appropriate.The type definition correctly captures the flexible middleware signature needed for intercepting various Ethereum RPC calls.
89-89: No action needed.structuredCloneis already used elsewhere in the codebase (e.g.,apps/explorer/src/explorer/pages/AppData/config.tsx) without compatibility issues or polyfills, indicating the project targets environments where it's supported. Cypress 15.4.0 with Node 20.x LTS runs in modern browsers that natively supportstructuredClone.
...wswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
50-56: Consider usingObject.getPrototypeOf()instead of__proto__.
__proto__is deprecated. A more modern approach:- // @ts-expect-error - delete win.navigator.__proto__.serviceWorker + // @ts-expect-error -- deleting from prototype to disable SW + delete Object.getPrototypeOf(win.navigator).serviceWorkerThis is a minor improvement for future compatibility.
apps/cowswap-frontend-e2e/cypress.config.ts (1)
24-24: Consider verifying if Babel macros are actively used.The
macrosPluginenables Babel macro transformations at build time. If the codebase doesn't use macro-based libraries (e.g.,styled-components/macro,preval.macro), this plugin may be unnecessary overhead.#!/bin/bash # Description: Search for macro usage in the codebase to verify if macrosPlugin is needed. echo "=== Searching for macro imports ===" rg -n --type ts --type tsx --type js --type jsx 'from.*\.macro' || echo "No macro imports found" echo "" echo "=== Searching for babel-plugin-macros usage ===" rg -n 'babel-plugin-macros|@babel/plugin-macros' package.json || echo "No babel macros plugin config found"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cowswap-frontend-e2e/cypress.config.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend-e2e/src/e2e/token.test.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-12-03T19:23:35.789Z
Learnt from: kernelwhisperer
Repo: cowprotocol/cowswap PR: 6610
File: apps/cowswap-frontend/project.json:38-39
Timestamp: 2025-12-03T19:23:35.789Z
Learning: In the cowswap-frontend project configuration (apps/cowswap-frontend/project.json), the Vite dev server option `"host": true` is set to support Windows + WSL development environments, where binding to all network interfaces is required for the Windows host to access the dev server running in WSL.
Applied to files:
apps/cowswap-frontend-e2e/cypress.config.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
injected(124-124)
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (9)
apps/cowswap-frontend-e2e/src/support/e2e.ts (3)
33-48: LGTM!The
visitcommand overwrite correctly ensures a clean localStorage state and injects the ethereum provider for each page visit.
58-71: LGTM!Blocking analytics and third-party requests in E2E tests is good practice for test reliability and speed.
85-119: LGTM!The
beforeEachsetup correctly handles:
- Infura origin headers using
baseUrlconfig- Blocking analytics/third-party requests
- Response caching for static resources
- Provider injection via
window:before:loadThe duplicate localStorage/ethereum setup in both the
visitoverwrite andwindow:before:loadhandler ensures coverage for all navigation patterns.apps/cowswap-frontend-e2e/cypress.config.ts (6)
11-26: LGTM! Vite configuration is appropriate for Cypress E2E testing.The
viteConfigOverridessection properly configures the Vite bundler for Cypress with Node.js polyfills, TypeScript path resolution, and Babel macro support. The plugin ordering (polyfills → path resolution → macro transformation) is logical.
13-20: LGTM! Node.js polyfills are correctly configured.The polyfills for Buffer, global, and process, along with
protocolImports, are necessary because Vite doesn't include Node.js polyfills by default. This configuration is appropriate for Cypress E2E tests.
33-33: Monitor for potential test flakiness from reduced timeout.Reducing
defaultCommandTimeoutfrom 15s to 5s could cause intermittent failures if network requests, UI rendering, or app interactions occasionally exceed 5 seconds. While this aligns with the "decrease delays" objective and you've validated the tests, monitor for flaky failures in CI, especially under varying network conditions or system load.
34-34: LGTM! Page load timeout is appropriate.Setting
pageLoadTimeoutto 15,000 ms is reasonable for E2E tests and aligns with the PR objective to fix page loading detection. This gives adequate time for the application to fully load before Cypress begins test execution.
21-23: Root path is correct and resolves to the monorepo root.The
root: '../../'correctly resolves to the repository root wheretsconfig.base.jsonis located.
3-5: All three plugins are at stable versions (1.0.6, 0.24.0, and 5.1.4 respectively) with no known direct security vulnerabilities.
crutch12
force-pushed
the
chore/restore-e2e-tests
branch
2 times, most recently
from
178be39 to
28c2122
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
77-78: Regex patterns still have unescaped dot beforejson.The
.*fix was applied, but the dot in.jsonremains unescaped, so it matches any character (e.g.,tokensXjson). This was flagged in a previous review.🔎 Proposed fix
- /files.cow.fi\/token-lists\/.*.json/, - /files.cow.fi\/tokens\/.*.json/, + /files.cow.fi\/token-lists\/.*\.json/, + /files.cow.fi\/tokens\/.*\.json/,
🧹 Nitpick comments (3)
apps/cowswap-frontend-e2e/src/support/e2e.ts (3)
27-29: TypeserviceWorker?: trueonly allows literaltrue.This type annotation is unusual—it restricts the property to only accept the literal value
true, notfalse. If the intent is to allow enabling/disabling, useboolean. If the intent is "when present, service worker is enabled", considerserviceWorker?: booleanor document the design choice.🔎 Suggested fix
interface VisitOptions { - serviceWorker?: true + serviceWorker?: boolean }
88-91: Non-null assertion onbaseUrlcould cause silent failures.If
baseUrlis not configured in Cypress config, this assertion will produceundefinedas the origin header, which may cause Infura requests to fail mysteriously.🔎 Suggested defensive approach
cy.intercept(/infura.io/, (req) => { - req.headers['origin'] = Cypress.config('baseUrl')! + const baseUrl = Cypress.config('baseUrl') + if (baseUrl) { + req.headers['origin'] = baseUrl + } req.continue() })
116-119: Duplicate initialization with visit command overwrite.Lines 43-44 in the
visitoverwrite already clear localStorage and injectethereum. ThisbeforeEachhandler does the same, resulting in double initialization whency.visit()is called. While harmless, consider removing one to avoid confusion.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cowswap-frontend-e2e/cypress.config.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend-e2e/cypress.config.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/token.test.ts
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-05-28T16:50:12.273Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
injected(124-124)
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (4)
apps/cowswap-frontend-e2e/src/e2e/token.test.ts (2)
3-6: Good approach to skip slow external service calls.Intercepting and returning 404 for the Safe tx-service speeds up test execution without affecting the token search functionality being tested.
14-30: Tests re-enabled with appropriate per-test page visits.The pattern of visiting the page within each
itblock ensures test isolation. Based on learnings, the placeholder test at lines 9-12 is intentionally kept to maintain CI stability.Consider adding explicit waits if these tests become flaky—
cy.gethas default retries, but token list loading may need additional guards if the backend is slow.apps/cowswap-frontend-e2e/src/support/e2e.ts (2)
50-56: Hacky but acceptable for test environment.Deleting from
__proto__to disable service workers is unconventional but reasonable for an e2e test harness where Safari SDK compatibility is needed. The@ts-expect-erroris appropriate here.
99-114: Module-level cache persists across spec files.
cypressCacheis defined at module scope, so cached responses persist across test files within the same Cypress run. This is likely intentional for performance, but be aware that stale data could affect tests if the underlying API changes mid-run.The
headers: undefinedworkaround forreq.replycompatibility is fragile—consider adding a brief comment explaining why headers must be stripped.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/cowswap-frontend-e2e/src/support/e2e.ts (3)
27-28: Minor type issue:serviceWorker?: trueseems unused.This type declaration appears unused in the visit command override. If this is meant to be a toggle to conditionally enable/disable service worker suppression, consider using
booleantype. Otherwise, if unused, it could be removed to reduce confusion.
77-78: Follow-up: Escape the literal dot beforejsonin regex patterns.The previous review flagged the
/*issue which was fixed, but the dot beforejsonis still unescaped. In regex,.matches any character, sofoo.jsonandfooXjsonwould both match. While unlikely to cause issues in practice, escaping the dot makes the intent clearer.🔎 Proposed fix
- /files.cow.fi\/token-lists\/.*.json/, - /files.cow.fi\/tokens\/.*.json/, + /files.cow.fi\/token-lists\/.*\.json/, + /files.cow.fi\/tokens\/.*\.json/,
88-91: Consider a fallback forbaseUrlto avoid silent failures.If
baseUrlis ever undefined (misconfigured test run), the origin header would be set to the string"undefined", which could cause confusing auth failures with Infura. A fallback or explicit error would make debugging easier.🔎 Proposed fix
cy.intercept(/infura.io/, (req) => { - req.headers['origin'] = Cypress.config('baseUrl')! + const baseUrl = Cypress.config('baseUrl') + if (!baseUrl) throw new Error('Cypress baseUrl must be configured for E2E tests') + req.headers['origin'] = baseUrl req.continue() })
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cowswap-frontend-e2e/cypress.config.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend-e2e/src/e2e/token.test.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-05-28T16:50:12.273Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-12-03T19:23:35.789Z
Learnt from: kernelwhisperer
Repo: cowprotocol/cowswap PR: 6610
File: apps/cowswap-frontend/project.json:38-39
Timestamp: 2025-12-03T19:23:35.789Z
Learning: In the cowswap-frontend project configuration (apps/cowswap-frontend/project.json), the Vite dev server option `"host": true` is set to support Windows + WSL development environments, where binding to all network interfaces is required for the Windows host to access the dev server running in WSL.
Applied to files:
apps/cowswap-frontend-e2e/cypress.config.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
injected(124-124)
⏰ 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: Setup
- GitHub Check: Cypress
🔇 Additional comments (8)
apps/cowswap-frontend-e2e/cypress.config.ts (4)
11-20: LGTM! Node polyfills configured appropriately for E2E tests.The nodePolyfills configuration with Buffer, global, and process globals is appropriate for E2E tests that interact with blockchain/Web3 libraries. The
protocolImports: truesetting enables Node.js protocol imports which is helpful for better compatibility.
21-25: LGTM! Path resolution and macros plugin configured correctly.The
viteTsConfigPathswithroot: '../../'correctly points to the monorepo root for resolving TypeScript path aliases. ThemacrosPlugin()enables Babel macro support which is useful for libraries like styled-components or twin.macro.
33-34: Monitor for flakiness after reducing command timeout.Reducing
defaultCommandTimeoutfrom 15s to 5s (67% reduction) significantly speeds up tests but may cause flakiness if any commands occasionally exceed 5s, especially in slower CI environments or under load. The newpageLoadTimeoutof 15s is reasonable.Consider monitoring test stability closely after this change, particularly in CI. If flakiness emerges, you may need to selectively increase timeout for specific commands using
cy.command({ timeout: 10000 })rather than reverting the global setting.
3-5: The Vite plugin packagesvite-plugin-babel-macros(^1.0.6),vite-plugin-node-polyfills(^0.24.0), andvite-tsconfig-paths(~5.1.4) are all installed in the project's devDependencies and properly imported. No action required.apps/cowswap-frontend-e2e/src/support/e2e.ts (4)
33-48: LGTM!The visit command override cleanly injects the ethereum provider and clears localStorage. Properly chains with user-provided
onBeforeLoadcallbacks.
50-56: LGTM!The prototype chain manipulation is an effective technique for disabling service workers in the test environment. The
@ts-expect-errorappropriately handles the non-standard delete operation.
116-119: Potentially redundant with visit command'sonBeforeLoad.This handler duplicates the localStorage clearing and ethereum injection that already happens in the overwritten
visitcommand (lines 41-44). Thecy.on('window:before:load')fires for all window loads including iframes, while the visit'sonBeforeLoadis scoped to the navigated page.If both are needed for specific scenarios (e.g., SPA navigation that doesn't trigger visit), consider adding a comment explaining why. Otherwise, this duplication may cause confusion or unnecessary double-initialization.
99-114: LGTM - reasonable caching strategy for static resources.The module-scoped cache improves test performance by avoiding repeated fetches of token lists and CMS data. Stripping headers is a known workaround for Cypress intercept limitations. The trade-off against strict test isolation is acceptable for these truly static resources.
kernelwhisperer
left a comment
kernelwhisperer
left a comment
There was a problem hiding this comment.
Amazing, thank you very much for this!
They are still a bit flaky on my machine (it might be the free infura key I'm using), but let's see how they behave in CI.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
kernelwhisperer
force-pushed
the
chore/restore-e2e-tests
branch
from
56d52a9 to
b0d3e65
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @.env:
- Around line 7-8: Remove the sensitive variable placeholders
CYPRESS_INTEGRATION_TEST_PRIVATE_KEY and CYPRESS_INTEGRATION_TESTS_INFURA_KEY
from the tracked .env file, add .env to .gitignore (or stop tracking it via git
rm --cached), create a .env.example that contains the same keys as non-sensitive
placeholders and usage notes, and update the README with instructions for
developers to create a local .env (or use a secret manager / .env.local) so real
secrets are not committed.
In @apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts:
- Around line 37-43: The callback branch in returnResult incorrectly invokes
callback(value) but the ethereum provider callback must follow (error, result)
semantics; update returnResult to call callback(null, { result: value }) (or
callback(null, value) to match surrounding usage in ethereum.ts at the
comparable call sites) so the first arg is null for no-error and the result
shape matches the other usages; ensure Promise.resolve is still returned when no
callback.
- Around line 28-35: getOriginalResult() currently forwards resolve directly as
the callback to send(), but CustomizedBridge.send calls callbacks with (error,
result) so the Promise resolves to null; change the callback form to create a
new Promise that takes (resolve, reject) and calls send({ method, params },
(err, res) => err ? reject(err) : resolve(res?.result ?? res)); update
getOriginalResult() to use this wrapper so the promise resolves with the actual
result bytes (or rejects on error).
♻️ Duplicate comments (1)
apps/cowswap-frontend-e2e/tsconfig.json (1)
7-32: Previous review comment remains unaddressed: missing@cowprotocol/cow-sdkmapping and unused path aliases.The prior review identified that:
@cowprotocol/cow-sdkis imported by E2E tests but is missing from thepathsconfiguration.- Only
@cowprotocol/multicallof the 23 mapped paths is actually used; the remaining 22 entries add unnecessary maintenance overhead.Please add the missing mapping and audit the unused entries to keep the configuration minimal and maintainable.
🧹 Nitpick comments (8)
.env (1)
1-8: Add trailing newline and reorder keys alphabetically.Two minor issues per static analysis:
- Missing blank line at end of file (standard POSIX compliance)
- Key ordering:
CYPRESS_INTEGRATION_TESTS_INFURA_KEYshould precedeCYPRESS_INTEGRATION_TEST_PRIVATE_KEYalphabetically🔎 Proposed fixes
####################################### # End To End Test ####################################### # ⚠️ Required for E2E tests to run # Cypress Integration Tests -CYPRESS_INTEGRATION_TEST_PRIVATE_KEY= CYPRESS_INTEGRATION_TESTS_INFURA_KEY= +CYPRESS_INTEGRATION_TEST_PRIVATE_KEY=(and add a blank line at the end of the file)
apps/cowswap-frontend/src/serviceWorkerRegistration.ts (1)
108-111: Minor redundancy with line 96 check, but harmless defensive coding.The check at line 96 already guards
register()before adding the event listener, so this inner guard would only execute if the outer check passed. This is technically redundant but provides an extra safety net.Consider removing if you prefer leaner code, otherwise keep it as-is for defense in depth.
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
20-23: Remove trailing comma from error message.The error message now ends with a trailing comma, which is inconsistent with standard error message formatting.
🔎 Proposed fix
assert( PROVIDER_URL, - `PROVIDER_URL is empty, NETWORK_URL=${NETWORK_URL}, INTEGRATION_TESTS_INFURA_KEY=${INTEGRATION_TESTS_INFURA_KEY}`, + `PROVIDER_URL is empty, NETWORK_URL=${NETWORK_URL}, INTEGRATION_TESTS_INFURA_KEY=${INTEGRATION_TESTS_INFURA_KEY}` )apps/cowswap-frontend-e2e/src/e2e/lists.test.ts (1)
13-16: Consider removing the placeholder test.Now that the actual test has been re-enabled, the placeholder test (
should be true) is no longer necessary and can be removed.Based on learnings, placeholder tests were used to keep CI green when the actual tests were disabled. Since the 'change list' test is now active, this placeholder is redundant.
apps/cowswap-frontend-e2e/src/e2e/search.test.ts (1)
5-8: Consider removing the placeholder test.The placeholder test is no longer necessary now that the Search test suite has been re-enabled and is active.
Based on learnings, this placeholder was used to keep CI green when tests were disabled. It can now be safely removed.
apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts (1)
40-43: Consider removing the placeholder test.With the actual fiat amounts test now re-enabled and functional, the placeholder test is redundant and can be removed.
Based on learnings, placeholder tests were used to keep CI green during test disablement. This is no longer necessary.
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (1)
102-102: Consider replacing hard wait with a more reliable assertion.
cy.wait(1000)introduces potential test flakiness. Consider waiting for a specific UI state instead.🔎 Proposed improvement
- cy.wait(1000) - cy.get('#classic-eth-flow-banner').should('contain', 'Wrap your ETH and use the classic WETH experience').click() + cy.get('#classic-eth-flow-banner', { timeout: 5000 }) + .should('be.visible') + .should('contain', 'Wrap your ETH and use the classic WETH experience') + .click()apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
116-119: Redundantwindow:before:loadhandler.This handler duplicates what the
visitcommand override already does (lines 43-44). Both clear localStorage and injectethereum. Sincevisitis called in every test, thisbeforeEachhandler appears unnecessary.🔎 Consider removing the redundant handler
cachedUrls.forEach((url) => { cy.intercept(url, (req) => { const cached = cypressCache.get(req.url) if (cached) { req.reply(cached) } else { req.continue((res) => { cypressCache.set(req.url, { ...res, headers: {}, // original headers break req.reply }) }) } }) }) - - cy.on('window:before:load', (win) => { - win.localStorage.clear() - win.ethereum = injected - }) })If there are scenarios where
cy.visitis not called (e.g., page refreshes or navigations that don't go through the overwritten command), please disregard this suggestion.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.envREADME.mdapps/cowswap-frontend-e2e/cypress.config.tsapps/cowswap-frontend-e2e/src/e2e/fee.test.tsapps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.tsapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend-e2e/src/e2e/lists.test.tsapps/cowswap-frontend-e2e/src/e2e/search.test.tsapps/cowswap-frontend-e2e/src/e2e/send.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/e2e/token.test.tsapps/cowswap-frontend-e2e/src/support/commands.tsapps/cowswap-frontend-e2e/src/support/e2e.tsapps/cowswap-frontend-e2e/src/support/ethereum.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.tsapps/cowswap-frontend-e2e/tsconfig.jsonapps/cowswap-frontend/.envapps/cowswap-frontend/src/legacy/components/ErrorBoundary/index.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsxapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend/src/serviceWorkerRegistration.ts
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/.env
- apps/cowswap-frontend-e2e/src/e2e/send.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/cowswap-frontend-e2e/src/e2e/token.test.ts
- apps/cowswap-frontend/src/legacy/components/ErrorBoundary/index.tsx
- README.md
- apps/cowswap-frontend-e2e/src/e2e/fee.test.ts
- apps/cowswap-frontend-e2e/cypress.config.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/search.test.tsapps/cowswap-frontend-e2e/src/e2e/lists.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-12-30T18:49:03.377Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6770
File: apps/cowswap-frontend/src/modules/tokensList/hooks/useAddListImport.ts:27-31
Timestamp: 2025-12-30T18:49:03.377Z
Learning: In apps/cowswap-frontend/src/modules/tokensList/hooks/useAddListImport.ts, when restrictedLists.isLoaded is false or geoStatus.isLoading is true, the code intentionally proceeds with the import immediately without blocking. This allows imports to proceed during loading states, deferring consent checks to trade time when necessary data isn't yet available.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/lists.test.ts
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend-e2e/src/support/commands.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/support/commands.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/e2e/limit-orders.test.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.tsapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.tsapps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-05-26T12:39:29.009Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 5715
File: libs/common-const/src/tokens.ts:539-555
Timestamp: 2025-05-26T12:39:29.009Z
Learning: The team accepts using NATIVE_CURRENCY_ADDRESS as a temporary placeholder for COW token contract addresses on new networks (Polygon, Avalanche) until actual COW contracts are deployed.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-06-25T07:28:32.570Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-02-21T13:00:04.809Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/hooks/useSwapWidgetActions.ts:37-37
Timestamp: 2025-02-21T13:00:04.809Z
Learning: Recipient address validation in the swap widget is handled in a different layer, not in the onChangeRecipient callback.
Applied to files:
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
📚 Learning: 2025-08-21T08:13:23.325Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6170
File: apps/cowswap-frontend/src/modules/twap/services/cancelTwapOrderTxs.ts:73-86
Timestamp: 2025-08-21T08:13:23.325Z
Learning: On zkSync chains, the standard ethers.js gas estimation approach using `{ from: address }` overrides doesn't work. The `.connect(FAKE_OWNER)` pattern with a plain address string is used instead, even though this seems unconventional compared to standard ethers.js usage where connect() expects a signer.
Applied to files:
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsxapps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx
📚 Learning: 2025-11-19T10:18:23.717Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6537
File: apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx:33-35
Timestamp: 2025-11-19T10:18:23.717Z
Learning: In apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx, when there is no partner fee (amount is null/undefined, bps is missing, or amount equals 0), FreeFeeRow is rendered with withTimelineDot={false} hardcoded. This is intentional design—free fee rows should not show the timeline dot regardless of what the parent component passes, as they have a distinct visual treatment from actual fee rows.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx
📚 Learning: 2025-05-28T16:50:12.273Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend-e2e/src/support/e2e.ts
🧬 Code graph analysis (4)
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (1)
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (4)
mockSendCall(141-152)handleTokenBalance(104-106)handleTokenAllowance(108-110)handleNativeBalance(137-139)
apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts (1)
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (2)
mockSendCall(141-152)handleNativeBalance(137-139)
apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (2)
send(47-118)injected(124-124)
apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
injected(124-124)
🪛 dotenv-linter (4.0.0)
.env
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 8-8: [UnorderedKey] The CYPRESS_INTEGRATION_TESTS_INFURA_KEY key should go before the CYPRESS_INTEGRATION_TEST_PRIVATE_KEY key
(UnorderedKey)
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (32)
apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx (1)
81-81: LGTM! Stable selector for E2E tests.Adding the
id="approve-trade-button"attribute provides a stable selector for E2E tests to target this button, which aligns with the PR's objective of restoring E2E test coverage.apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/swapTradeButtonsMap.tsx (1)
85-90: LGTM! Stable selector for E2E tests.Adding the
id="do-trade-button"attribute provides a stable selector for E2E tests to target the primary trade action button. The multi-line formatting of the ButtonError props also improves readability.apps/cowswap-frontend/src/serviceWorkerRegistration.ts (1)
26-26: LGTM: Explicit void return types improve type clarity.Adding explicit
: voidreturn types toregisterValidSW,checkValidServiceWorker,register, andunregisteris good TypeScript practice for self-documenting code.apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
42-47: LGTM with TODOs noted.The removal of
max-lines-per-functionfrom the eslint-disable list is appropriate. The TODO comments acknowledge the technical debt regarding function size and complexity, which could be addressed in a future refactor.apps/cowswap-frontend-e2e/src/e2e/lists.test.ts (1)
6-11: LGTM! Test successfully re-enabled.The test has been properly re-enabled with the addition of
cy.unlockCrossChainSwap()as a prerequisite step, which is consistent with the pattern used across other test files in this PR.apps/cowswap-frontend-e2e/src/e2e/search.test.ts (2)
1-3: LGTM! Good addition of explicit return type.Adding the explicit
: voidreturn type annotation improves type safety and code clarity.
10-86: LGTM! Test suite successfully re-enabled.The Search test suite has been properly re-enabled with consistent addition of
cy.unlockCrossChainSwap()across all nine tests. This ensures the prerequisite cross-chain swap flow is unlocked before token search interactions.apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts (2)
1-7: LGTM! Excellent addition of explicit return types.Adding explicit
Cypress.Chainablereturn type annotations togetInputFiatAmountandgetInputTokenimproves type safety and makes the code more maintainable.
18-38: LGTM! Test successfully re-enabled.The fiat amounts test has been properly re-enabled with the addition of
cy.unlockCrossChainSwap()as a prerequisite step, consistent with the pattern used across other test files.apps/cowswap-frontend-e2e/src/support/commands.ts (2)
69-74: LGTM! Well-implemented new Cypress command.The
unlockCrossChainSwapcommand is properly implemented with:
- Clear JSDoc documentation
- Explicit return type (
Cypress.Chainable)- Straightforward implementation (select element, verify visibility, click)
- Proper command registration
This command is consistently used across all re-enabled test files in the PR.
Also applies to: 185-187, 214-214
107-114: Selector change is correct and aligns with the DOM structure.The token list items in the frontend are rendered as
styled.divelements (not buttons), so changing the e2e test selector frombutton[data-address="..."]todiv[data-address="..."]is necessary and correct. The styled div component includes button-like styling (cursor pointer, hover effects, click handling) but is fundamentally a div element.apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts (5)
1-2: LGTM!Clean import of mock helpers that are used in the test at line 81-87.
6-9: Placeholder test is acceptable.Based on learnings, using trivial placeholder tests in e2e files is an intentional pattern in this codebase to keep CI green while tests are being fixed.
11-19: LGTM!Test properly initializes state by visiting the swap page and unlocking cross-chain swap before assertions. The updated expectation for "Select a token" placeholder aligns with the new token-picker flow.
78-96: LGTM - well-structured mock setup.The async
onBeforeLoadcorrectly awaitssigner.getAddress()before setting up the mock. The mock simulates 50 ETH balance which is sufficient for the 0.5 ETH swap test. The updated#do-trade-buttonselector is consistent with the broader UI changes in this PR.
104-115: LGTM!Recipient tests properly initialize state in
beforeEachand verify expert mode behavior.apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (4)
5-10: LGTM!Clean import of all required mock helpers used throughout the test file.
16-24: LGTM!The helper correctly uses the new
#do-trade-buttonselector and has an explicitvoidreturn type annotation.
33-58: LGTM - comprehensive mock setup.Mocking both token balance and allowance ensures the swap flow can proceed without approval prompts. The mock values (5 WETH balance and allowance) appropriately exceed the test swap amount (0.5 WETH).
107-141: LGTM!URL param tests correctly call
cy.unlockCrossChainSwap()after navigation to ensure proper test state initialization.apps/cowswap-frontend-e2e/src/e2e/limit-orders.test.ts (3)
5-23: LGTM!Good addition of explicit return type annotations (
voidfor side-effect functions,Cypress.Chainablefor query functions). This improves type safety and code clarity.
25-27: LGTM!Enabling the test suite while keeping the specific broken test skipped with a TODO comment is a pragmatic approach. This allows other tests to run while the modal test is fixed separately.
44-75: LGTM!URL params tests are now enabled and provide good coverage for input/output amount handling via URL parameters.
apps/cowswap-frontend-e2e/src/support/e2e.ts (4)
17-18: LGTM!Clean import of Cypress net-stubbing types for proper typing of cached responses.
50-56: LGTM - service worker disabling.Deleting from
__proto__is an unconventional but effective approach to disable service workers before they can register. The@ts-expect-errorcomment appropriately acknowledges this.
75-83: Previous regex issue has been addressed.The patterns now correctly use
.*instead of*for matching any characters (e.g.,/files.cow.fi\/token-lists\/.*.json/).
99-114: LGTM - smart caching strategy.The in-memory cache for static resources reduces external network calls during tests. Stripping original headers before caching avoids
req.replycompatibility issues.apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts (5)
1-10: LGTM - imports and ESLint configuration.The direct import from
libs/multicall/src/utils/getMulticallContractis intentional per past discussion to avoid slow bundle times from the barrel file.
12-51: Well-structured argument parsing.
parseSendArgscorrectly handles both the callback form (send({method, params}, callback)) and the promise form (send(method, params)), providing unified access patterns viagetOriginalResultandreturnResult.
56-102: LGTM - eth_call and multicall handling.The middleware correctly intercepts both direct
eth_callrequests and multicall3tryAggregatecalls, patching the relevant results while preserving other call results. ThestructuredCloneon line 89 ensures the decoded tuple is safely mutable.
112-135: LGTM - native balance handling.The previous review issue about incorrect parameter checking for
eth_getBalancehas been addressed. Line 118 now correctly checksparams?.[0]?.toLowerCase()instead ofparams?.[0]?.to?.toLowerCase().
141-152: LGTM - clean middleware orchestration.The
mockSendCallfunction correctly chains middlewares, falling back to the originalsendwhen no middleware handles the call.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
14-14: Add explicit assertion for better error messages.Unlike
INTEGRATION_TEST_PRIVATE_KEY(line 12),INTEGRATION_TESTS_ALCHEMY_KEYlacks an explicit assertion. If undefined, the template literal on line 18 creates a URL containing"undefined", which passes the truthy check on line 20 but results in cryptic network errors during test execution.🔎 Proposed fix
const INTEGRATION_TESTS_ALCHEMY_KEY = Cypress.env('INTEGRATION_TESTS_ALCHEMY_KEY') +assert(INTEGRATION_TESTS_ALCHEMY_KEY, 'INTEGRATION_TESTS_ALCHEMY_KEY env missing')
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.github/workflows/ci.ymlREADME.mdapps/cowswap-frontend-e2e/src/support/ethereum.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend-e2e/src/support/ethereum.ts
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend-e2e/src/support/ethereum.ts
🪛 dotenv-linter (4.0.0)
.env
[warning] 8-8: [UnorderedKey] The CYPRESS_INTEGRATION_TESTS_ALCHEMY_KEY key should go before the CYPRESS_INTEGRATION_TEST_PRIVATE_KEY key
(UnorderedKey)
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (2)
.github/workflows/ci.yml (1)
203-203: LGTM! Clean migration to Alchemy.The secret name change from
CYPRESS_INTEGRATION_TESTS_INFURA_KEYtoCYPRESS_INTEGRATION_TESTS_ALCHEMY_KEYis consistent with the provider migration across the codebase.apps/cowswap-frontend-e2e/src/support/ethereum.ts (1)
18-18: LGTM! Clean Alchemy migration and EIP-1193 compatibility improvements.The changes are well-executed:
- Provider URL correctly updated to Alchemy
- Error message reflects the new key name
TEST_ADDRESS_NEVER_USEexport has clear naming for test-only usagesendAsyncmethod properly delegates tosendfor EIP-1193 compatibilityAlso applies to: 22-22, 26-26, 38-41
|
I made a new branch so the CI tests can run (secrets cannot be passed to external prs) A couple of tests are failing in the first run. 
Nontheless, let's merge it and continue with #6789 |
|
Update: a green CI has been achieved https://github.com/cowprotocol/cowswap/actions/runs/20720174721/job/59481506508 Diff between this branch and the other: https://github.com/cowprotocol/cowswap/pull/6789/changes/019fa9367e936a2717e28e626b5b6adc70a4a7f9..cf103847168634e8279f3f3a720712355211941e After trying to make it work, I tend to agree with @crutch12:
|
3 participants
Summary
Tests have been disabled for a long time because of fails, so I've decided to bring them back alive.
Main changes
mockSendCallto mock user'sbalanceandallowanceTo Test
yarn e2eshould work and successyarn e2e:openshould work (idk why, but it doesn't work with my Google Chrome, so I use Ms Edge)Since dev server doesn't work too fast, I've tested it only for production build.
Background
Offtop
I think Cypress is not the best e2e framework 🫠Summary by CodeRabbit
Tests
Documentation
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.