Skip to content

Commit

Permalink
test: Add UI integration tests (#24428)
Browse files Browse the repository at this point in the history
## **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]>
  • Loading branch information
cryptotavares and pnarayanaswamy authored Jul 10, 2024
1 parent fb5a218 commit ba4a98b
Show file tree
Hide file tree
Showing 33 changed files with 3,143 additions and 63 deletions.
20 changes: 20 additions & 0 deletions .github/workflows/run-integration-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: Run integration tests

on:
push:
branches: [develop, master]
pull_request:
types: [opened,reopened,synchronize]

jobs:
test-integration:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Setup environment
uses: ./.github/actions/setup-environment

- name: test:integration:coverage
run: yarn test:integration:coverage
16 changes: 16 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
}
},
{
"type": "node",
"request": "launch",
"name": "Jest Integration: current file",
"program": "${workspaceFolder}/node_modules/.bin/jest",
"args": [
"${fileBasenameNoExtension}",
"--config",
"jest.integration.config.js"
],
"console": "integratedTerminal",
"internalConsoleOptions": "neverOpen",
"windows": {
"program": "${workspaceFolder}/node_modules/jest/bin/jest"
}
},
{
"type": "node",
"request": "launch",
Expand Down
37 changes: 37 additions & 0 deletions jest.integration.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
module.exports = {
collectCoverageFrom: [
'<rootDir>/shared/**/*.(js|ts|tsx)',
'<rootDir>/ui/**/*.(js|ts|tsx)',
],
coverageDirectory: './coverage/integration',
coveragePathIgnorePatterns: ['.stories.*', '.snap', '.test.(js|ts|tsx)'],
coverageReporters: ['html', 'json'],
reporters: [
'default',
[
'jest-junit',
{
outputDirectory: 'test/test-results/integration',
outputName: 'junit.xml',
},
],
],
restoreMocks: true,
setupFiles: [
'<rootDir>/test/integration/config/setup.js',
'<rootDir>/test/integration/config/env.js',
],
setupFilesAfterEnv: ['<rootDir>/test/integration/config/setupAfter.js'],
testMatch: ['<rootDir>/test/integration/**/*.test.(js|ts|tsx)'],
testPathIgnorePatterns: ['<rootDir>/test/integration/config/*'],
testTimeout: 5500,
// We have to specify the environment we are running in, which is jsdom. The
// default is 'node'. This can be modified *per file* using a comment at the
// head of the file. So it may be worthwhile to switch to 'node' in any
// background tests.
testEnvironment: 'jsdom',
testEnvironmentOptions: {
customExportConditions: ['node', 'node-addons'],
},
workerIdleMemoryLimit: '500MB',
};
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
"dapp-chain": "GANACHE_ARGS='-b 2' concurrently -k -n ganache,dapp -p '[{time}][{name}]' 'yarn ganache:start' 'sleep 5 && yarn dapp'",
"forwarder": "node ./development/static-server.js ./node_modules/@metamask/forwarder/dist/ --port 9010",
"dapp-forwarder": "concurrently -k -n forwarder,dapp -p '[{time}][{name}]' 'yarn forwarder' 'yarn dapp'",
"test:integration": "jest --config jest.integration.config.js",
"test:integration:coverage": "jest --config jest.integration.config.js --coverage",
"test:unit": "node ./test/run-unit-tests.js --jestGlobal --jestDev",
"test:unit:jest": "node ./test/run-unit-tests.js --jestGlobal --jestDev",
"test:unit:jest:watch": "node --inspect ./node_modules/.bin/jest --watch",
Expand Down
6 changes: 4 additions & 2 deletions test/e2e/tests/confirmations/header.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,14 @@ describe('Confirmation Header Component', function () {

async function clickHeaderInfoBtn(driver) {
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.clickElement('button[data-testid="header-info-button"]');
await driver.clickElement(
'button[data-testid="header-info__account-details-button"]',
);
}

async function assertHeaderInfoBalance(driver, walletEthBalance) {
const headerBalanceEl = await driver.findElement(
'[data-testid="header-balance"]',
'[data-testid="confirmation-account-details-modal__account-balance"]',
);
await driver.waitForNonEmptyElement(headerBalanceEl);
assert.equal(await headerBalanceEl.getText(), `${walletEthBalance}\nETH`);
Expand Down
24 changes: 24 additions & 0 deletions test/helpers/setup-after-helper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// This file is for Jest-specific setup only and runs before our Jest tests.

import nock from 'nock';
import '@testing-library/jest-dom';

jest.mock('webextension-polyfill', () => {
return {
runtime: {
getManifest: () => ({ manifest_version: 2 }),
onMessage: {
removeListener: jest.fn(),
addListener: jest.fn(),
},
},
};
});

/* eslint-disable-next-line jest/require-top-level-describe */
beforeEach(() => {
nock.cleanAll();
});

// Setup window.prompt
global.prompt = () => undefined;
7 changes: 7 additions & 0 deletions test/helpers/setup-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ global.chrome = {
},
};

global.indexedDB = {};

nock.disableNetConnect();
nock.enableNetConnect('localhost');
if (typeof beforeEach === 'function') {
Expand Down Expand Up @@ -125,3 +127,8 @@ if (!window.navigator.clipboard) {
if (!window.navigator.clipboard.writeText) {
window.navigator.clipboard.writeText = () => undefined;
}

window.SVGPathElement = window.SVGPathElement || { prototype: {} };

// scrollIntoView is not available in JSDOM
window.HTMLElement.prototype.scrollIntoView = () => undefined;
1 change: 1 addition & 0 deletions test/integration/config/env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require('../../env');
53 changes: 53 additions & 0 deletions test/integration/config/setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
require('@babel/register');
require('ts-node').register({ transpileOnly: true });
const fs = require('node:fs/promises');
const path = require('path');

require('../../helpers/setup-helper');

// Fetch
// fetch is part of node js in future versions, thus triggering no-shadow
// eslint-disable-next-line no-shadow
const { default: fetch, Headers, Request, Response } = require('node-fetch');

const handleRelativePathRequest = async (url, localeRelativePathRequest) => {
try {
const fullLocalePath = path.join(
process.cwd(),
'app',
localeRelativePathRequest[0],
);

const content = await fs.readFile(fullLocalePath, { encoding: 'utf8' });

return new Response(content);
} catch (error) {
throw new Error(`Failed to fetch ${url}: ${error.message}`);
}
};

const shimmedFetch = async (url, ...args) => {
try {
return await fetch(url, ...args);
} catch (error) {
if (error.message !== 'Only absolute URLs are supported') {
throw error;
}

const regex = /_locales\/([^/]+)\/messages\.json/gu;
const localeRelativePathRequest = url.match(regex);

if (!localeRelativePathRequest?.length) {
throw error;
}

return handleRelativePathRequest(url, localeRelativePathRequest);
}
};

Object.assign(window, { fetch: shimmedFetch, Headers, Request, Response });
// some of our libraries currently assume that `fetch` is globally available,
// so we need to assign this for tests to run
global.fetch = shimmedFetch;

global.metamask = {};
2 changes: 2 additions & 0 deletions test/integration/config/setupAfter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file is for Jest-specific setup only and runs before our Jest tests.
import '../../helpers/setup-after-helper';
Loading

0 comments on commit ba4a98b

Please sign in to comment.