Skip to content

Commit

Permalink
test: Fix test config
Browse files Browse the repository at this point in the history
Our Mocha and Jest configuration has been messy because we're in the
middle of a migration from Mocha to Jest. Each file had to be
explicitly included in the Jest configuration or ignored in the Mocha
configuration. This was inconvenient and error-prone, and resulted in
some tests not being run at all.

Both test configurations have been updated to use a shared list of
Mocha test files. There are only a few of these files left, and this
list should only get shorter as we migrate more tests to Jest. No
further configuration changes will be needed to add Jest tests.
  • Loading branch information
Gudahtt committed Jun 3, 2024
1 parent 63851c9 commit 32b777e
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 91 deletions.
47 changes: 11 additions & 36 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const path = require('node:path');
const ts = require('typescript');
const { version: reactVersion } = require('react/package.json');

const { legacyMochaTests } = require('./test/mocha/legacy-mocha-tests');

const tsconfigPath = ts.findConfigFile('./', ts.sys.fileExists);
const { config } = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
const tsconfig = ts.parseJsonConfigFileContent(config, ts.sys, './');
Expand Down Expand Up @@ -261,26 +263,7 @@ module.exports = {
* Mocha library.
*/
{
files: [
'**/*.test.js',
'test/lib/wait-until-called.js',
'test/e2e/**/*.spec.js',
],
excludedFiles: [
'app/scripts/controllers/app-state.test.js',
'app/scripts/controllers/mmi-controller.test.js',
'app/scripts/controllers/permissions/**/*.test.js',
'app/scripts/controllers/preferences.test.js',
'app/scripts/lib/**/*.test.js',
'app/scripts/metamask-controller.test.js',
'app/scripts/migrations/*.test.js',
'app/scripts/platforms/*.test.js',
'development/**/*.test.js',
'shared/**/*.test.js',
'ui/**/*.test.js',
'ui/__mocks__/*.js',
'test/e2e/helpers.test.js',
],
files: [...legacyMochaTests, 'test/e2e/**/*.spec.js'],
extends: ['@metamask/eslint-config-mocha'],
rules: {
// In Mocha tests, it is common to use `this` to store values or do
Expand All @@ -298,26 +281,18 @@ module.exports = {
{
files: [
'**/__snapshots__/*.snap',
'app/scripts/controllers/app-state.test.js',
'app/scripts/controllers/mmi-controller.test.ts',
'app/scripts/controllers/permissions/**/*.test.js',
'app/scripts/controllers/preferences.test.js',
'app/scripts/lib/**/*.test.js',
'app/scripts/metamask-controller.test.js',
'app/scripts/migrations/*.test.js',
'app/scripts/platforms/*.test.js',
'development/**/*.test.js',
'development/**/*.test.ts',
'**/__mocks__/*.js',
'app/scripts/**/*.test.js',
'app/scripts/**/*.test.ts',
'app/scripts/**/*.test.tsx',
'shared/**/*.test.js',
'shared/**/*.test.ts',
'test/helpers/*.js',
'test/jest/*.js',
'test/lib/timer-helpers.js',
'test/e2e/helpers.test.js',
'shared/**/*.test.tsx',
'ui/**/*.test.js',
'ui/__mocks__/*.js',
'shared/lib/error-utils.test.js',
'ui/**/*.test.ts',
'ui/**/*.test.tsx',
],
excludedFiles: legacyMochaTests,
extends: ['@metamask/eslint-config-jest'],
parserOptions: {
sourceType: 'module',
Expand Down
13 changes: 0 additions & 13 deletions .mocharc.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,6 @@
module.exports = {
// TODO: Remove the `exit` setting, it can hide broken tests.
exit: true,
ignore: [
'./app/scripts/lib/**/*.test.js',
'./app/scripts/migrations/*.test.js',
'./app/scripts/platforms/*.test.js',
'./app/scripts/controllers/app-state.test.js',
'./app/scripts/controllers/permissions/**/*.test.js',
'./app/scripts/controllers/mmi-controller.test.ts',
'./app/scripts/controllers/preferences.test.js',
'./app/scripts/constants/error-utils.test.js',
'./app/scripts/metamask-controller.test.js',
'./development/fitness-functions/**/*.test.ts',
'./test/e2e/helpers.test.js',
],
recursive: true,
require: ['test/env.js', 'test/setup.js'],
};
59 changes: 18 additions & 41 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,22 @@
const {
legacyMochaTests,
legacyMochaTestCoveredFiles,
} = require('./test/mocha/legacy-mocha-tests');

module.exports = {
collectCoverageFrom: [
'<rootDir>/app/scripts/constants/error-utils.js',
'<rootDir>/app/scripts/controllers/permissions/**/*.js',
'<rootDir>/app/scripts/controllers/sign.ts',
'<rootDir>/app/scripts/controllers/decrypt-message.ts',
'<rootDir>/app/scripts/controllers/transactions/etherscan.ts',
'<rootDir>/app/scripts/controllers/transactions/EtherscanRemoteTransactionSource.ts',
'<rootDir>/app/scripts/controllers/transactions/IncomingTransactionHelper.ts',
'<rootDir>/app/scripts/controllers/preferences.js',
'<rootDir>/app/scripts/flask/**/*.js',
'<rootDir>/app/scripts/lib/**/*.(js|ts)',
'<rootDir>/app/scripts/metamask-controller.js',
'<rootDir>/app/scripts/migrations/*.js',
'<rootDir>/app/scripts/migrations/*.ts',
'!<rootDir>/app/scripts/migrations/*.test.(js|ts)',
'<rootDir>/app/scripts/platforms/*.js',
'<rootDir>/app/scripts/**/*.(js|ts|tsx)',
'<rootDir>/shared/**/*.(js|ts|tsx)',
'<rootDir>/ui/**/*.(js|ts|tsx)',
'<rootDir>/development/fitness-functions/**/*.test.(js|ts|tsx)',
'<rootDir>/test/e2e/helpers.test.js',
],
coverageDirectory: './coverage',
coveragePathIgnorePatterns: ['.stories.*', '.snap'],
coveragePathIgnorePatterns: [
'.stories.*',
'.snap',
...legacyMochaTestCoveredFiles.map((filePath) => {
return `<rootDir>/${filePath}`;
}),
],
coverageReporters: ['html', 'json'],
reporters: [
'default',
Expand All @@ -39,33 +34,15 @@ module.exports = {
setupFiles: ['<rootDir>/test/setup.js', '<rootDir>/test/env.js'],
setupFilesAfterEnv: ['<rootDir>/test/jest/setup.js'],
testMatch: [
'<rootDir>/app/scripts/constants/error-utils.test.js',
'<rootDir>/app/scripts/controllers/app-state.test.js',
'<rootDir>/app/scripts/controllers/transactions/etherscan.test.ts',
'<rootDir>/app/scripts/controllers/transactions/EtherscanRemoteTransactionSource.test.ts',
'<rootDir>/app/scripts/controllers/transactions/IncomingTransactionHelper.test.ts',
'<rootDir>/app/scripts/controllers/onboarding.test.ts',
'<rootDir>/app/scripts/controllers/mmi-controller.test.ts',
'<rootDir>/app/scripts/controllers/permissions/**/*.test.js',
'<rootDir>/app/scripts/controllers/preferences.test.js',
'<rootDir>/app/scripts/controllers/sign.test.ts',
'<rootDir>/app/scripts/controllers/decrypt-message.test.ts',
'<rootDir>/app/scripts/controllers/authentication/**/*.test.ts',
'<rootDir>/app/scripts/controllers/push-platform-notifications/**/*.test.ts',
'<rootDir>/app/scripts/controllers/user-storage/**/*.test.ts',
'<rootDir>/app/scripts/controllers/metamask-notifications/**/*.test.ts',
'<rootDir>/app/scripts/flask/**/*.test.js',
'<rootDir>/app/scripts/lib/**/*.test.(js|ts)',
'<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js',
'<rootDir>/app/scripts/metamask-controller.test.js',
'<rootDir>/app/scripts/migrations/*.test.(js|ts)',
'<rootDir>/app/scripts/platforms/*.test.js',
'<rootDir>/app/scripts/translate.test.ts',
'<rootDir>/shared/**/*.test.(js|ts)',
'<rootDir>/app/scripts/**/*.test.(js|ts|tsx)',
'<rootDir>/shared/**/*.test.(js|ts|tsx)',
'<rootDir>/ui/**/*.test.(js|ts|tsx)',
'<rootDir>/development/fitness-functions/**/*.test.(js|ts|tsx)',
'<rootDir>/test/e2e/helpers.test.js',
],
testPathIgnorePatterns: legacyMochaTests.map((filePath) => {
return `<rootDir>/${filePath}`;
}),
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
Expand Down
23 changes: 23 additions & 0 deletions test/mocha/legacy-mocha-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const legacyMochaTests = [
'app/scripts/controllers/app-metadata.test.ts',
'app/scripts/controllers/metametrics.test.js',
'app/scripts/controllers/swaps.test.js',
'app/scripts/detect-multiple-instances.test.js',
'app/scripts/metamask-controller.actions.test.js',
]

const legacyMochaTestCoveredFiles = [
'app/scripts/controllers/app-metadata.ts',
'app/scripts/controllers/metametrics.js',
'app/scripts/controllers/swaps.js',
'app/scripts/detect-multiple-instances.js',
// Intentionally omitted because it's partially covered by Jest tests as well
// Lets consider the Jest coverage as the authority on what is covered here, since we'll be
// migrating these tests to Jest eventually.
// 'app/scripts/metamask-controller.js'
]

module.exports = {
legacyMochaTests,
legacyMochaTestCoveredFiles,
}
3 changes: 2 additions & 1 deletion test/run-unit-tests.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { hideBin } = require('yargs/helpers');
const yargs = require('yargs/yargs');
const { runCommand, runInShell } = require('../development/lib/run-command');
const { legacyMochaTests } = require('./mocha/legacy-mocha-tests');

const { CIRCLE_NODE_INDEX, CIRCLE_NODE_TOTAL } = process.env;

Expand Down Expand Up @@ -71,7 +72,7 @@ async function runJest(
* @param {boolean} coverage - Use nyc to collect coverage
*/
async function runMocha({ coverage }) {
const options = ['mocha', './app/**/*.test.js'];
const options = ['mocha', ...legacyMochaTests];
// If coverage is true, then we need to run nyc as the first command
// and mocha after, so we use unshift to add three options to the beginning
// of the options array.
Expand Down

0 comments on commit 32b777e

Please sign in to comment.