Viem Migration - e2e tests missing changes from viem migration#7336
Open
Viem Migration - e2e tests missing changes from viem migration#7336
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
WalkthroughAdd network-level JSON-RPC mocking for E2E, migrate tests to use RPC mocks, seed wagmi connector state in test setup, refine Ethereum bridge request handling, adjust Cypress response handling, and add a Wagmi adapter test mock and Jest mapping. Changes
sequenceDiagram
participant Test as E2E Test
participant Intercept as RPC Interceptor (cy.intercept)
participant Patches as Patch Registry
participant Server as RPC Server
Test->>Patches: setupRpcMocks()
Patches-->>Test: {mockNativeBalance, mockTokenBalance, mockTokenAllowance}
Test->>Patches: mockNativeBalance(owner, value)
Patches->>Patches: register patch
Test->>Server: app triggers RPC POST (eth_getBalance / eth_call)
Server->>Intercept: network request reaches intercept
Intercept->>Intercept: parse JSON-RPC payload
rect rgba(100, 150, 200, 0.5)
note right of Intercept: Handle eth_getBalance
Intercept->>Patches: lookup nativeBalance by address
Patches-->>Intercept: found -> toHex(value)
Intercept-->>Test: reply with patched JSON-RPC result
end
rect rgba(150, 100, 200, 0.5)
note right of Intercept: Handle eth_call (Multicall3: aggregate3)
Intercept->>Intercept: decode multicall payload
Intercept->>Patches: inspect inner calls for matches
Patches-->>Intercept: return per-call patch data
Intercept->>Server: forward original request
Server-->>Intercept: actual aggregate3 response
Intercept->>Intercept: decode response, replace results with patches
Intercept-->>Test: reply with mutated response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
Author
|
There were some DNS connection issues on the e2e tests, so I stopped the development to wait until DNS bug is fixed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/cowswap-frontend-e2e/src/support/mocks/mockRpcCall.ts (1)
61-64: Please use the shared address comparison helper here.The new matcher layer hand-rolls address normalization in several spots. Reusing the repo helper keeps the mock behavior aligned with the rest of the codebase and avoids more one-off lowercasing rules. As per coding guidelines "Never compare addresses with
===,toLowerCase(), or manual string comparison; always useareAddressesEqualfrom@cowprotocol/cow-sdk".Also applies to: 84-86, 123-125, 139-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cowswap-frontend-e2e/src/support/mocks/mockRpcCall.ts` around lines 61 - 64, Replace manual address comparisons and lowercasing with the shared helper areAddressesEqual from `@cowprotocol/cow-sdk`: use areAddressesEqual(call.target, MULTICALL3_ADDRESS) instead of call.target.toLowerCase() !== MULTICALL3_ADDRESS, and use areAddressesEqual(p.owner, args[0]) instead of p.owner.toLowerCase() === args[0].toLowerCase(); apply the same replacement for the other comparisons noted (the comparisons around lines 84-86, 123-125, and 139) so any checks against MULTICALL3_ADDRESS, p.owner, call.from, or similar address variables use areAddressesEqual consistently; import areAddressesEqual at the top of the file and remove manual toLowerCase()/string comparisons.apps/cowswap-frontend-e2e/src/support/e2e.ts (1)
22-33: Please reuse the wallet storage prefix instead of hardcoding it here.This prefix has to stay byte-for-byte aligned with the frontend wallet storage config. If it drifts,
seedWagmiConnectionState()silently stops auto-connecting the injected connector across the suite. As per coding guidelines "Use shared config/enums instead of hardcoding environment-specific lists/toggles".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cowswap-frontend-e2e/src/support/e2e.ts` around lines 22 - 33, Currently the file defines a local hardcoded WAGMI_STORAGE_KEY and uses it in seedWagmiConnectionState, which can drift from the frontend config; remove the local constant and instead import and reuse the shared wallet storage prefix constant exported by the frontend wallet config (the same constant used by the app's wagmi storage setup) and use that imported symbol in seedWagmiConnectionState when calling storage.setItem(`${<IMPORTED_PREFIX>}.injected.connected`, 'true') and storage.setItem(`${<IMPORTED_PREFIX>}.recentConnectorId`, '"injected"'); ensure you reference the exact exported name from the shared config and update any imports accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cowswap-frontend-e2e/src/support/commands.ts`:
- Around line 121-129: The current closure uses a truthy check on the outer
variable "body" to decide stubbing, which treats valid JSON payloads like false,
0, '' or null as "no stub"; update the condition in the returned function (the
closure that calls req.reply) to explicitly check for undefined (e.g. if (body
!== undefined) ...) so any provided body, including false/0/''/null, will be
used for req.reply instead of falling through to the real server. Ensure the
branch still stringifies the body and sets headers as before.
In `@apps/cowswap-frontend-e2e/src/support/mocks/mockRpcCall.ts`:
- Around line 18-20: The RPC intercept matcher in mockRpcCall.ts currently uses
a hardcoded RPC_URL_PATTERN which prevents mocks from firing for custom RPCs;
update the matcher to derive allowed hosts from the app/provider configuration
(the same source used by apps/cowswap-frontend-e2e/src/support/ethereum.ts and
REACT_APP_NETWORK_URL_11155111) or accept the provider config/share the provider
instance so the intercept logic (where MULTICALL3_ADDRESS and the RPC matcher
are used) matches whatever RPC URL is configured at runtime instead of the
hardcoded infura/alchemy/publicnode/sepolia list.
In `@apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts`:
- Around line 176-202: The recursion guard uses a single boolean isInsideSend
which is unsafe for overlapping RPCs; replace it with a numeric depth counter
(e.g., sendDepth = 0) and in the cy.stub(ethereum, 'send') fake increment
sendDepth at the start and decrement it in the finally block, and update the
request stub to check sendDepth > 0 instead of isInsideSend; adjust references
to isInsideSend, originalSend and originalRequest so send increments/decrements
always occur even on errors and the request -> send -> request recursion is
prevented per-call rather than globally.
---
Nitpick comments:
In `@apps/cowswap-frontend-e2e/src/support/e2e.ts`:
- Around line 22-33: Currently the file defines a local hardcoded
WAGMI_STORAGE_KEY and uses it in seedWagmiConnectionState, which can drift from
the frontend config; remove the local constant and instead import and reuse the
shared wallet storage prefix constant exported by the frontend wallet config
(the same constant used by the app's wagmi storage setup) and use that imported
symbol in seedWagmiConnectionState when calling
storage.setItem(`${<IMPORTED_PREFIX>}.injected.connected`, 'true') and
storage.setItem(`${<IMPORTED_PREFIX>}.recentConnectorId`, '"injected"'); ensure
you reference the exact exported name from the shared config and update any
imports accordingly.
In `@apps/cowswap-frontend-e2e/src/support/mocks/mockRpcCall.ts`:
- Around line 61-64: Replace manual address comparisons and lowercasing with the
shared helper areAddressesEqual from `@cowprotocol/cow-sdk`: use
areAddressesEqual(call.target, MULTICALL3_ADDRESS) instead of
call.target.toLowerCase() !== MULTICALL3_ADDRESS, and use
areAddressesEqual(p.owner, args[0]) instead of p.owner.toLowerCase() ===
args[0].toLowerCase(); apply the same replacement for the other comparisons
noted (the comparisons around lines 84-86, 123-125, and 139) so any checks
against MULTICALL3_ADDRESS, p.owner, call.from, or similar address variables use
areAddressesEqual consistently; import areAddressesEqual at the top of the file
and remove manual toLowerCase()/string comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a2ea75ba-0142-4c4a-a9b2-ff05d5f3b10c
📒 Files selected for processing (8)
apps/cowswap-frontend-e2e/src/e2e/fee.test.tsapps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.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/mockRpcCall.tsapps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts
Comment on lines
+121
to
+129
| return (req: any) => { | ||
| if (body) { | ||
| // Fully stub the response without forwarding to the real server | ||
| req.reply({ | ||
| statusCode: 200, | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: JSON.stringify(body), | ||
| }) | ||
| } else { |
Contributor
There was a problem hiding this comment.
Don't use truthiness to decide whether a stub body exists.
stubResponse({ body: false | 0 | '' | null }) now falls through to the real server because if (body) treats valid JSON payloads as "no stub". If undefined is the passthrough signal, check that explicitly.
Suggested fix
- if (body) {
+ if (typeof body !== 'undefined') {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return (req: any) => { | |
| if (body) { | |
| // Fully stub the response without forwarding to the real server | |
| req.reply({ | |
| statusCode: 200, | |
| headers: { 'content-type': 'application/json' }, | |
| body: JSON.stringify(body), | |
| }) | |
| } else { | |
| return (req: any) => { | |
| if (typeof body !== 'undefined') { | |
| // Fully stub the response without forwarding to the real server | |
| req.reply({ | |
| statusCode: 200, | |
| headers: { 'content-type': 'application/json' }, | |
| body: JSON.stringify(body), | |
| }) | |
| } else { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cowswap-frontend-e2e/src/support/commands.ts` around lines 121 - 129,
The current closure uses a truthy check on the outer variable "body" to decide
stubbing, which treats valid JSON payloads like false, 0, '' or null as "no
stub"; update the condition in the returned function (the closure that calls
req.reply) to explicitly check for undefined (e.g. if (body !== undefined) ...)
so any provided body, including false/0/''/null, will be used for req.reply
instead of falling through to the real server. Ensure the branch still
stringifies the body and sets headers as before.
Comment on lines
+18
to
+20
| const MULTICALL3_ADDRESS = '0xca11bde05977b3631167028862be2a173976ca11' | ||
| const RPC_URL_PATTERN = /infura\.io|1rpc\.io|alchemy\.com|publicnode\.com|sepolia/ | ||
|
|
Contributor
There was a problem hiding this comment.
The RPC intercept won't fire on custom network URLs.
apps/cowswap-frontend-e2e/src/support/ethereum.ts already supports REACT_APP_NETWORK_URL_11155111, but this matcher only recognizes a small host allowlist plus a loose sepolia substring. When tests run against a custom RPC host, the new balance/allowance mocks silently stop applying and fall back to live chain state. Please derive the intercept matcher from the configured RPC URL (or share the provider config) instead of hardcoding hosts. As per coding guidelines "Use shared config/enums instead of hardcoding environment-specific lists/toggles".
Also applies to: 176-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cowswap-frontend-e2e/src/support/mocks/mockRpcCall.ts` around lines 18 -
20, The RPC intercept matcher in mockRpcCall.ts currently uses a hardcoded
RPC_URL_PATTERN which prevents mocks from firing for custom RPCs; update the
matcher to derive allowed hosts from the app/provider configuration (the same
source used by apps/cowswap-frontend-e2e/src/support/ethereum.ts and
REACT_APP_NETWORK_URL_11155111) or accept the provider config/share the provider
instance so the intercept logic (where MULTICALL3_ADDRESS and the RPC matcher
are used) matches whatever RPC URL is configured at runtime instead of the
hardcoded infura/alchemy/publicnode/sepolia list.
Comment on lines
+176
to
+202
| // Flag to prevent infinite recursion: request -> send -> request | ||
| let isInsideSend = false | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| cy.stub(ethereum, 'send').callsFake(async (...args: any[]) => { | ||
| isInsideSend = true | ||
| try { | ||
| for (const middleware of middlewares) { | ||
| const handledResult = await middleware(...args) | ||
| if (typeof handledResult === 'undefined') continue | ||
| return handledResult | ||
| } | ||
| return await (originalSend as Function)(...args) | ||
| } finally { | ||
| isInsideSend = false | ||
| } | ||
| }) | ||
|
|
||
| cy.stub(ethereum, 'send').callsFake(async (...args) => { | ||
| for (const middleware of middlewares) { | ||
| const handledResult = await middleware(...args) | ||
| if (typeof handledResult === 'undefined') continue | ||
| return handledResult | ||
| // Also stub `request` — wagmi/viem call request() directly, not send(). | ||
| // Delegate to the (already-stubbed) send so middleware interception works. | ||
| cy.stub(ethereum, 'request').callsFake(async (reqObj: { method: string; params?: unknown[] }) => { | ||
| // If called from within a send middleware (e.g. getOriginalResult), bypass to avoid recursion | ||
| if (isInsideSend) { | ||
| return originalRequest(reqObj) | ||
| } | ||
| return send(...args) | ||
| // Use the stubbed send in promise form so middlewares can intercept | ||
| return ethereum.send(reqObj.method, reqObj.params) |
Contributor
There was a problem hiding this comment.
Use a depth counter for the recursion guard.
isInsideSend is shared across every in-flight RPC call. If two send() calls overlap, the first finally can flip it back to false while the second is still inside originalSend(), which reopens the request -> send -> request recursion path and makes this stub flaky under concurrent RPCs.
Suggested fix
- let isInsideSend = false
+ let sendDepth = 0
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
cy.stub(ethereum, 'send').callsFake(async (...args: any[]) => {
- isInsideSend = true
+ sendDepth += 1
try {
for (const middleware of middlewares) {
const handledResult = await middleware(...args)
if (typeof handledResult === 'undefined') continue
return handledResult
}
return await (originalSend as Function)(...args)
} finally {
- isInsideSend = false
+ sendDepth -= 1
}
})
cy.stub(ethereum, 'request').callsFake(async (reqObj: { method: string; params?: unknown[] }) => {
- if (isInsideSend) {
+ if (sendDepth > 0) {
return originalRequest(reqObj)
}
return ethereum.send(reqObj.method, reqObj.params)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cowswap-frontend-e2e/src/support/mocks/mockSendCall.ts` around lines 176
- 202, The recursion guard uses a single boolean isInsideSend which is unsafe
for overlapping RPCs; replace it with a numeric depth counter (e.g., sendDepth =
0) and in the cy.stub(ethereum, 'send') fake increment sendDepth at the start
and decrement it in the finally block, and update the request stub to check
sendDepth > 0 instead of isInsideSend; adjust references to isInsideSend,
originalSend and originalRequest so send increments/decrements always occur even
on errors and the request -> send -> request recursion is prevented per-call
rather than globally.
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@testing/reownAdapterMock.ts`:
- Around line 4-13: The module-level mockWagmiConfig causes shared state across
tests; instead, construct a fresh config inside the WagmiAdapter constructor.
Remove or stop referencing the top-level mockWagmiConfig, and in the
WagmiAdapter constructor assign this.wagmiConfig = createConfig({ chains:
[mainnet], transports: { [mainnet.id]: http() } }) so each WagmiAdapter instance
gets its own config for test isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1ac5a88e-a474-4fa0-9704-34ad3efbc2c6
📒 Files selected for processing (2)
apps/cowswap-frontend/jest.config.mjstesting/reownAdapterMock.ts
Comment on lines
+4
to
+13
| const mockWagmiConfig = createConfig({ | ||
| chains: [mainnet], | ||
| transports: { [mainnet.id]: http() }, | ||
| }) | ||
|
|
||
| export class WagmiAdapter { | ||
| wagmiConfig = mockWagmiConfig | ||
|
|
||
| constructor() {} | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether WagmiAdapter/wagmiConfig is instantiated or mutated in multiple test contexts.
rg -nP -C3 'new\s+WagmiAdapter|wagmiConfig\b|\.setState\s*\(|connect\(|disconnect\(' apps/cowswap-frontend testingRepository: cowprotocol/cowswap
Length of output: 6732
🏁 Script executed:
# Search for imports of the mock file and WagmiAdapter usage
rg -n 'from.*reownAdapterMock|import.*WagmiAdapter|testing/reownAdapterMock' apps/cowswap-frontend testing --type ts --type tsxRepository: cowprotocol/cowswap
Length of output: 90
🏁 Script executed:
# Also check if WagmiAdapter is exported and used elsewhere
rg -n 'WagmiAdapter' . --type ts --type tsx | grep -v node_modules | head -20Repository: cowprotocol/cowswap
Length of output: 90
🏁 Script executed:
# Search for imports of the mock file
rg -n 'reownAdapterMock|WagmiAdapter' . | grep -v node_modules | head -30Repository: cowprotocol/cowswap
Length of output: 559
🏁 Script executed:
# Check the actual file structure and size
wc -l testing/reownAdapterMock.ts
cat -n testing/reownAdapterMock.tsRepository: cowprotocol/cowswap
Length of output: 453
🏁 Script executed:
# Check the real usage in libs/wallet/src/wagmi/config.ts
cat -n libs/wallet/src/wagmi/config.tsRepository: cowprotocol/cowswap
Length of output: 3824
🏁 Script executed:
# Search for test files that might be using the mock
rg -l '\.test\.|\.spec\.' testing apps/cowswap-frontend | head -10Repository: cowprotocol/cowswap
Length of output: 211
🏁 Script executed:
# Check if there are any tests that create multiple WagmiAdapter instances
rg -n 'WagmiAdapter|reownAdapterMock' --type-add 'test:={*.test.*,*.spec.*}' -t test . | grep -v node_modulesRepository: cowprotocol/cowswap
Length of output: 45
🏁 Script executed:
# Search for tests that import wagmiAdapter or wagmi config
rg -n 'from.*wagmi/config|from.*reownAdapterMock|import.*wagmiAdapter' apps/cowswap-frontend testing | head -20Repository: cowprotocol/cowswap
Length of output: 45
🏁 Script executed:
# Check if there are any test setup files that might instantiate multiple configs
rg -n 'beforeEach|setUp|new WagmiAdapter' apps/cowswap-frontend/jest.setup --type-add 'setup:=*.setup.*' -t setup -A 5Repository: cowprotocol/cowswap
Length of output: 135
🏁 Script executed:
# Look at how tests actually use wagmi - search for walletConnectorMock or similar setup patterns
find testing apps/cowswap-frontend -name '*setup*' -o -name '*mock*' | grep -E '\.(ts|js)$' | head -10Repository: cowprotocol/cowswap
Length of output: 428
🏁 Script executed:
# Check the jest setup file
cat -n apps/cowswap-frontend/jest.setup.tsRepository: cowprotocol/cowswap
Length of output: 2721
🏁 Script executed:
# Search for any render wrapper patterns or test wrappers that might create configs
rg -n 'wrapper|Provider|createConfig' apps/cowswap-frontend --type-add 'test:={*.test.*,*.spec.*}' | grep -i wagmiRepository: cowprotocol/cowswap
Length of output: 218
🏁 Script executed:
# Check if wagmi's createConfig is stateful - look at wagmi package in node_modules or imports
rg -n 'wagmiConfig\.' testing/reownAdapterMock.ts apps/cowswap-frontendRepository: cowprotocol/cowswap
Length of output: 45
🏁 Script executed:
# Search for any test that might instantiate WagmiAdapter multiple times in a single test file
rg -n 'new WagmiAdapter|WagmiAdapter\(' . --type-add 'test:={*.test.*,*.spec.*}' | grep -v node_modulesRepository: cowprotocol/cowswap
Length of output: 146
🏁 Script executed:
# Check jest.config.mjs to understand the mock setup better
cat -n apps/cowswap-frontend/jest.config.mjsRepository: cowprotocol/cowswap
Length of output: 1270
Create a fresh wagmi config per adapter instance to ensure test isolation.
The module-level mockWagmiConfig is shared across all test files that instantiate WagmiAdapter, risking state leakage between tests. Per the codebase's test isolation preference, create the config in the constructor instead.
🔧 Suggested change
-const mockWagmiConfig = createConfig({
- chains: [mainnet],
- transports: { [mainnet.id]: http() },
-})
-
export class WagmiAdapter {
- wagmiConfig = mockWagmiConfig
-
- constructor() {}
+ public readonly wagmiConfig = createConfig({
+ chains: [mainnet],
+ transports: { [mainnet.id]: http() },
+ })
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@testing/reownAdapterMock.ts` around lines 4 - 13, The module-level
mockWagmiConfig causes shared state across tests; instead, construct a fresh
config inside the WagmiAdapter constructor. Remove or stop referencing the
top-level mockWagmiConfig, and in the WagmiAdapter constructor assign
this.wagmiConfig = createConfig({ chains: [mainnet], transports: { [mainnet.id]:
http() } }) so each WagmiAdapter instance gets its own config for test
isolation.
…ute redirect; fix: add @reown/appkit-adapter-wagmi Jest mock to resolve 26 failing test suites;
lgahdl
force-pushed
the
bruno/cow-890-fix-e2e-tests-for-viem-migration
branch
from
2f6a72f to
39693e1
Compare
Collaborator
|
I have read the CLA Document and I hereby sign the CLA |
…-bruno/cow-890-fix-e2e-tests-for-viem-migration # Conflicts: # apps/cowswap-frontend-e2e/src/e2e/swap.test.ts
lgahdl
changed the title
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (1)
68-85:⚠️ Potential issue | 🟠 MajorFix the false-positive quote assertion.
Line 80 uses
should('not.equal', '')on the yielded DOM element, not on its input value, so this assertion always passes regardless of whether the output amount is populated. Replace it with the idiomatic Cypress assertion to check the input's actual value:Suggested fix
- cy.get('#output-currency-input .token-amount-input').should('not.equal', '') + cy.get('#output-currency-input .token-amount-input').should('not.have.value', '')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cowswap-frontend-e2e/src/e2e/swap.test.ts` around lines 68 - 85, The test assertion on the output input is checking the yielded DOM element instead of its value, causing a false positive; locate the test "Swap ETH for USDC - shows optional Switch to WETH" and replace the line using cy.get('#output-currency-input .token-amount-input').should('not.equal', '') with the correct Cypress input-value assertion (e.g., use .should('not.have.value', '') or invoke('val').should('not.equal', '')) so the check verifies the input's actual value before proceeding to acceptFeesExceedWarning() and waitForSwapAction().
🧹 Nitpick comments (1)
apps/cowswap-frontend-e2e/src/e2e/swap.test.ts (1)
83-85: Avoid pinning this E2E to the full banner copy.The banner selector already proves the ETH-specific path rendered. Asserting the whole sentence will make this fail on copy-only or i18n changes without any behavior regression. Prefer existence of the banner, or a stable CTA inside it, over the exact marketing text.
Suggested simplification
- cy.get('#classic-eth-flow-banner') - .should('exist') - .should('contain.text', 'Switch to the classic WETH experience and benefit!') + cy.get('#classic-eth-flow-banner').should('exist')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cowswap-frontend-e2e/src/e2e/swap.test.ts` around lines 83 - 85, The test currently asserts the full marketing copy for the banner using cy.get('#classic-eth-flow-banner').should('contain.text', ...), which makes the E2E brittle to copy/i18n changes; change the assertion to only verify the banner exists or a stable internal element (e.g., a CTA button) exists. Locate the check in swap.test.ts referencing '#classic-eth-flow-banner' and replace the .should('contain.text', 'Switch to the classic WETH experience and benefit!') with either .should('exist') alone or a stable selector assertion such as .find('button[data-testid="classic-eth-cta"]').should('exist') (or similar stable CTA selector used in the component).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/cowswap-frontend-e2e/src/e2e/swap.test.ts`:
- Around line 68-85: The test assertion on the output input is checking the
yielded DOM element instead of its value, causing a false positive; locate the
test "Swap ETH for USDC - shows optional Switch to WETH" and replace the line
using cy.get('#output-currency-input .token-amount-input').should('not.equal',
'') with the correct Cypress input-value assertion (e.g., use
.should('not.have.value', '') or invoke('val').should('not.equal', '')) so the
check verifies the input's actual value before proceeding to
acceptFeesExceedWarning() and waitForSwapAction().
---
Nitpick comments:
In `@apps/cowswap-frontend-e2e/src/e2e/swap.test.ts`:
- Around line 83-85: The test currently asserts the full marketing copy for the
banner using cy.get('#classic-eth-flow-banner').should('contain.text', ...),
which makes the E2E brittle to copy/i18n changes; change the assertion to only
verify the banner exists or a stable internal element (e.g., a CTA button)
exists. Locate the check in swap.test.ts referencing '#classic-eth-flow-banner'
and replace the .should('contain.text', 'Switch to the classic WETH experience
and benefit!') with either .should('exist') alone or a stable selector assertion
such as .find('button[data-testid="classic-eth-cta"]').should('exist') (or
similar stable CTA selector used in the component).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df109248-c9f2-45fc-be45-4cd58c6ca8f2
📒 Files selected for processing (2)
apps/cowswap-frontend-e2e/src/e2e/swap.test.tsapps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend-e2e/src/e2e/swapMod.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #890
Resolves 26 failing test suites caused by the Viem/Wagmi/Reown migration. Jest could not resolve
@reown/appkit-adapter-wagmibecause it was missing frommoduleNameMapperinjest.config.mjs.Added a
WagmiAdaptermock that returns a validwagmiConfigbuilt via wagmi'screateConfig, which is required byWagmiProviderin the test wrappers (WithMockedWeb3,Web3Provider).To Test
pnpm testpasses with 115/115 suites and 931/931 tests (previously 26 suites were failing)Background
After the Viem/Wagmi/Reown migration,
libs/wallet/src/wagmi/config.tsintroduced an import from@reown/appkit-adapter-wagmi. This package was already handled intransformIgnorePatternsbut was never added tomoduleNameMapper, so Jest could not resolve it at all — causing every test suite that transitively imported from@cowprotocol/walletto fail withCannot find module '@reown/appkit-adapter-wagmi'.The mock exports a
WagmiAdapterclass with a minimal but validwagmiConfig(usingcreateConfigfrom wagmi), matching the shape expected byWagmiProviderin the test environment.