-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
intercept and mock e2e swaps calls #1754
base: master
Are you sure you want to change the base?
Conversation
|
||
const url = new URL(input); | ||
|
||
if (url.hostname === 'swap.p.rainbow.me') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intercepts requests to swap.p.rainbow.me
and responds with our mocked responses
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
@@ -35,7 +36,6 @@ let driver: WebDriver; | |||
|
|||
const browser = process.env.BROWSER || 'chrome'; | |||
const os = process.env.OS || 'mac'; | |||
const isFirefox = browser === 'firefox'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we no longer run FF tests
package.json
Outdated
@@ -53,7 +56,7 @@ | |||
"// Build and zip": "", | |||
"bundle": "yarn build && yarn zip", | |||
"// Runs tests": "", | |||
"anvil": "ETH_MAINNET_RPC=$(grep ETH_MAINNET_RPC .env | cut -d '=' -f2) && anvil --fork-url $ETH_MAINNET_RPC", | |||
"anvil": "ETH_MAINNET_RPC=$(grep ETH_MAINNET_RPC .env | cut -d '=' -f2) && anvil --fork-url $ETH_MAINNET_RPC --fork-block-number 21065950", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forked to block number from quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Biased approve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. I do have once concern that I'd live to see resolved or at least to have a plan before we decide to merge this.
Once this lands we're making all the tests 100% depending on hardhat chain id.
This takes away from the next goal which is to be able to run swaps e2e on l2.
Ideally hardhat should have the same chain id as the chain that is being forked, which makes everything else easier: not having to change test ids, not having to touch unit tests and keeping the ground work for supporting multichain mocked e2e in the future.
Can we try to do that?
Added back the chainIds to the testIds, and also removed the reliance on chainId `1337` (hardhat) from unit tests. this was a step back to the larger goal of e2e L2 support
@@ -0,0 +1,66 @@ | |||
/* eslint-disable @typescript-eslint/no-var-requires */ | |||
|
|||
// this file is ran locally with ts-node to fetch the responses from the swap quotes urls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran locally to get swap_quotes outputs
@@ -0,0 +1,169 @@ | |||
[ | |||
"https://swap.p.rainbow.me/v1/quote?bridgeVersion=4&buyToken=0xff970a61a04b1ca14834a43f5de4533ebddb5cc8&chainId=1¤cy=USD&fromAddress=0x3c44cdddb6a900fa2b585dd299e03d12fa4293bc&refuel=false&sellAmount=1000000000000000000&sellToken=0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE&slippage=5&toChainId=42161", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are all the requests we send out during e2e
@@ -38,7 +38,7 @@ export const getAssetRawAllowance = async ({ | |||
chainId: ChainId; | |||
}) => { | |||
try { | |||
const provider = await getProvider({ chainId }); | |||
const provider = getProvider({ chainId }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary await
@@ -13,6 +13,9 @@ require('../../core/utils/lockdown'); | |||
initThemingLocal(); | |||
syncStores(); | |||
|
|||
if (process.env.IS_TESTING === 'true') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is where we set up the mock fetch to intercept requests
Fixes APP-1868
This PR sets up mocking for swaps.
Changes:
mockFetch.ts
) to intercept calls to our swaps endpoint and then return hashed responses stored ine2e/mocks/swap_quotes
.20923142
src/entries/popup/index.ts