From fab71524f17bf0e3e2239f3a10ed11420619c341 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Mon, 2 May 2016 17:59:06 -0700 Subject: [PATCH 1/2] SELA-301 Support Mocha without Karma as a test runner. --- package.json | 1 + src/cli.js | 14 +++++++-- src/commands/test.js | 27 +++++++++++++---- src/config/karma-config.js | 2 +- src/lib/testHelperKarma.js | 9 ++++++ src/lib/testHelperMocha.js | 29 +++++++++++++++++++ .../{testHelper.js => testHelperShared.js} | 8 ----- 7 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 src/lib/testHelperKarma.js create mode 100644 src/lib/testHelperMocha.js rename src/lib/{testHelper.js => testHelperShared.js} (50%) diff --git a/package.json b/package.json index b7ac510fb..980a9c475 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "babel-eslint": "6.0.4", "eslint": "2.9.0", "eslint-plugin-react": "5.0.1", + "jsdom": "8.5.0", "temp": "0.8.3" } } diff --git a/src/cli.js b/src/cli.js index ade0741d6..63c772080 100755 --- a/src/cli.js +++ b/src/cli.js @@ -72,14 +72,20 @@ const debugOption = { description: "debug server side rendering with node-inspector" }; +const nodeTestOption = { + command: "-n, --mocha-only", + description: "run tests in Node.js" +}; + commander .command("start") .description("start everything") .option("-T, --no_tests", "ignore test hook") .option(debugOption.command, debugOption.description) + .option(nodeTestOption.command, nodeTestOption.description) .action(checkGluestickProject) .action(() => notifyUpdates()) - .action((options) => startAll(options.no_tests, options.debug)) + .action((options) => startAll(options.no_tests, options.debug, options.mochaOnly)) .action(() => updateLastVersionUsed(currentGluestickVersion)); commander @@ -126,6 +132,7 @@ commander .command("start-test", null, {noHelp: true}) .option(firefoxOption.command, firefoxOption.description) .option(singleRunOption.command, singleRunOption.description) + .option(nodeTestOption.command, nodeTestOption.description) .description("start test") .action(checkGluestickProject) .action((options) => startTest(options)) @@ -135,6 +142,7 @@ commander .command("test") .option(firefoxOption.command, firefoxOption.description) .option(singleRunOption.command, singleRunOption.description) + .option(nodeTestOption.command, nodeTestOption.description) .description("start tests") .action(checkGluestickProject) .action(() => updateLastVersionUsed(currentGluestickVersion)) @@ -193,7 +201,7 @@ function spawnProcess (type, args=[]) { return childProcess; } -async function startAll(withoutTests=false, debug=false) { +async function startAll(withoutTests=false, debug=false, mochaOnly=false) { try { await autoUpgrade(); } @@ -207,7 +215,7 @@ async function startAll(withoutTests=false, debug=false) { // Start tests unless they asked us not to or we are in production mode if (!isProduction && !withoutTests) { - spawnProcess("test"); + spawnProcess("test", (mochaOnly ? ["--mocha-only"] : [])); } } diff --git a/src/commands/test.js b/src/commands/test.js index 0be361aab..4a255f858 100644 --- a/src/commands/test.js +++ b/src/commands/test.js @@ -2,23 +2,38 @@ const karma = require("karma"); const Server = karma.Server; const runner = karma.runner; const karmaConfig = require("../config/karma-config").default; - -const config = karmaConfig; +const spawn = require("cross-spawn").spawn; +const mocha = require("mocha"); +const path = require("path"); +const CWD = process.cwd(); module.exports = function (options) { + // override to run tests in Node without Karma/Webpack + if (options.mochaOnly) { + const helperPath = path.join(__dirname, "..", "lib", "testHelperMocha.js"); + const testPath = `${CWD}/test/**/*.js`; + const mochaEnv = Object.assign({}, process.env, { NODE_PATH: 'src', NODE_ENV: 'test'}); + spawn( + path.join(__dirname, "..", "..", "node_modules", ".bin", "mocha"), + ["--require", helperPath, "--reporter", "dot", `${testPath}`, "--watch"], + { stdio: "inherit", env: mochaEnv } + ); + return; + } + // override browser setting to use firefox instead of Chrome if specified if (options.firefox) { - config.browsers = ["Firefox"]; + karmaConfig.browsers = ["Firefox"]; } if (options.single) { - config.singleRun = options.single; + karmaConfig.singleRun = options.single; } - const server = new Server(config); + const server = new Server(karmaConfig); server.start(); server.on("browsers_ready", function () { - runner.run(config, () => {}); + runner.run(karmaConfig, () => {}); }); }; diff --git a/src/config/karma-config.js b/src/config/karma-config.js index c76b0a7d6..6d93521bc 100644 --- a/src/config/karma-config.js +++ b/src/config/karma-config.js @@ -12,7 +12,7 @@ const CWD = process.cwd(); const webpackIsomorphicToolsPlugin = new WebpackIsomorphicToolsPlugin(require("./webpack-isomorphic-tools-config")).development(true); const preprocessors = {}; -const helperPath = path.resolve(__dirname, "../lib/testHelper.js"); +const helperPath = path.resolve(__dirname, "../lib/testHelperKarma.js"); preprocessors[helperPath] = ["webpack", "sourcemap"]; export default { diff --git a/src/lib/testHelperKarma.js b/src/lib/testHelperKarma.js new file mode 100644 index 000000000..94d78af3b --- /dev/null +++ b/src/lib/testHelperKarma.js @@ -0,0 +1,9 @@ +/* global TEST_PATH */ +/* global SRC_PATH */ +require('./testHelperShared'); + +const context = require.context(TEST_PATH, true, /\.test\.js$/); +context.keys().forEach(context); + +const srcContext = require.context(SRC_PATH, true, /\.js$/); +srcContext.keys().forEach(srcContext); diff --git a/src/lib/testHelperMocha.js b/src/lib/testHelperMocha.js new file mode 100644 index 000000000..f31248624 --- /dev/null +++ b/src/lib/testHelperMocha.js @@ -0,0 +1,29 @@ +/* global global */ +require('babel-register'); + +function noop() { + return null; +} + +const jsdom = require("jsdom").jsdom; + +global.document = jsdom(""); +global.window = document.defaultView; +Object.keys(document.defaultView).forEach((property) => { + if (typeof global[property] === "undefined") { + global[property] = document.defaultView[property]; + } +}); + +global.navigator = { + userAgent: "node.js" +}; + +require.extensions[".scss"] = noop; +require.extensions[".png"] = noop; +require.extensions[".svg"] = noop; + +require('./testHelperShared'); + +global.expect = require("chai").expect; +global.sinon = require("sinon"); diff --git a/src/lib/testHelper.js b/src/lib/testHelperShared.js similarity index 50% rename from src/lib/testHelper.js rename to src/lib/testHelperShared.js index 01999bf18..ed6ada476 100644 --- a/src/lib/testHelper.js +++ b/src/lib/testHelperShared.js @@ -1,5 +1,3 @@ -/*global TEST_PATH*/ -/*global SRC_PATH*/ require("babel-polyfill"); const React = require("react"); const ReactDOM = require("react-dom"); @@ -7,9 +5,3 @@ const ReactTestUtils = require("react-addons-test-utils"); global.React = React; global.ReactDOM = ReactDOM; global.TestUtils = ReactTestUtils; - -const context = require.context(TEST_PATH, true, /\.test\.js$/); -context.keys().forEach(context); - -const srcContext = require.context(SRC_PATH, true, /\.js$/); -srcContext.keys().forEach(srcContext); From c75355a2c4dd23c546d5ec357a3ebc10003ff9d9 Mon Sep 17 00:00:00 2001 From: David Edmondson Date: Tue, 3 May 2016 11:50:47 -0700 Subject: [PATCH 2/2] SELA-301 Support multiple aliases for src + assets. Filter to only run .test.js files. Add lots of comments around the wackier bits. --- .eslintrc | 63 ++++++++++++++++++++++++++++++++++++++ package.json | 3 +- src/commands/test.js | 12 ++++++-- src/lib/testHelperKarma.js | 2 +- src/lib/testHelperMocha.js | 40 +++++++++++++++++++----- 5 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 .eslintrc diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 000000000..b74fb7592 --- /dev/null +++ b/.eslintrc @@ -0,0 +1,63 @@ +{ + "parser": "babel-eslint", + "rules": { + "comma-dangle": 0, + "no-cond-assign": [2, "always"], + "no-extra-boolean-cast": 0, + + "curly": 2, + "default-case": 2, + "eqeqeq": 2, + "no-case-declarations": 2, + "no-else-return": 2, + "no-fallthrough": 2, + "no-redeclare": 2, + "no-warning-comments": [1, { "terms": ["todo"], "location": "start" }], + + "no-undef": 2, + "no-undef-init": 2, + "no-undefined": 2, + "no-unused-vars": 2, + + "eol-last": 2, + "indent": [2, 2, {"SwitchCase": 1, "VariableDeclarator": { "var": 2, "let": 2, "const": 3}}], + "jsx-quotes": [2, "prefer-double"], + "linebreak-style": [2, "unix"], + "no-trailing-spaces": 1, + "quotes": [2, "double"], + "semi": [2, "always"], + + "no-var": 2, + "prefer-const": 2, + + "react/display-name": 0 + }, + "env": { + "es6": true, + "node": true, + "browser": true + }, + "extends": ["eslint:recommended", "plugin:react/recommended"], + "ecmaFeatures": { + "jsx": true, + "experimentalObjectRestSpread": true + }, + "plugins": [ + "react" + ], + "globals": { + "document": true, + "React": true, + "ReactDOM": true, + "window": true, + "describe": true, + "expect": true, + "it": true, + "TestUtils": true, + "sinon": true, + "beforeEach": true, + "afterEach": true, + "before": true, + "after": true + } +} diff --git a/package.json b/package.json index 980a9c475..f4d37878e 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "bugs": "https://github.com/TrueCar/gluestick/issues", "license": "MIT", "scripts": { - "lint": "eslint src test templates", + "lint": "./node_modules/.bin/eslint src test templates", "test": "mocha --compilers js:babel-core/register --require babel-polyfill --recursive", "test-coverage": "./node_modules/.bin/babel-node ./node_modules/babel-istanbul/lib/cli.js cover --include-all-sources --report html --dir coverage/html ./node_modules/.bin/_mocha -- --recursive", "prepublish": "./docker/generate-dockerfile.js", @@ -98,6 +98,7 @@ }, "peerDependencies": {}, "devDependencies": { + "app-module-path": "1.0.6", "babel-eslint": "6.0.4", "eslint": "2.9.0", "eslint-plugin-react": "5.0.1", diff --git a/src/commands/test.js b/src/commands/test.js index 4a255f858..5d8929432 100644 --- a/src/commands/test.js +++ b/src/commands/test.js @@ -3,7 +3,6 @@ const Server = karma.Server; const runner = karma.runner; const karmaConfig = require("../config/karma-config").default; const spawn = require("cross-spawn").spawn; -const mocha = require("mocha"); const path = require("path"); const CWD = process.cwd(); @@ -11,8 +10,15 @@ module.exports = function (options) { // override to run tests in Node without Karma/Webpack if (options.mochaOnly) { const helperPath = path.join(__dirname, "..", "lib", "testHelperMocha.js"); - const testPath = `${CWD}/test/**/*.js`; - const mochaEnv = Object.assign({}, process.env, { NODE_PATH: 'src', NODE_ENV: 'test'}); + const testPath = `${CWD}/test/**/*.test.js`; + + // In order to support the Webpack `assets` alias and `src` root, the node path needs to be set up to be + // at the base directory. This allows all module resolves to be relative to the project, rather than to + // Gluestick's CWD. + const mochaEnv = Object.assign({}, process.env, { NODE_PATH: CWD, NODE_ENV: "test" }); + + // Currently defaulting to "dot" reporter and watch. This can be improved to allow a reporter override + // and single-run. spawn( path.join(__dirname, "..", "..", "node_modules", ".bin", "mocha"), ["--require", helperPath, "--reporter", "dot", `${testPath}`, "--watch"], diff --git a/src/lib/testHelperKarma.js b/src/lib/testHelperKarma.js index 94d78af3b..1ccc10569 100644 --- a/src/lib/testHelperKarma.js +++ b/src/lib/testHelperKarma.js @@ -1,6 +1,6 @@ /* global TEST_PATH */ /* global SRC_PATH */ -require('./testHelperShared'); +require("./testHelperShared"); const context = require.context(TEST_PATH, true, /\.test\.js$/); context.keys().forEach(context); diff --git a/src/lib/testHelperMocha.js b/src/lib/testHelperMocha.js index f31248624..4487a39a7 100644 --- a/src/lib/testHelperMocha.js +++ b/src/lib/testHelperMocha.js @@ -1,10 +1,23 @@ /* global global */ -require('babel-register'); -function noop() { - return null; -} +const CWD = process.cwd(); +const IS_WINDOWS = process.platform === "win32"; + +// Because we have multiple resolves and an alias set up in Webpack, we need to modify require.main.paths at runtime to +// deal with that without the use of a bundler. Unlike require.paths, this is documented and should be safe. However, +// modifying it after anything has been require()d causes cache issues. +// +// Therefore: +// **ACHTUNG** THIS SHOULD ALWAYS BE RUN BEFORE ANY OTHER REQUIRE()s. +// +// Cannot use path.join here since it would take a require()! +require("app-module-path").addPath(CWD + (IS_WINDOWS ? "'" : "/") + "assets"); +require("app-module-path").addPath(CWD + (IS_WINDOWS ? "'" : "/") + "src"); + +require("babel-register"); +// Set up jsdom for component rendering through Enzyme. +// This code is directly from Enzyme's docs. const jsdom = require("jsdom").jsdom; global.document = jsdom(""); @@ -19,11 +32,22 @@ global.navigator = { userAgent: "node.js" }; -require.extensions[".scss"] = noop; -require.extensions[".png"] = noop; -require.extensions[".svg"] = noop; +function noop() { + return null; +} + +// Note: require.extensions is deprecated, but is currently the only way to filter out require() of non-JS modules +// without the expense and complication of a pre-compilation step. +// +// From the docs: +// +// "Since the Module system is locked, this feature will probably never go away. However, it may have subtle bugs +// and complexities that are best left untouched." +[".css", ".jpg", ".png", ".scss", ".svg"].forEach((extension) => { + require.extensions[extension] = noop; +}); -require('./testHelperShared'); +require("./testHelperShared"); global.expect = require("chai").expect; global.sinon = require("sinon");