Skip to content

Commit ba4a98b

Browse files
test: Add UI integration tests (MetaMask#24428)
## **Description** This PR introduces UI integration tests to the Extension codebase. By introducing UI Integration tests we are not just providing a new testing tool but also aiming to better clarify the differences between unit, integration and e2e tests, as well as when developers should aim at each. ### **What are integration tests?** Integration tests evaluate the interaction between multiple components within the application, ensuring they work together as intended. For MetaMask Extension, we are aiming to make UI integration tests particularly focused on page-level components and are conducted in the context of the full UI application. **When to write integration tests?** Integration tests should be written for page-level components. These tests should simulate user journeys and validate the application's behavior in a full app context. **Generic guidelines** * **Full App Context**: Always test page-level components in the context of the full application. This approach ensures that tests reflect real user scenarios and interactions within the app. * **Focus on User Journeys**: Design tests to mimic actual user actions and flows. This helps in identifying issues that could affect the user experience. * **Avoid Testing Non-Page Components**: Integration tests should not be written for components other than page-level components. Other components should be tested using unit tests. * **Avoid Snapshots**: Refrain from using snapshot testing in integration tests. They tend to be less meaningful in the context of full app testing and can lead to brittle tests. * **Test Location**: Place integration test files alongside the page component they are testing, with an appropriate naming convention (e.g., page.integration.test.ts). * **No Mocks**: Keep mocking to minimum. Ideally only the background connection, or any network request (fired from the UI) should be mocked. ### **What are unit tests?** Unit tests understanding is a bit more diffuse. As there is no consensus on how they should be written and what they should test. But generically, [as captured by Martin Fowler](https://martinfowler.com/articles/2021-test-shapes.html), there's two lines of thought: solitary unit tests and sociable unit tests. Within MetaMask there are multiple examples from tests and discussions highlighting the two types (for example you can check the discussion on [this PR](https://www.google.com/url?q=https://github.com/MetaMask/core/pull/3827%23discussion_r1469377179&sa=D&source=editors&ust=1716475694980436&usg=AOvVaw3oTcfVgfRuiQFSDChZPrFM)) Unit tests examine individual components or functions in isolation or within their immediate context. At MetaMask we encourage a flexible approach to unit testing, recognizing the spectrum between sociable and solitary unit tests. **When to write unit tests?** All components, except page-level components, should be validated through unit tests. These tests focus on the component's internal logic and functionality. Integration tests are preferred for page-level components. However, developers may choose to write sociable unit tests for specific scenarios where integration tests might not be necessary or practical. **Generic guidelines** * **Flexibility in Isolation**: Depending on the test's purpose, developers can choose between sociable unit tests (without mocking child components) and solitary unit tests (with mocked dependencies) to best suit the testing needs. * **Colocation**: Keep unit test files next to their corresponding implementation files. This practice enhances test discoverability and maintenance. * **Granularity and Focus**: Ensure unit tests are focused and granular, targeting specific functionalities. The choice between sociable and solitary testing should be guided by the test's objectives and the component's role within the application. * **Unit Tests for Page Components**: While integration tests are the primary method for testing page-level components, developers have the discretion to use unit tests for specific cases. This flexibility allows for targeted testing of component logic or behavior that may not require the full app context. ### **What are e2e tests?** e2e tests simulate real user scenarios from start to finish, testing the application as a whole. In the Extension, e2e tests are tests that strive to test a real extension app (including background, contentscript and ui processes) in a controlled environment from a user perspective. **When to write e2e tests?** Testing application's integration with external systems or services through critical user journeys. This includes any interactions between the UI, background process, dapp, blockchain, etc. **Generic guidelines** * **Realistic Scenarios**: Test scenarios that closely mimic real user actions and flows. ### **Broader guidelines** * **Code fencing:** is very problematic for unit/integration testing. We should avoid it. * **jest manual mocks:** This types of mocks are automatically picked up by jest for all tests. We should be very careful when writing this types of mocks as they will be shared across all tests. And with integration tests, as we are aiming to mock as little as possible, we should avoid this at all costs. ### **Conclusion** To sum up, understanding the distinction between integration and unit tests and applying them appropriately is crucial for maintaining MetaMask's code quality and reliability. Integration tests offer a broad view of user interactions and system integration for page-level components, while unit tests provide detailed insights into the functionality of individual components. By adhering to these guidelines and embracing the flexibility between sociable and solitary unit tests, developers can effectively contribute to MetaMask's robustness and user satisfaction. ### **References** Some of the references used to sustain the UI integration tests approach are: https://martinfowler.com/articles/2021-test-shapes.html https://martinfowler.com/bliki/TestPyramid.html https://martinfowler.com/bliki/IntegrationTest.html https://kentcdodds.com/blog/write-tests#how-to-write-more-integration-tests https://kentcdodds.com/blog/static-vs-unit-vs-integration-vs-e2e-tests https://redux.js.org/usage/writing-tests#guiding-principles https://testing-library.com/docs/guiding-principles ## Relevant changes * UI integration tests configuration: jest config file, specific setup handlers, package.json scripts * Test renderer helper for integration tests: split redux store initialisation from render the application; add helper to render the actual UI application given a metamask state (which is just a flatten controllers state served by the background process) * Remove react router dom manual mock: manual mocks are picked up by jest, and this was preventing proper routing for the integration tests. Removed the generic manual mock and added specific mocks to all unit tests that required it. * Two UI integration tests implemented (derived from existing e2e tests): personal sign confirmation (new design) and wallet created events * Added CI job to run UI integration tests * Added vscode launch.json config to attach debugger to UI integration tests ## Codespaces [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24428?quickstart=1) ## **Manual testing steps** In the terminal run: ```sh yarn test:integration ``` ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Priya Narayanaswamy <[email protected]>
1 parent fb5a218 commit ba4a98b

File tree

33 files changed

+3143
-63
lines changed

33 files changed

+3143
-63
lines changed
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
name: Run integration tests
2+
3+
on:
4+
push:
5+
branches: [develop, master]
6+
pull_request:
7+
types: [opened,reopened,synchronize]
8+
9+
jobs:
10+
test-integration:
11+
runs-on: ubuntu-latest
12+
steps:
13+
- name: Checkout repository
14+
uses: actions/checkout@v4
15+
16+
- name: Setup environment
17+
uses: ./.github/actions/setup-environment
18+
19+
- name: test:integration:coverage
20+
run: yarn test:integration:coverage

.vscode/launch.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,22 @@
1717
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
1818
}
1919
},
20+
{
21+
"type": "node",
22+
"request": "launch",
23+
"name": "Jest Integration: current file",
24+
"program": "${workspaceFolder}/node_modules/.bin/jest",
25+
"args": [
26+
"${fileBasenameNoExtension}",
27+
"--config",
28+
"jest.integration.config.js"
29+
],
30+
"console": "integratedTerminal",
31+
"internalConsoleOptions": "neverOpen",
32+
"windows": {
33+
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
34+
}
35+
},
2036
{
2137
"type": "node",
2238
"request": "launch",

jest.integration.config.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
module.exports = {
2+
collectCoverageFrom: [
3+
'<rootDir>/shared/**/*.(js|ts|tsx)',
4+
'<rootDir>/ui/**/*.(js|ts|tsx)',
5+
],
6+
coverageDirectory: './coverage/integration',
7+
coveragePathIgnorePatterns: ['.stories.*', '.snap', '.test.(js|ts|tsx)'],
8+
coverageReporters: ['html', 'json'],
9+
reporters: [
10+
'default',
11+
[
12+
'jest-junit',
13+
{
14+
outputDirectory: 'test/test-results/integration',
15+
outputName: 'junit.xml',
16+
},
17+
],
18+
],
19+
restoreMocks: true,
20+
setupFiles: [
21+
'<rootDir>/test/integration/config/setup.js',
22+
'<rootDir>/test/integration/config/env.js',
23+
],
24+
setupFilesAfterEnv: ['<rootDir>/test/integration/config/setupAfter.js'],
25+
testMatch: ['<rootDir>/test/integration/**/*.test.(js|ts|tsx)'],
26+
testPathIgnorePatterns: ['<rootDir>/test/integration/config/*'],
27+
testTimeout: 5500,
28+
// We have to specify the environment we are running in, which is jsdom. The
29+
// default is 'node'. This can be modified *per file* using a comment at the
30+
// head of the file. So it may be worthwhile to switch to 'node' in any
31+
// background tests.
32+
testEnvironment: 'jsdom',
33+
testEnvironmentOptions: {
34+
customExportConditions: ['node', 'node-addons'],
35+
},
36+
workerIdleMemoryLimit: '500MB',
37+
};

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
"dapp-chain": "GANACHE_ARGS='-b 2' concurrently -k -n ganache,dapp -p '[{time}][{name}]' 'yarn ganache:start' 'sleep 5 && yarn dapp'",
3939
"forwarder": "node ./development/static-server.js ./node_modules/@metamask/forwarder/dist/ --port 9010",
4040
"dapp-forwarder": "concurrently -k -n forwarder,dapp -p '[{time}][{name}]' 'yarn forwarder' 'yarn dapp'",
41+
"test:integration": "jest --config jest.integration.config.js",
42+
"test:integration:coverage": "jest --config jest.integration.config.js --coverage",
4143
"test:unit": "node ./test/run-unit-tests.js --jestGlobal --jestDev",
4244
"test:unit:jest": "node ./test/run-unit-tests.js --jestGlobal --jestDev",
4345
"test:unit:jest:watch": "node --inspect ./node_modules/.bin/jest --watch",

test/e2e/tests/confirmations/header.spec.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,14 @@ describe('Confirmation Header Component', function () {
8686

8787
async function clickHeaderInfoBtn(driver) {
8888
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
89-
await driver.clickElement('button[data-testid="header-info-button"]');
89+
await driver.clickElement(
90+
'button[data-testid="header-info__account-details-button"]',
91+
);
9092
}
9193

9294
async function assertHeaderInfoBalance(driver, walletEthBalance) {
9395
const headerBalanceEl = await driver.findElement(
94-
'[data-testid="header-balance"]',
96+
'[data-testid="confirmation-account-details-modal__account-balance"]',
9597
);
9698
await driver.waitForNonEmptyElement(headerBalanceEl);
9799
assert.equal(await headerBalanceEl.getText(), `${walletEthBalance}\nETH`);

test/helpers/setup-after-helper.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// This file is for Jest-specific setup only and runs before our Jest tests.
2+
3+
import nock from 'nock';
4+
import '@testing-library/jest-dom';
5+
6+
jest.mock('webextension-polyfill', () => {
7+
return {
8+
runtime: {
9+
getManifest: () => ({ manifest_version: 2 }),
10+
onMessage: {
11+
removeListener: jest.fn(),
12+
addListener: jest.fn(),
13+
},
14+
},
15+
};
16+
});
17+
18+
/* eslint-disable-next-line jest/require-top-level-describe */
19+
beforeEach(() => {
20+
nock.cleanAll();
21+
});
22+
23+
// Setup window.prompt
24+
global.prompt = () => undefined;

test/helpers/setup-helper.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ global.chrome = {
2222
},
2323
};
2424

25+
global.indexedDB = {};
26+
2527
nock.disableNetConnect();
2628
nock.enableNetConnect('localhost');
2729
if (typeof beforeEach === 'function') {
@@ -125,3 +127,8 @@ if (!window.navigator.clipboard) {
125127
if (!window.navigator.clipboard.writeText) {
126128
window.navigator.clipboard.writeText = () => undefined;
127129
}
130+
131+
window.SVGPathElement = window.SVGPathElement || { prototype: {} };
132+
133+
// scrollIntoView is not available in JSDOM
134+
window.HTMLElement.prototype.scrollIntoView = () => undefined;

test/integration/config/env.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
require('../../env');

test/integration/config/setup.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
require('@babel/register');
2+
require('ts-node').register({ transpileOnly: true });
3+
const fs = require('node:fs/promises');
4+
const path = require('path');
5+
6+
require('../../helpers/setup-helper');
7+
8+
// Fetch
9+
// fetch is part of node js in future versions, thus triggering no-shadow
10+
// eslint-disable-next-line no-shadow
11+
const { default: fetch, Headers, Request, Response } = require('node-fetch');
12+
13+
const handleRelativePathRequest = async (url, localeRelativePathRequest) => {
14+
try {
15+
const fullLocalePath = path.join(
16+
process.cwd(),
17+
'app',
18+
localeRelativePathRequest[0],
19+
);
20+
21+
const content = await fs.readFile(fullLocalePath, { encoding: 'utf8' });
22+
23+
return new Response(content);
24+
} catch (error) {
25+
throw new Error(`Failed to fetch ${url}: ${error.message}`);
26+
}
27+
};
28+
29+
const shimmedFetch = async (url, ...args) => {
30+
try {
31+
return await fetch(url, ...args);
32+
} catch (error) {
33+
if (error.message !== 'Only absolute URLs are supported') {
34+
throw error;
35+
}
36+
37+
const regex = /_locales\/([^/]+)\/messages\.json/gu;
38+
const localeRelativePathRequest = url.match(regex);
39+
40+
if (!localeRelativePathRequest?.length) {
41+
throw error;
42+
}
43+
44+
return handleRelativePathRequest(url, localeRelativePathRequest);
45+
}
46+
};
47+
48+
Object.assign(window, { fetch: shimmedFetch, Headers, Request, Response });
49+
// some of our libraries currently assume that `fetch` is globally available,
50+
// so we need to assign this for tests to run
51+
global.fetch = shimmedFetch;
52+
53+
global.metamask = {};

test/integration/config/setupAfter.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// This file is for Jest-specific setup only and runs before our Jest tests.
2+
import '../../helpers/setup-after-helper';

0 commit comments

Comments
 (0)