From 3552a39d58604031caa82132bf22360348863ae0 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Fri, 21 Jun 2024 23:00:45 +0200 Subject: [PATCH] fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` (#24898) This PR fixes some flakiness of the test `Settings Redirects to ENS domains when user inputs ENS into address bar`. After looking into the code I've identified a source of flakiness, as well as a problem that prevents to upload the artifacts (screenshot) if the test fail. - The test was not using any mock: this is a source of flakiness, and it's now fixed by adding the necessary mocks to the ENS contracts and Infura calls (we need to set Mainnet, since this feature is only available on Mainnet) - The test was not using fixtures: this per se is not a problem, but the issue here is that we are not triggering the `verboseReportOnFailure(title,error)` on error, making it harder to debug, as no artifacts are uploaded for this test, if it fails. Now we are going to be able to see the screenshot if the test ever fails again Circle ci job: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/83569/workflows/0883aa51-cd94-4066-9f96-77d1375e5e9b/jobs/2994904/tests Failure log: `TimeoutError: Wait timed out after 213ms` Extra note: @davidmurdoch is also working on mitigating the rest of the flakiness by waiting for the metamask to be fully initialized before proceeding with the test. This work will be done in a separate PR. --------- Co-authored-by: Dan J Miller Co-authored-by: Harika Jetpoluru <153644847+hjetpoluru@users.noreply.github.com> Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com> Co-authored-by: Howard Braham --- test/e2e/tests/ppom/mocks/json-rpc-result.ts | 4 + .../settings/ipfs-ens-resolution.spec.js | 200 ++++++++++++++++-- test/e2e/webdriver/chrome.js | 20 +- test/e2e/webdriver/firefox.js | 26 ++- test/e2e/webdriver/index.js | 13 +- 5 files changed, 226 insertions(+), 37 deletions(-) diff --git a/test/e2e/tests/ppom/mocks/json-rpc-result.ts b/test/e2e/tests/ppom/mocks/json-rpc-result.ts index 775e049e02b7..e42b35c0dfc8 100644 --- a/test/e2e/tests/ppom/mocks/json-rpc-result.ts +++ b/test/e2e/tests/ppom/mocks/json-rpc-result.ts @@ -9,6 +9,7 @@ export type mockJsonRpcResultType = { export const mockJsonRpcResult: mockJsonRpcResultType = { eth_blockNumber: { default: MOCK_BLOCK_NUMBER, + custom: '0x130E45F', }, eth_estimateGas: { @@ -387,4 +388,7 @@ export const mockJsonRpcResult: mockJsonRpcResultType = { BUSD_STORAGE_2: '0x000000000000000000000000137dcd97872de27a4d3bf36a4643c5e18fa40713', }, + net_version: { + default: '1', + }, }; diff --git a/test/e2e/tests/settings/ipfs-ens-resolution.spec.js b/test/e2e/tests/settings/ipfs-ens-resolution.spec.js index 02e65cd946b0..fe06449f1d47 100644 --- a/test/e2e/tests/settings/ipfs-ens-resolution.spec.js +++ b/test/e2e/tests/settings/ipfs-ens-resolution.spec.js @@ -1,6 +1,151 @@ -const { buildWebDriver } = require('../../webdriver'); -const { withFixtures, tinyDelayMs, unlockWallet } = require('../../helpers'); +const { + defaultGanacheOptions, + tinyDelayMs, + unlockWallet, + withFixtures, +} = require('../../helpers'); const FixtureBuilder = require('../../fixture-builder'); +const { mockServerJsonRpc } = require('../ppom/mocks/mock-server-json-rpc'); + +const BALANCE_CHECKER = '0xb1f8e55c7f64d203c1400b9d8555d050f94adf39'; +const ENS_PUBLIC_RESOLVER = '0x4976fb03c32e5b8cfe2b6ccb31c09ba78ebaba41'; +const ENS_REGISTRY_WITH_FALLBACK = '0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e'; + +async function mockInfura(mockServer) { + await mockServerJsonRpc(mockServer, [ + [ + 'eth_blockNumber', + { + methodResultVariant: 'custom', + }, + ], + ['eth_estimateGas'], + ['eth_feeHistory'], + ['eth_gasPrice'], + ['eth_getBalance'], + ['eth_getBlockByNumber'], + ['eth_getTransactionCount'], + ['net_version'], + ]); + + // Resolver Call + await mockServer + .forPost() + .withJsonBodyIncluding({ + method: 'eth_call', + params: [ + { + to: ENS_REGISTRY_WITH_FALLBACK, + }, + ], + }) + .thenCallback(async (req) => { + return { + statusCode: 200, + json: { + jsonrpc: '2.0', + id: (await req.body.getJson()).id, + result: + '0x0000000000000000000000004976fb03c32e5b8cfe2b6ccb31c09ba78ebaba41', + }, + }; + }); + + // Supports Interface Call + await mockServer + .forPost() + .withJsonBodyIncluding({ + method: 'eth_call', + params: [ + { + data: '0x01ffc9a7bc1c58d100000000000000000000000000000000000000000000000000000000', + to: ENS_PUBLIC_RESOLVER, + }, + ], + }) + .thenCallback(async (req) => { + return { + statusCode: 200, + json: { + jsonrpc: '2.0', + id: (await req.body.getJson()).id, + result: + '0x0000000000000000000000000000000000000000000000000000000000000001', + }, + }; + }); + + // Supports Interface Call + await mockServer + .forPost() + .withJsonBodyIncluding({ + method: 'eth_call', + params: [ + { + data: '0x01ffc9a7d8389dc500000000000000000000000000000000000000000000000000000000', + to: ENS_PUBLIC_RESOLVER, + }, + ], + }) + .thenCallback(async (req) => { + return { + statusCode: 200, + json: { + jsonrpc: '2.0', + id: (await req.body.getJson()).id, + result: + '0x0000000000000000000000000000000000000000000000000000000000000000', + }, + }; + }); + + // Content Hash Call + await mockServer + .forPost() + .withJsonBodyIncluding({ + method: 'eth_call', + params: [ + { + data: '0xbc1c58d1e650784434622eeb4ffbbb3220ebb371e26ad1a77f388680d42d8b1624baa6df', + to: ENS_PUBLIC_RESOLVER, + }, + ], + }) + .thenCallback(async (req) => { + return { + statusCode: 200, + json: { + jsonrpc: '2.0', + id: (await req.body.getJson()).id, + result: + '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000', + }, + }; + }); + + // Token Balance Call + await mockServer + .forPost() + .withJsonBodyIncluding({ + method: 'eth_call', + params: [ + { + to: BALANCE_CHECKER, + }, + ], + }) + .thenCallback(async (req) => { + return { + statusCode: 200, + json: { + jsonrpc: '2.0', + id: (await req.body.getJson()).id, + result: + '0x0000000000000000000000000000000000000000000000000000000000000000', + }, + }; + }); +} describe('Settings', function () { const ENS_NAME = 'metamask.eth'; @@ -10,25 +155,38 @@ describe('Settings', function () { it('Redirects to ENS domains when user inputs ENS into address bar', async function () { // Using proxy port that doesn't resolve so that the browser can error out properly // on the ".eth" hostname. The proxy does too much interference with 8000. - const { driver } = await buildWebDriver({ proxyUrl: '127.0.0.1:8001' }); - await driver.navigate(); - - // The setting defaults to "on" so we can simply enter an ENS address - // into the address bar and listen for address change - try { - await driver.openNewPage(ENS_NAME_URL); - } catch (e) { - // Ignore ERR_PROXY_CONNECTION_FAILED error - // since all we care about is getting to the correct URL - } - - // Ensure that the redirect to ENS Domains has happened - await driver.wait(async () => { - const currentUrl = await driver.getCurrentUrl(); - return currentUrl === ENS_DESTINATION_URL; - }, tinyDelayMs); - - await driver.quit(); + + await withFixtures( + { + fixtures: new FixtureBuilder().withNetworkControllerOnMainnet().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.title, + testSpecificMock: mockInfura, + driverOptions: { + proxyPort: '8001', + }, + }, + async ({ driver }) => { + await driver.navigate(); + + // The setting defaults to "on" so we can simply enter an ENS address + // into the address bar and listen for address change + try { + await driver.openNewPage(ENS_NAME_URL); + } catch (e) { + // Ignore ERR_PROXY_CONNECTION_FAILED error + // since all we care about is getting to the correct URL + } + + // Ensure that the redirect to ENS Domains has happened + await driver.wait(async () => { + const currentUrl = await driver.getCurrentUrl(); + return currentUrl === ENS_DESTINATION_URL; + }, 10000); + // Setting a large delay has proven to stabilize the flakiness of the redirect + // and it's only a MAX value + }, + ); }); it('Does not fetch ENS data for ENS Domain when ENS and IPFS switched off', async function () { diff --git a/test/e2e/webdriver/chrome.js b/test/e2e/webdriver/chrome.js index b5068759812a..edbf90179f5a 100644 --- a/test/e2e/webdriver/chrome.js +++ b/test/e2e/webdriver/chrome.js @@ -4,21 +4,27 @@ const { ThenableWebDriver } = require('selenium-webdriver'); // eslint-disable-l const { isHeadless } = require('../../helpers/env'); /** - * Proxy host to use for HTTPS requests + * Determine the appropriate proxy server value to use * - * @type {string} + * @param {string|number} [proxyPort] - The proxy port to use + * @returns {string} The proxy server address */ -const HTTPS_PROXY_HOST = `${ - process.env.SELENIUM_HTTPS_PROXY || '127.0.0.1:8000' -}`; +function getProxyServer(proxyPort) { + const DEFAULT_PROXY_HOST = '127.0.0.1:8000'; + const { SELENIUM_HTTPS_PROXY } = process.env; + if (proxyPort) { + return `127.0.0.1:${proxyPort}`; + } + return SELENIUM_HTTPS_PROXY || DEFAULT_PROXY_HOST; +} /** * A wrapper around a {@code WebDriver} instance exposing Chrome-specific functionality */ class ChromeDriver { - static async build({ openDevToolsForTabs, port }) { + static async build({ openDevToolsForTabs, port, proxyPort }) { const args = [ - `--proxy-server=${HTTPS_PROXY_HOST}`, // Set proxy in the way that doesn't interfere with Selenium Manager + `--proxy-server=${getProxyServer(proxyPort)}`, // Set proxy in the way that doesn't interfere with Selenium Manager '--disable-features=OptimizationGuideModelDownloading,OptimizationHintsFetching,OptimizationTargetPrediction,OptimizationHints,NetworkTimeServiceQuerying', // Stop chrome from calling home so much (auto-downloads of AI models; time sync) '--disable-component-update', // Stop chrome from calling home so much (auto-update) '--disable-dev-shm-usage', diff --git a/test/e2e/webdriver/firefox.js b/test/e2e/webdriver/firefox.js index f56436dc3892..e202c36c4092 100644 --- a/test/e2e/webdriver/firefox.js +++ b/test/e2e/webdriver/firefox.js @@ -20,11 +20,20 @@ const { isHeadless } = require('../../helpers/env'); const TEMP_PROFILE_PATH_PREFIX = path.join(os.tmpdir(), 'MetaMask-Fx-Profile'); /** - * Proxy host to use for HTTPS requests + * Determine the appropriate proxy server value to use + * + * @param {string|number} [proxyPort] - The proxy port to use + * @returns {string} The proxy server URL */ -const HTTPS_PROXY_HOST = new URL( - process.env.SELENIUM_HTTPS_PROXY || 'http://127.0.0.1:8000', -); +function getProxyServerURL(proxyPort) { + const DEFAULT_PROXY_HOST = 'http://127.0.0.1:8000'; + const { SELENIUM_HTTPS_PROXY } = process.env; + + if (proxyPort) { + return new URL(`http://127.0.0.1:${proxyPort}`); + } + return new URL(SELENIUM_HTTPS_PROXY || DEFAULT_PROXY_HOST); +} /** * A wrapper around a {@code WebDriver} instance exposing Firefox-specific functionality @@ -36,18 +45,21 @@ class FirefoxDriver { * @param {object} options - the options for the build * @param options.responsive * @param options.port + * @param options.proxyPort * @returns {Promise<{driver: !ThenableWebDriver, extensionUrl: string, extensionId: string}>} */ - static async build({ responsive, port }) { + static async build({ responsive, port, proxyPort }) { const templateProfile = fs.mkdtempSync(TEMP_PROFILE_PATH_PREFIX); const options = new firefox.Options().setProfile(templateProfile); + const proxyServerURL = getProxyServerURL(proxyPort); + // Set proxy in the way that doesn't interfere with Selenium Manager options.setPreference('network.proxy.type', 1); - options.setPreference('network.proxy.ssl', HTTPS_PROXY_HOST.hostname); + options.setPreference('network.proxy.ssl', proxyServerURL.hostname); options.setPreference( 'network.proxy.ssl_port', - parseInt(HTTPS_PROXY_HOST.port, 10), + parseInt(proxyServerURL.port, 10), ); options.setAcceptInsecureCerts(true); diff --git a/test/e2e/webdriver/index.js b/test/e2e/webdriver/index.js index 900e2f599157..4d5197bb0dac 100644 --- a/test/e2e/webdriver/index.js +++ b/test/e2e/webdriver/index.js @@ -3,14 +3,23 @@ const { Driver } = require('./driver'); const ChromeDriver = require('./chrome'); const FirefoxDriver = require('./firefox'); -async function buildWebDriver({ openDevToolsForTabs, port, timeOut } = {}) { +async function buildWebDriver({ + openDevToolsForTabs, + port, + timeOut, + proxyPort, +} = {}) { const browser = process.env.SELENIUM_BROWSER; const { driver: seleniumDriver, extensionId, extensionUrl, - } = await buildBrowserWebDriver(browser, { openDevToolsForTabs, port }); + } = await buildBrowserWebDriver(browser, { + openDevToolsForTabs, + port, + proxyPort, + }); const driver = new Driver(seleniumDriver, browser, extensionUrl, timeOut); return {