From 33f0d3baa477a95990092ba6cabab8013bb28ed0 Mon Sep 17 00:00:00 2001 From: Grzegorz Ziolkowski Date: Sat, 27 Jan 2018 19:36:23 +0100 Subject: [PATCH 1/3] Scripts: Provide the default configuration for the `test` command It is used in the case when the project does not have a config for Jest or Babel --- lerna.json | 7 ++ package.json | 2 +- packages/scripts/README.md | 12 ++-- packages/scripts/bin/wp-scripts.js | 40 ++--------- packages/scripts/config/babel-transform.js | 8 +++ packages/scripts/config/jest.config.js | 27 ++++++++ packages/scripts/package.json | 9 ++- packages/scripts/scripts/test-script.js | 19 ----- packages/scripts/scripts/test-unit-jest.js | 39 +++++++++++ packages/scripts/scripts/test-unit.js | 1 + packages/scripts/utils/index.js | 81 ++++++++++++++++++++++ packages/scripts/utils/process.js | 7 ++ packages/scripts/utils/test/index.js | 54 +++++++++++++++ 13 files changed, 245 insertions(+), 61 deletions(-) create mode 100644 packages/scripts/config/babel-transform.js create mode 100644 packages/scripts/config/jest.config.js delete mode 100644 packages/scripts/scripts/test-script.js create mode 100644 packages/scripts/scripts/test-unit-jest.js create mode 100644 packages/scripts/scripts/test-unit.js create mode 100644 packages/scripts/utils/index.js create mode 100644 packages/scripts/utils/process.js create mode 100644 packages/scripts/utils/test/index.js diff --git a/lerna.json b/lerna.json index 1dd1138..cf4b459 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,12 @@ { "lerna": "2.8.0", + "commands": { + "publish": { + "ignore": [ + "**/test/*.js" + ] + } + }, "packages": [ "packages/*" ], diff --git a/package.json b/package.json index 4453076..3b56e4e 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "prebuild": "check-node-version --package", "build": "node ./scripts/build.js", "postinstall": "lerna bootstrap --hoist && ./scripts/create-symlinks.sh && npm run build", - "test": "wp-scripts test", + "test": "wp-scripts test-unit", "test:coverage": "npm run test -- --coverage", "test:coverage-ci": "npm run test -- --coverage && codecov", "test:watch": "npm run test -- --watch", diff --git a/packages/scripts/README.md b/packages/scripts/README.md index cd10065..97fd239 100644 --- a/packages/scripts/README.md +++ b/packages/scripts/README.md @@ -16,9 +16,9 @@ This is a CLI and exposes a binary called `wp-scripts` so you can call it direct ```json { "scripts": { - "test": "wp-scripts", - "test:help": "wp-scripts --help", - "test:watch": "wp-scripts --watch" + "test": "wp-scripts test-unit", + "test:help": "wp-scripts test-unit --help", + "test:watch": "wp-scripts test-unit --watch" } } ``` @@ -30,9 +30,11 @@ This is how you execute those scripts using the presented setup: ## Available Scripts -### `wp-scripts test` +### `wp-scripts test-unit` -Launches the test runner. It uses [Jest](https://facebook.github.io/jest/) behind the scenes and you are able to utilize all of its [CLI options](https://facebook.github.io/jest/docs/en/cli.html). You can also run `./node_modules/.bin/wp-scripts --help` or `npm run test:help` (if you use `package.json` setup shared above) to view all of the available options. +_Alias_: `wp-scripts test-unit-jest` + +Launches the test runner. It uses [Jest](https://facebook.github.io/jest/) behind the scenes and you are able to utilize all of its [CLI options](https://facebook.github.io/jest/docs/en/cli.html). You can also run `./node_modules/.bin/wp-scripts test-unit --help` or `npm run test:help` (if you use `package.json` setup shared above) to view all of the available options. ## Inspiration diff --git a/packages/scripts/bin/wp-scripts.js b/packages/scripts/bin/wp-scripts.js index d0a7ea8..05e9154 100755 --- a/packages/scripts/bin/wp-scripts.js +++ b/packages/scripts/bin/wp-scripts.js @@ -1,38 +1,10 @@ #!/usr/bin/env node -const spawn = require( 'cross-spawn' ); +/** + * Internal dependencies + */ +const { getCliArgs, spawnScript } = require( '../utils' ); -const allowedScripts = [ 'test' ]; -const [ scriptName, ...nodeArgs ] = process.argv.slice( 2 ); +const [ scriptName, ...nodesArgs ] = getCliArgs(); -if ( allowedScripts.indexOf( scriptName ) === -1 ) { - console.log( 'Unknown script "' + scriptName + '".' ); - console.log( 'Perhaps you need to update @wordpress/scripts?' ); - process.exit( 1 ); -} - -const result = spawn.sync( - 'node', - [ - require.resolve( `../scripts/${ scriptName }-script` ), - ...nodeArgs - ], - { stdio: 'inherit' } -); -if ( result.signal ) { - if ( result.signal === 'SIGKILL' ) { - console.log( - 'The build failed because the process exited too early. ' + - 'This probably means the system ran out of memory or someone called ' + - '`kill -9` on the process.' - ); - } else if ( result.signal === 'SIGTERM' ) { - console.log( - 'The build failed because the process exited too early. ' + - 'Someone might have called `kill` or `killall`, or the system could ' + - 'be shutting down.' - ); - } - process.exit( 1 ); -} -process.exit( result.status ); +spawnScript( scriptName, nodesArgs ); diff --git a/packages/scripts/config/babel-transform.js b/packages/scripts/config/babel-transform.js new file mode 100644 index 0000000..cf27415 --- /dev/null +++ b/packages/scripts/config/babel-transform.js @@ -0,0 +1,8 @@ +/** + * External dependencies + */ +const babelJest = require( 'babel-jest' ); + +module.exports = babelJest.createTransformer( { + presets: [ '@wordpress/default' ], +} ); diff --git a/packages/scripts/config/jest.config.js b/packages/scripts/config/jest.config.js new file mode 100644 index 0000000..41c8828 --- /dev/null +++ b/packages/scripts/config/jest.config.js @@ -0,0 +1,27 @@ +/** + * External dependencies + */ +const path = require( 'path' ); + +/** + * Internal dependencies + */ +const { + hasProjectFile, + hasPackageProp, +} = require( '../utils' ); + +const jestConfig = { + preset: '@wordpress/jest-preset-default' +}; + +const hasBabelConfig = hasProjectFile( '.babelrc' ) || + hasPackageProp( 'babel' ); + +if ( ! hasBabelConfig ) { + jestConfig.transform = { + '^.+\\.js$': path.join( __dirname, 'babel-transform' ), + }; +} + +module.exports = jestConfig; diff --git a/packages/scripts/package.json b/packages/scripts/package.json index a5d79b0..4c46696 100644 --- a/packages/scripts/package.json +++ b/packages/scripts/package.json @@ -23,14 +23,19 @@ }, "files": [ "bin", - "scripts" + "config", + "scripts", + "utils" ], "bin": { "wp-scripts": "./bin/wp-scripts.js" }, "dependencies": { + "@wordpress/babel-preset-default": "^1.0.2", + "@wordpress/jest-preset-default": "^1.0.0", "cross-spawn": "^5.1.0", - "jest": "^22.0.4" + "jest": "^22.0.4", + "read-pkg-up": "^3.0.0" }, "publishConfig": { "access": "public" diff --git a/packages/scripts/scripts/test-script.js b/packages/scripts/scripts/test-script.js deleted file mode 100644 index 6f3f9ab..0000000 --- a/packages/scripts/scripts/test-script.js +++ /dev/null @@ -1,19 +0,0 @@ -// Do this as the first thing so that any code reading it knows the right env. -process.env.BABEL_ENV = 'test'; -process.env.NODE_ENV = 'test'; - -// Makes the script crash on unhandled rejections instead of silently -// ignoring them. In the future, promise rejections that are not handled will -// terminate the Node.js process with a non-zero exit code. -process.on( 'unhandledRejection', err => { - throw err; -} ); - -/** - * External dependencies - */ -const jest = require( 'jest' ); - -const argv = process.argv.slice( 2 ); - -jest.run( argv ); diff --git a/packages/scripts/scripts/test-unit-jest.js b/packages/scripts/scripts/test-unit-jest.js new file mode 100644 index 0000000..19891b4 --- /dev/null +++ b/packages/scripts/scripts/test-unit-jest.js @@ -0,0 +1,39 @@ +// Do this as the first thing so that any code reading it knows the right env. +process.env.BABEL_ENV = 'test'; +process.env.NODE_ENV = 'test'; + +// Makes the script crash on unhandled rejections instead of silently +// ignoring them. In the future, promise rejections that are not handled will +// terminate the Node.js process with a non-zero exit code. +process.on( 'unhandledRejection', err => { + throw err; +} ); + +/** + * External dependencies + */ +const jest = require( 'jest' ); + +/** + * Internal dependencies + */ +const { + getCliArgs, + hasCliArg, + hasProjectFile, + hasPackageProp, +} = require( '../utils' ); + +const args = getCliArgs(); + +const hasJestConfig = hasCliArg( '-c' ) || + hasCliArg( '--config' ) || + hasProjectFile( 'jest.config.js' ) || + hasProjectFile( 'jest.config.json' ) || + hasPackageProp( 'jest' ); + +const config = ! hasJestConfig + ? [ '--config', JSON.stringify( require( '../config/jest.config' ) ) ] + : []; + +jest.run( [ ...config, ...args ] ); diff --git a/packages/scripts/scripts/test-unit.js b/packages/scripts/scripts/test-unit.js new file mode 100644 index 0000000..04cde53 --- /dev/null +++ b/packages/scripts/scripts/test-unit.js @@ -0,0 +1 @@ +require( './test-unit-jest' ); diff --git a/packages/scripts/utils/index.js b/packages/scripts/utils/index.js new file mode 100644 index 0000000..2c53158 --- /dev/null +++ b/packages/scripts/utils/index.js @@ -0,0 +1,81 @@ +/** + * External dependencies + */ +const spawn = require( 'cross-spawn' ); +const { existsSync, realpathSync } = require( 'fs' ); +const path = require( 'path' ); +const readPkgUp = require( 'read-pkg-up' ); + +/** + * Internal dependencies + */ +const { exit, getCliArgs, getCurrentWorkingDirectory } = require( './process' ); + +const first = list => list[ 0 ]; + +const hasCliArg = ( arg ) => getCliArgs() + .some( ( value ) => first( value.split( '=' ) ) === arg ); + +const { pkg, path: pkgPath } = readPkgUp.sync( { + cwd: realpathSync( getCurrentWorkingDirectory() ), +} ); + +const appDirectory = path.dirname( pkgPath ); + +const fromProjectRoot = ( fileName ) => path.join( appDirectory, fileName ); + +const hasProjectFile = ( fileName ) => existsSync( fromProjectRoot( fileName ) ); + +const fromScriptsRoot = ( scriptName ) => path.join( path.dirname( __dirname ), 'scripts', `${ scriptName }.js` ); + +const hasScriptFile = ( scriptName ) => existsSync( fromScriptsRoot( scriptName ) ); + +const hasPackageProp = ( prop ) => pkg && pkg.hasOwnProperty( prop ); + +const handleSignal = ( signal ) => { + if ( signal === 'SIGKILL' ) { + console.log( + 'The build failed because the process exited too early. ' + + 'This probably means the system ran out of memory or someone called ' + + '`kill -9` on the process.' + ); + } else if ( signal === 'SIGTERM' ) { + console.log( + 'The build failed because the process exited too early. ' + + 'Someone might have called `kill` or `killall`, or the system could ' + + 'be shutting down.' + ); + } + exit( 1 ); +}; + +const spawnScript = ( scriptName, args ) => { + if ( ! hasScriptFile( scriptName ) ) { + console.log( 'Unknown script "' + scriptName + '".' ); + console.log( 'Perhaps you need to update @wordpress/scripts?' ); + exit( 1 ); + } + + const { signal, status } = spawn.sync( + 'node', + [ + fromScriptsRoot( scriptName ), + ...args + ], + { stdio: 'inherit' } + ); + + if ( signal ) { + handleSignal( signal ); + } + + exit( status ); +}; + +module.exports = { + getCliArgs, + hasCliArg, + hasProjectFile, + hasPackageProp, + spawnScript, +}; diff --git a/packages/scripts/utils/process.js b/packages/scripts/utils/process.js new file mode 100644 index 0000000..f8d3a0f --- /dev/null +++ b/packages/scripts/utils/process.js @@ -0,0 +1,7 @@ +const getCliArgs = () => process.argv.slice( 2 ); + +module.exports = { + exit: process.exit, + getCliArgs, + getCurrentWorkingDirectory: process.cwd, +}; diff --git a/packages/scripts/utils/test/index.js b/packages/scripts/utils/test/index.js new file mode 100644 index 0000000..3993ebf --- /dev/null +++ b/packages/scripts/utils/test/index.js @@ -0,0 +1,54 @@ +jest.mock( '../process', () => { + const process = require.requireActual( '../process' ); + + return Object.keys( process ).reduce( + ( accumulator, methodName ) => ( { + ...accumulator, + [ methodName ]: jest.spyOn( process, methodName ), + } ), + {}, + ); +} ); + +/** + * Internal dependencies + */ +import { + getCliArgs, + hasCliArg, +} from '../'; +import { getCliArgs as getCliArgsMock } from '../process'; + +describe( 'utils', () => { + describe( 'getCliArgs', () => { + test( 'should have function defined', () => { + expect( getCliArgs() ).toBeDefined(); + } ); + } ); + + describe( 'hasCliArg', () => { + beforeAll( () => { + getCliArgsMock.mockReturnValue( [ '-a', '--b', '--config=test' ] ); + } ); + + afterAll( () => { + getCliArgsMock.mockReset(); + } ); + + test( 'should return false when no args passed', () => { + getCliArgsMock.mockReturnValueOnce( [] ); + + expect( hasCliArg( '--no-args' ) ).toBe( false ); + } ); + + test( 'should return false when checking for unrecognized arg', () => { + expect( hasCliArg( '--non-existent' ) ).toBe( false ); + } ); + + test( 'should return true when CLI arg found', () => { + expect( hasCliArg( '-a' ) ).toBe( true ); + expect( hasCliArg( '--b' ) ).toBe( true ); + expect( hasCliArg( '--config' ) ).toBe( true ); + } ); + } ); +} ); From cf7900ca1d6dc2835fb6d425a530b39e1491ea73 Mon Sep 17 00:00:00 2001 From: Grzegorz Ziolkowski Date: Thu, 15 Feb 2018 09:49:42 +0100 Subject: [PATCH 2/3] Scripts: Add tests for utils to ensure they work properly --- packages/scripts/utils/index.js | 39 +++++----- packages/scripts/utils/package.js | 23 ++++++ packages/scripts/utils/test/index.js | 107 +++++++++++++++++++++++---- 3 files changed, 134 insertions(+), 35 deletions(-) create mode 100644 packages/scripts/utils/package.js diff --git a/packages/scripts/utils/index.js b/packages/scripts/utils/index.js index 2c53158..791341a 100644 --- a/packages/scripts/utils/index.js +++ b/packages/scripts/utils/index.js @@ -2,46 +2,42 @@ * External dependencies */ const spawn = require( 'cross-spawn' ); -const { existsSync, realpathSync } = require( 'fs' ); +const { existsSync } = require( 'fs' ); const path = require( 'path' ); -const readPkgUp = require( 'read-pkg-up' ); /** * Internal dependencies */ -const { exit, getCliArgs, getCurrentWorkingDirectory } = require( './process' ); +const { getPackagePath, hasPackageProp } = require( './package' ); +const { exit, getCliArgs } = require( './process' ); const first = list => list[ 0 ]; const hasCliArg = ( arg ) => getCliArgs() .some( ( value ) => first( value.split( '=' ) ) === arg ); -const { pkg, path: pkgPath } = readPkgUp.sync( { - cwd: realpathSync( getCurrentWorkingDirectory() ), -} ); +const fromProjectRoot = ( fileName ) => + path.join( path.dirname( getPackagePath() ), fileName ); -const appDirectory = path.dirname( pkgPath ); +const hasProjectFile = ( fileName ) => + existsSync( fromProjectRoot( fileName ) ); -const fromProjectRoot = ( fileName ) => path.join( appDirectory, fileName ); +const fromScriptsRoot = ( scriptName ) => + path.join( path.dirname( __dirname ), 'scripts', `${ scriptName }.js` ); -const hasProjectFile = ( fileName ) => existsSync( fromProjectRoot( fileName ) ); - -const fromScriptsRoot = ( scriptName ) => path.join( path.dirname( __dirname ), 'scripts', `${ scriptName }.js` ); - -const hasScriptFile = ( scriptName ) => existsSync( fromScriptsRoot( scriptName ) ); - -const hasPackageProp = ( prop ) => pkg && pkg.hasOwnProperty( prop ); +const hasScriptFile = ( scriptName ) => + existsSync( fromScriptsRoot( scriptName ) ); const handleSignal = ( signal ) => { if ( signal === 'SIGKILL' ) { console.log( - 'The build failed because the process exited too early. ' + + 'The script failed because the process exited too early. ' + 'This probably means the system ran out of memory or someone called ' + '`kill -9` on the process.' ); } else if ( signal === 'SIGTERM' ) { console.log( - 'The build failed because the process exited too early. ' + + 'The script failed because the process exited too early. ' + 'Someone might have called `kill` or `killall`, or the system could ' + 'be shutting down.' ); @@ -49,7 +45,12 @@ const handleSignal = ( signal ) => { exit( 1 ); }; -const spawnScript = ( scriptName, args ) => { +const spawnScript = ( scriptName, args = [] ) => { + if ( ! scriptName ) { + console.log( 'Script name is missing.' ); + exit( 1 ); + } + if ( ! hasScriptFile( scriptName ) ) { console.log( 'Unknown script "' + scriptName + '".' ); console.log( 'Perhaps you need to update @wordpress/scripts?' ); @@ -62,7 +63,7 @@ const spawnScript = ( scriptName, args ) => { fromScriptsRoot( scriptName ), ...args ], - { stdio: 'inherit' } + { stdio: 'inherit' }, ); if ( signal ) { diff --git a/packages/scripts/utils/package.js b/packages/scripts/utils/package.js new file mode 100644 index 0000000..88e90aa --- /dev/null +++ b/packages/scripts/utils/package.js @@ -0,0 +1,23 @@ +/** + * External dependencies + */ +const { realpathSync } = require( 'fs' ); +const readPkgUp = require( 'read-pkg-up' ); + +/** + * Internal dependencies + */ +const { getCurrentWorkingDirectory } = require( './process' ); + +const { pkg, path: pkgPath } = readPkgUp.sync( { + cwd: realpathSync( getCurrentWorkingDirectory() ), +} ); + +const getPackagePath = () => pkgPath; + +const hasPackageProp = ( prop ) => pkg && pkg.hasOwnProperty( prop ); + +module.exports = { + getPackagePath, + hasPackageProp, +}; diff --git a/packages/scripts/utils/test/index.js b/packages/scripts/utils/test/index.js index 3993ebf..57e5cbf 100644 --- a/packages/scripts/utils/test/index.js +++ b/packages/scripts/utils/test/index.js @@ -1,30 +1,38 @@ -jest.mock( '../process', () => { - const process = require.requireActual( '../process' ); - - return Object.keys( process ).reduce( - ( accumulator, methodName ) => ( { - ...accumulator, - [ methodName ]: jest.spyOn( process, methodName ), - } ), - {}, - ); +jest.mock( '../package', () => { + const module = require.requireActual( '../package' ); + + jest.spyOn( module, 'getPackagePath' ); + + return module; } ); +jest.mock( '../process', () => { + const module = require.requireActual( '../process' ); + + jest.spyOn( module, 'exit' ); + jest.spyOn( module, 'getCliArgs' ); + return module; +} ); /** * Internal dependencies */ +import crossSpawn from 'cross-spawn'; import { getCliArgs, hasCliArg, + hasProjectFile, + spawnScript, } from '../'; -import { getCliArgs as getCliArgsMock } from '../process'; +import { + getPackagePath as getPackagePathMock, +} from '../package'; +import { + exit as exitMock, + getCliArgs as getCliArgsMock, +} from '../process'; describe( 'utils', () => { - describe( 'getCliArgs', () => { - test( 'should have function defined', () => { - expect( getCliArgs() ).toBeDefined(); - } ); - } ); + const crossSpawnMock = jest.spyOn( crossSpawn, 'sync' ); describe( 'hasCliArg', () => { beforeAll( () => { @@ -51,4 +59,71 @@ describe( 'utils', () => { expect( hasCliArg( '--config' ) ).toBe( true ); } ); } ); + + describe( 'hasProjectFile', () => { + test( 'should return false for the current directory and unknown file', () => { + getPackagePathMock.mockReturnValueOnce( __dirname ); + + expect( hasProjectFile( 'unknown-file.name' ) ).toBe( false ); + } ); + + test( 'should return true for the current directory and this file', () => { + getPackagePathMock.mockReturnValueOnce( __dirname ); + + expect( hasProjectFile( 'index.js' ) ).toBe( true ); + } ); + } ); + + describe( 'spawnScript', () => { + const scriptName = 'test-unit'; + + beforeAll( () => { + exitMock.mockImplementation( ( code ) => { + throw new Error( `Exit code: ${ code }.` ) + } ); + } ); + + afterAll( () => { + exitMock.mockReset(); + } ); + + test( 'should exit when no script name provided', () => { + expect( () => spawnScript() ).toThrow( 'Exit code: 1.' ); + } ); + + test( 'should exit when an unknown script name provided', () => { + expect( () => spawnScript( 'unknown-script' ) ).toThrow( 'Exit code: 1.' ); + } ); + + test( 'should exit when the script failed because of SIGKILL signal', () => { + crossSpawnMock.mockReturnValueOnce( { signal: 'SIGKILL' } ); + + expect( () => spawnScript( scriptName ) ).toThrow( 'Exit code: 1.' ); + } ); + + test( 'should exit when the script failed because of SIGTERM signal', () => { + crossSpawnMock.mockReturnValueOnce( { signal: 'SIGTERM' } ); + + expect( () => spawnScript( scriptName ) ).toThrow( 'Exit code: 1.' ); + } ); + + test( 'should finish successfully when the script properly executed', () => { + crossSpawnMock.mockReturnValueOnce( { status: 0 } ); + + expect( () => spawnScript( scriptName ) ).toThrow( 'Exit code: 0.' ); + expect( crossSpawnMock ).toHaveBeenCalledWith( + 'node', [ expect.stringContaining( scriptName ) ], { stdio: 'inherit' } + ); + } ); + + test( 'should finish successfully when the script properly executed with args', () => { + crossSpawnMock.mockReturnValueOnce( { status: 0 } ); + const args = [ '-a', '--bbb', '-c=ccccc' ]; + + expect( () => spawnScript( scriptName, args ) ).toThrow( 'Exit code: 0.' ); + expect( crossSpawnMock ).toHaveBeenCalledWith( + 'node', [ expect.stringContaining( scriptName ), ...args ], { stdio: 'inherit' } + ); + } ); + } ); } ); From a925568ba32a2239556020fe89285742f95497ae Mon Sep 17 00:00:00 2001 From: Stephen Edgar Date: Tue, 20 Feb 2018 10:57:30 +1100 Subject: [PATCH 3/3] Add back end-of-file new line --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index ab7854f..20f3a11 100644 --- a/lerna.json +++ b/lerna.json @@ -11,4 +11,4 @@ "packages/*" ], "version": "independent" -} \ No newline at end of file +}