Skip to content

Commit

Permalink
fix: flaky test `Settings Redirects to ENS domains when user inputs E…
Browse files Browse the repository at this point in the history 
…NS 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 <danjm.com@gmail.com>
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 <howrad@gmail.com>
  • Loading branch information
@seaona @danjm
@hjetpoluru @legobeat @HowardBraham
5 people authored Jun 21, 2024
1 parent 889ff38 commit 3552a39
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 37 deletions.
4 changes: 4 additions & 0 deletions test/e2e/tests/ppom/mocks/json-rpc-result.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type mockJsonRpcResultType = {
export const mockJsonRpcResult: mockJsonRpcResultType = {
eth_blockNumber: {
default: MOCK_BLOCK_NUMBER,
custom: '0x130E45F',
},

eth_estimateGas: {
Expand Down Expand Up @@ -387,4 +388,7 @@ export const mockJsonRpcResult: mockJsonRpcResultType = {
BUSD_STORAGE_2:
'0x000000000000000000000000137dcd97872de27a4d3bf36a4643c5e18fa40713',
},
net_version: {
default: '1',
},
};
200 changes: 179 additions & 21 deletions test/e2e/tests/settings/ipfs-ens-resolution.spec.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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 () {
Expand Down
20 changes: 13 additions & 7 deletions test/e2e/webdriver/chrome.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
26 changes: 19 additions & 7 deletions test/e2e/webdriver/firefox.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions test/e2e/webdriver/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 3552a39

Please sign in to comment.