Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate apps unit tests away from wp scripts #79097

Merged
merged 6 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 1 addition & 16 deletions .teamcity/_self/projects/WPComPlugins.kt
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,7 @@ private object EditingToolkit : WPComPluginBuild(
sed -i -e "/^Stable tag:\s/c\Stable tag: %build.number%" ./editing-toolkit-plugin/readme.txt
"""
}
bashNodeScript {
name = "Run JS tests"
scriptContent = """
cd apps/editing-toolkit
yarn test:js --reporters=default --reporters=jest-teamcity --maxWorkers=${'$'}JEST_MAX_WORKERS
"""
}

// Note: We run the PHP lint after the build to verify that the newspack-blocks
// code is also formatted correctly.
bashNodeScript {
Expand Down Expand Up @@ -223,15 +217,6 @@ private object OdysseyStats : WPComPluginBuild(
withPRNotify = "false",
docsLink = "PejTkB-3N-p2",
buildSteps = {
bashNodeScript {
name = "Run Unit Tests"
scriptContent = """
cd apps/odyssey-stats

# run unit tests
yarn test:js --reporters=default --reporters=jest-teamcity --maxWorkers=${'$'}JEST_MAX_WORKERS
"""
}
bashNodeScript {
name = "Run Size Test"
scriptContent = """
Expand Down
5 changes: 0 additions & 5 deletions apps/editing-toolkit/bin/babel-transform.js

This file was deleted.

2 changes: 0 additions & 2 deletions apps/editing-toolkit/bin/js-unit-setup.js
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
import '@testing-library/jest-dom/extend-expect';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is handled by the calypso-jest setup file.


jest.mock( 'a8c-fse-common-data-stores', () => {}, { virtual: true } );
39 changes: 3 additions & 36 deletions apps/editing-toolkit/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,4 @@
/**
* Test configuration for the FSE plugin.
*
* Will match files such that:
* 1. Must be in the apps/editing-toolkit/ directory
* 2. Must have .test.EXT at the end of the filename
* 3. EXT (above) must be one of js, ts, jsx, or tsx.
*
* Note: In order to use a different jest config for e2e tests, this config file
* must be kept in the bin/ folder to prevent it from being detected as the
* config file for e2e tests.
*/

const path = require( 'path' );
const base = require( '@automattic/calypso-jest' );
// @wordpress/scripts manually adds additional Jest config ontop of
// @wordpress/jest-preset-default so we pull in this file to extend it
const defaults = require( '@wordpress/scripts/config/jest-unit.config.js' );

// Basically, CWD, so 'apps/editing-toolkit'.
// Without this, it tries to use 'apps/editing-toolkit/bin'
const pluginRoot = path.resolve( './' );

const config = {
...base,
...defaults,
rootDir: path.normalize( '../../' ), // To detect wp-calypso root node_modules
testMatch: [ `${ pluginRoot }/**/?(*.)test.[jt]s?(x)` ],
transform: { '^.+\\.[jt]sx?$': path.join( __dirname, 'bin', 'babel-transform' ) },
testEnvironment: 'jsdom',
setupFilesAfterEnv: [
...( defaults.setupFilesAfterEnv || [] ), // extend if present
'<rootDir>/apps/editing-toolkit/bin/js-unit-setup',
],
module.exports = {
preset: '../../test/apps/jest-preset.js',
setupFilesAfterEnv: [ require.resolve( './bin/js-unit-setup' ) ],
};

module.exports = config;
10 changes: 2 additions & 8 deletions apps/editing-toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@
"lint:php": "../../vendor/bin/phpcs --standard=../phpcs.xml ./ ",
"lint:php:fix": "../../vendor/bin/phpcbf --standard=../phpcs.xml ./ ",
"sync:newspack-blocks": "./bin/sync-newspack-blocks.sh",
"test:js": "npx wp-scripts test-unit-js --config='jest.config.js' --colors",
"test:js:help": "npx wp-scripts test-unit-js --config='jest.config.js' --help --colors",
"test:js:update-snapshots": "npx wp-scripts test-unit-js -u --config='jest.config.js' --colors",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these are useful -- yarn test:js -u, or yarn test:js --help will access these options. --watch would also work, but it's more commonly used.

"test:js:watch": "npx wp-scripts test-unit-js --config='jest.config.js' --watch --colors",
"test:js": "yarn run -T test-apps apps/editing-toolkit",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-T executes the script from the workspace root. In this case, it's necessary because babel-jest won't work unless it can find a babel config file. (And without that, imports don't work!) That config file is found in the root of the repo. There doesn't seem to be an easy way to configure this, other than running jest from the same directory. Running unit tests from the root package.json seems to solve that.

"test:js:watch": "yarn test:js --watch",
"test:php": "npx wp-env run phpunit 'phpunit -c /var/www/html/wp-content/plugins/editing-toolkit-plugin/phpunit.xml.dist'",
"wpcom-sync": "./bin/wpcom-watch-and-sync.sh"
},
Expand Down Expand Up @@ -98,7 +96,6 @@
"@wordpress/primitives": "^3.21.0",
"@wordpress/private-apis": "^0.9.0",
"@wordpress/rich-text": "^6.0.0",
"@wordpress/scripts": "^25.0.0",
"@wordpress/server-side-render": "^4.0.0",
"@wordpress/url": "^3.24.0",
"calypso": "workspace:^",
Expand All @@ -121,21 +118,18 @@
"devDependencies": {
"@automattic/calypso-apps-builder": "workspace:^",
"@automattic/calypso-eslint-overrides": "workspace:^",
"@automattic/calypso-jest": "workspace:^",
"@tanstack/eslint-plugin-query": "^4.29.8",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^14.0.0",
"@types/node": "^18.11.18",
"@types/wordpress__plugins": "^3.0.0",
"@wordpress/eslint-plugin": "^13.7.0",
"@wordpress/jest-preset-default": "^10.4.0",
"@wordpress/readable-js-assets-webpack-plugin": "^2.6.0",
"babel-jest": "^27.5.1",
"eslint-plugin-inclusive-language": "^2.2.0",
"eslint-plugin-json-es": "^1.5.7",
"eslint-plugin-md": "^1.0.19",
"eslint-plugin-you-dont-need-lodash-underscore": "^6.12.0",
"jest-teamcity": "^1.9.0",
"wait-for-expect": "^3.0.2",
"webpack": "^5.68.0"
}
Expand Down
5 changes: 0 additions & 5 deletions apps/odyssey-stats/bin/babel-transform.js

This file was deleted.

33 changes: 2 additions & 31 deletions apps/odyssey-stats/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,3 @@
/**
* Test configuration for the Odyssey Stats.
*
* Will match files such that:
* 1. Must be in the apps/odyssey-stats/ directory
* 2. Must have .test.EXT at the end of the filename
* 3. EXT (above) must be one of js, ts, jsx, or tsx.
*/

const path = require( 'path' );
const base = require( '@automattic/calypso-jest' );
// @wordpress/scripts manually adds additional Jest config ontop of
// @wordpress/jest-preset-default so we pull in this file to extend it
const defaults = require( '@wordpress/scripts/config/jest-unit.config.js' );

// Basically, CWD, so 'apps/odyssey-stats'.
// Without this, it tries to use 'apps/odyssey-stats/bin'
const pluginRoot = path.resolve( './' );

const config = {
...base,
...defaults,
rootDir: path.normalize( '../../' ), // To detect wp-calypso root node_modules
testMatch: [ `${ pluginRoot }/**/?(*.)test.[jt]s?(x)` ],
transform: { '^.+\\.[jt]sx?$': path.join( __dirname, 'bin', 'babel-transform' ) },
testEnvironment: 'jsdom',
setupFilesAfterEnv: [
...( defaults.setupFilesAfterEnv || [] ), // extend if present
],
module.exports = {
preset: '../../test/apps/jest-preset.js',
};

module.exports = config;
9 changes: 2 additions & 7 deletions apps/odyssey-stats/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@
"build:stats": "calypso-build",
"dev": "yarn run calypso-apps-builder --localPath dist --remotePath /home/wpcom/public_html/widgets.wp.com/calypso-stats",
"show-stats": "NODE_ENV=production EMIT_STATS=true yarn build",
"test:js": "npx wp-scripts test-unit-js --config='jest.config.js' --colors",
"test:js:help": "npx wp-scripts test-unit-js --config='jest.config.js' --help --colors",
"test:js:update-snapshots": "npx wp-scripts test-unit-js -u --config='jest.config.js' --colors",
"test:js:watch": "npx wp-scripts test-unit-js --config='jest.config.js' --watch --colors",
"test:js": "yarn run -T test-apps apps/odyssey-stats",
"test:js:watch": "yarn test:js --watch",
"test:size": "yarn run build && size-limit",
"translate": "rm -rf dist/strings && mkdirp dist && wp-babel-makepot '../../{client,packages,apps}/**/*.{js,jsx,ts,tsx}' --ignore '**/node_modules/**,**/test/**,**/*.d.ts' --base '../../' --dir './dist/strings' --output './dist/odyssey-strings.pot' && node bin/build-languages.js"
},
Expand Down Expand Up @@ -57,21 +55,18 @@
"@automattic/calypso-babel-config": "workspace:^",
"@automattic/calypso-build": "workspace:^",
"@automattic/calypso-eslint-overrides": "workspace:^",
"@automattic/calypso-jest": "workspace:^",
"@automattic/languages": "workspace:^",
"@automattic/webpack-extensive-lodash-replacement-plugin": "workspace:^",
"@automattic/webpack-inline-constant-exports-plugin": "workspace:^",
"@automattic/wp-babel-makepot": "workspace:^",
"@babel/core": "^7.17.5",
"@size-limit/file": "^8.2.4",
"@wordpress/dependency-extraction-webpack-plugin": "^4.6.0",
"@wordpress/scripts": "^25.0.0",
"autoprefixer": "^10.2.5",
"babel-jest": "^27.5",
"gettext-parser": "^6.0.0",
"html-webpack-plugin": "^5.0.0-beta.4",
"jest": "^27.2.4",
"jest-teamcity": "^1.9.0",
"lodash": "^4.17.21",
"mkdirp": "^1.0.4",
"node-fetch": "^2.6.6",
Expand Down
2 changes: 1 addition & 1 deletion bin/teamcity-task-runner.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { spawn } from 'child_process';
import { spawn } from 'node:child_process';
import chalk from 'chalk';

// This is technically an import side-effect for the callee, but any TeamCity
Expand Down
5 changes: 4 additions & 1 deletion bin/unit-test-suite.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env node
import util from 'util';
import util from 'node:util';
import glob from 'glob';
import runTask from './teamcity-task-runner.mjs';

Expand Down Expand Up @@ -60,6 +60,8 @@ const testClient = withUnitTestInfo( 'test-client --maxWorkers=8' );
const testPackages = withUnitTestInfo( 'test-packages --maxWorkers=4' );
const testServer = withUnitTestInfo( 'test-server --maxWorkers=4' );
const testBuildTools = withUnitTestInfo( 'test-build-tools --maxWorkers=4' );
// Includes ETK and Odyssey Stats, migrated here from their individual builds.
const testApps = withUnitTestInfo( 'test-apps --maxWorkers=1' );

const testWorkspaces = {
name: 'yarn',
Expand Down Expand Up @@ -95,6 +97,7 @@ try {
await runTask( testServer );
await runTask( testBuildTools );
await runTask( testWorkspaces );
await runTask( testApps );
} )();

await completeTasks( [ testClientTask, tscTasks, otherTestTasks ] );
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,10 @@
"test-desktop:e2e": "echo 'Deprecated, run `cd desktop && yarn run test:e2e` instead'",
"test-integration": "jest -c=test/integration/jest.config.js",
"test-integration:watch": "yarn run test-integration --watch",
"test-apps": "jest -c=test/apps/jest.config.js",
"test-apps:watch": "yarn run test-apps --watch",
"test-packages": "jest -c=test/packages/jest.config.js",
"test-packages:watch": "jest -c=test/packages/jest.config.js --watch",
"test-packages:watch": "yarn run test-packages --watch",
"test-server": "jest -c=test/server/jest.config.js",
"test-server:coverage": "yarn run test-server --coverage",
"test-server:watch": "yarn run test-server --watch",
Expand Down Expand Up @@ -377,7 +379,6 @@
"@wordpress/redux-routine": "4.23.0",
"@wordpress/reusable-blocks": "4.0.0",
"@wordpress/rich-text": "6.0.0",
"@wordpress/scripts": "25.0.0",
"@wordpress/server-side-render": "4.0.0",
"@wordpress/shortcode": "3.23.0",
"@wordpress/style-engine": "1.6.0",
Expand Down
14 changes: 14 additions & 0 deletions test/apps/jest-preset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const path = require( 'path' );
const base = require( '@automattic/calypso-jest' );

module.exports = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This copies a lot of things from test/apps/client and test/apps/packages. It doesn't strictly need to be separate, but probably useful to be more granular, as the projects are all deployed to different locations.

...base,
cacheDirectory: path.join( __dirname, '../../.cache/jest' ),
testEnvironment: 'jsdom',
transformIgnorePatterns: [
'node_modules[\\/\\\\](?!.*\\.(?:gif|jpg|jpeg|png|svg|scss|sass|css)$)',
],
setupFiles: [ 'jest-canvas-mock' ],
// This includes a lot of globals that don't exist, like fetch, matchMedia, etc.
setupFilesAfterEnv: [ require.resolve( '../client/setup-test-framework.js' ) ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth evaluating if we need all those mocks and polyfills or if we can include a more minimal version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looked into this, but we'd need to duplicate at least a few things from that file. From what I could tell, none of it was specific enough to the client setup to justify splitting it out.

I think it'd be good to keep this in mind in the future though. I personally like having this shared as much as possible, since there aren't big differences in the environments! Both the client and ETK run in a browser, after all.

};
5 changes: 5 additions & 0 deletions test/apps/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
rootDir: './../../',
// run tests for all packages that have a Jest config file
projects: [ '<rootDir>/apps/*/jest.config.js' ],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically same approach as packages.

};
Loading