From c6c4657f83264525ee4f510c2417ae58a7e72a4e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 15 Jul 2013 11:34:47 -0400 Subject: [PATCH 1/5] Use populist v0.1.2 to bundle test modules instead of browserify. This will allow full support for mocking, dumpCache, and correct line numbers in error messages. --- Gruntfile.js | 6 +++++- grunt/config/populist.js | 13 +++++++++++++ grunt/tasks/populist.js | 28 ++++++++++++++++++++++++++++ package.json | 1 + src/test/all.js | 3 +++ 5 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 grunt/config/populist.js create mode 100644 grunt/tasks/populist.js diff --git a/Gruntfile.js b/Gruntfile.js index dd04aa2d139c..fad8b47971b3 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -4,6 +4,7 @@ var exec = require('child_process').exec; var jsxTask = require('./grunt/tasks/jsx'); var browserifyTask = require('./grunt/tasks/browserify'); var wrapupTask = require('./grunt/tasks/wrapup'); +var populistTask = require('./grunt/tasks/populist'); var phantomTask = require('./grunt/tasks/phantom'); var npmTask = require('./grunt/tasks/npm'); var releaseTasks = require('./grunt/tasks/release'); @@ -16,6 +17,7 @@ module.exports = function(grunt) { jsx: require('./grunt/config/jsx/jsx'), browserify: require('./grunt/config/browserify'), wrapup: require('./grunt/config/wrapup'), + populist: require('./grunt/config/populist'), phantom: require('./grunt/config/phantom'), npm: require('./grunt/config/npm'), clean: ['./build', './*.gem', './docs/_site', './examples/shared/*.js'], @@ -44,6 +46,8 @@ module.exports = function(grunt) { // defines global variables instead of using require. grunt.registerMultiTask('wrapup', wrapupTask); + grunt.registerMultiTask('populist', populistTask); + grunt.registerMultiTask('phantom', phantomTask); grunt.registerMultiTask('npm', npmTask); @@ -56,7 +60,7 @@ module.exports = function(grunt) { 'jsx:jasmine', 'jsx:test', 'browserify:jasmine', - 'browserify:test' + 'populist:test' ]); grunt.registerTask('test', ['build:test', 'phantom:run']); diff --git a/grunt/config/populist.js b/grunt/config/populist.js new file mode 100644 index 000000000000..2f353ac3d1c3 --- /dev/null +++ b/grunt/config/populist.js @@ -0,0 +1,13 @@ +'use strict'; + +var test = { + args: ["test/all:"], + requires: [ + "**/__tests__/*-test.js" + ], + outfile: './build/react-test.js' +}; + +module.exports = { + test: test +}; diff --git a/grunt/tasks/populist.js b/grunt/tasks/populist.js new file mode 100644 index 000000000000..f89f45ccef5f --- /dev/null +++ b/grunt/tasks/populist.js @@ -0,0 +1,28 @@ +'use strict'; + +var grunt = require('grunt'); + +module.exports = function() { + var config = this.data; + var done = this.async(); + + // create the bundle we'll work with + var args = config.args; + + // Make sure the things that need to be exposed are. + var requires = config.requires || {}; + grunt.file.expand({ + nonull: true, // Keep IDs that don't expand to anything. + cwd: "src" + }, requires).forEach(function(name) { + args.push(name.replace(/\.js$/i, "")); + }); + + require("populist").buildP({ + rootDirectory: "build/modules", + args: args + }).then(function(output) { + grunt.file.write(config.outfile, output); + done(); + }); +}; diff --git a/package.json b/package.json index 802e3e82689e..d35a9af1b830 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "devDependencies": { "browserify": "~2.24.1", "wrapup": "~0.12.0", + "populist": "~0.1.2", "grunt-cli": "~0.1.9", "grunt": "~0.4.1", "grunt-contrib-copy": "~0.4.1", diff --git a/src/test/all.js b/src/test/all.js index b370c31b629f..3b3a3639a9de 100644 --- a/src/test/all.js +++ b/src/test/all.js @@ -6,6 +6,9 @@ var Ap = Array.prototype; var slice = Ap.slice; var Fp = Function.prototype; +var global = Function("return this")(); +global.require = require; + if (!Fp.bind) { // PhantomJS doesn't support Function.prototype.bind natively, so // polyfill it whenever this module is required. From 37014e1002a0fd6bcb32f71482e231705b5bb872 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 3 Jul 2013 11:37:44 -0400 Subject: [PATCH 2/5] Call require("mock-modules").register in every mockable module. Mocking happens only when config.constants.__MOCK__ is true. --- bin/jsx | 20 +++++++++++++++++++- grunt/config/jsx/debug.json | 1 + src/test/mock-modules.js | 8 ++++++-- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/bin/jsx b/bin/jsx index 2a6437d535b3..76fe4aa93260 100755 --- a/bin/jsx +++ b/bin/jsx @@ -32,5 +32,23 @@ require("commoner").resolve(function(id) { // Constant propagation means removing any obviously dead code after // replacing constant expressions with literal (boolean) values. - return propagate(constants, source); + source = propagate(constants, source); + + if (constants.__MOCK__) { + // Make sure there is exactly one newline at the end of the module. + source = source.replace(/\s+$/m, "\n"); + + return context.getProvidedP().then(function(idToPath) { + if (id !== "mock-modules" && + id !== "mocks" && + idToPath.hasOwnProperty("mock-modules")) { + return source + '\nrequire("mock-modules").register(' + + JSON.stringify(id) + ', module);\n'; + } + + return source; + }); + } + + return source; }); diff --git a/grunt/config/jsx/debug.json b/grunt/config/jsx/debug.json index 1596c562e857..d4099010b2f1 100644 --- a/grunt/config/jsx/debug.json +++ b/grunt/config/jsx/debug.json @@ -1,6 +1,7 @@ { "debug": true, "constants": { + "__MOCK__": true, "__DEV__": true } } diff --git a/src/test/mock-modules.js b/src/test/mock-modules.js index 6c0efd95f870..52d0393b8bb0 100644 --- a/src/test/mock-modules.js +++ b/src/test/mock-modules.js @@ -16,8 +16,9 @@ * @providesModule mock-modules */ -var global = Function("return this")(); -require('test/mock-timers').installMockTimers(global); +exports.register = function(id, module) { + // TODO +}; exports.dumpCache = function() { require("mocks").clear(); @@ -31,3 +32,6 @@ exports.dontMock = function() { exports.mock = function() { return exports; }; + +var global = Function("return this")(); +require('test/mock-timers').installMockTimers(global); From 204796868d377acbcbbb1e2de4698b8827dc6b83 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 3 Jul 2013 12:22:51 -0400 Subject: [PATCH 3/5] Enable module.exports mocking in react-test.js. We don't currently attempt to mock modules automatically, but we do respect require("mock-modules").mock, .dontMock, and .dumpCache. I'm going to keep investigating auto-mocking, since that would move us much closer to the behavior used within Facebook. Closes #154. Closes #155. --- src/test/mock-modules.js | 91 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 9 deletions(-) diff --git a/src/test/mock-modules.js b/src/test/mock-modules.js index 52d0393b8bb0..d8bd57f81a48 100644 --- a/src/test/mock-modules.js +++ b/src/test/mock-modules.js @@ -16,22 +16,95 @@ * @providesModule mock-modules */ +var mocks = require("mocks"); +var exportsRegistry = {}; +var hasOwn = exportsRegistry.hasOwnProperty; +var explicitMockMap = {}; + +function getMock(exports) { + try { + return mocks.generateFromMetadata(mocks.getMetadata(exports)); + } catch (err) { + console.warn(err); + return exports; + } +} + +// This function should be called at the bottom of any module that might +// need to be mocked, after the final value of module.exports is known. exports.register = function(id, module) { - // TODO + exportsRegistry[id] = { + module: module, + actual: module.exports, + mocked: null // Filled in lazily later. + }; + + // If doMock or doNotMock was called earlier, before the module was + // registered, then the choice should have been recorded in + // explicitMockMap. Now that the module is registered, we can finally + // fulfill the request. + if (hasOwn.call(explicitMockMap, id)) { + if (explicitMockMap[id]) { + doMock(id); + } else { + doNotMock(id); + } + } + + return exports; }; +function resetEntry(id) { + if (hasOwn.call(exportsRegistry, id)) { + delete exportsRegistry[id].module.exports; + delete exportsRegistry[id]; + } +} + exports.dumpCache = function() { - require("mocks").clear(); - return exports; -}; + require("mocks").clear(); -exports.dontMock = function() { - return exports; -}; + // Deleting module.exports will cause the module to be lazily + // reevaluated the next time it is required. + for (var id in exportsRegistry) { + resetEntry(id); + } -exports.mock = function() { - return exports; + return exports; }; +// Call this function to ensure that require(id) returns the actual +// exports object created by the module. +function doNotMock(id) { + explicitMockMap[id] = false; + + var entry = exportsRegistry[id]; + if (entry && entry.module && entry.actual) { + entry.module.exports = entry.actual; + } + + return exports; +} + +// Call this function to ensure that require(id) returns a mock exports +// object based on the actual exports object created by the module. +function doMock(id) { + explicitMockMap[id] = true; + + var entry = exportsRegistry[id]; + if (entry && entry.module && entry.actual) { + // Because mocking can be expensive, create the mock exports object on + // demand, the first time doMock is called. + entry.mocked || (entry.mocked = getMock(entry.actual)); + entry.module.exports = entry.mocked; + } + + return exports; +} + var global = Function("return this")(); require('test/mock-timers').installMockTimers(global); + +// Exported names are different for backwards compatibility. +exports.dontMock = doNotMock; +exports.mock = doMock; From 2d61639f9089e58bf33b29264c6a09fdd1c91637 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 8 Jul 2013 19:19:40 -0400 Subject: [PATCH 4/5] Store dirtyMocks array globally so it can survive dumpCache(). When require("mock-modules").dumpCache() is called, all mock functions previously created continue to refer to the old dirtyMocks array. If we replace that array with a new one, those mock functions will never have their .mockClear() methods called again. The upstream version of mocks.js pulls a similar global trick, and I never understood why until now. --- src/test/mocks.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/mocks.js b/src/test/mocks.js index d0d60b0f398a..ea25b052c425 100644 --- a/src/test/mocks.js +++ b/src/test/mocks.js @@ -72,7 +72,7 @@ function makeComponent(metadata) { metadata.members.prototype.members) || {}; var f = function() { - dirtyMocks.push(f); + global.dirtyMocks.push(f); instances.push(this); calls.push(Array.prototype.slice.call(arguments)); @@ -305,7 +305,8 @@ function removeUnusedRefs(metadata) { }); } -var dirtyMocks = []; +var global = Function("return this")(); +global.dirtyMocks = global.dirtyMocks || []; module.exports = { /** @@ -313,8 +314,8 @@ module.exports = { * called since the last time clear was called. */ clear: function() { - var old = dirtyMocks; - dirtyMocks = []; + var old = global.dirtyMocks; + global.dirtyMocks = []; old.forEach(function(mock) { mock.mockClear(); }); From b763d7d029dd63fb32fdcb2e581712b0ef946263 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 15 Jul 2013 18:21:26 -0400 Subject: [PATCH 5/5] Use a separate grunt/config/jsx config file for tests. No longer injecting __MOCK__ as a global constant (it's just a config property now). Turns out the `grunt jsx:debug` task was never necessary for tests. --- Gruntfile.js | 1 - bin/jsx | 2 +- grunt/config/jsx/debug.json | 1 - grunt/config/jsx/jsx.js | 2 +- grunt/config/jsx/test.json | 7 +++++++ 5 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 grunt/config/jsx/test.json diff --git a/Gruntfile.js b/Gruntfile.js index fad8b47971b3..e2fdacbe4868 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -56,7 +56,6 @@ module.exports = function(grunt) { grunt.registerTask('build:transformer', ['jsx:debug', 'browserify:transformer']); grunt.registerTask('build:min', ['jsx:release', 'browserify:min']); grunt.registerTask('build:test', [ - 'jsx:debug', 'jsx:jasmine', 'jsx:test', 'browserify:jasmine', diff --git a/bin/jsx b/bin/jsx index 76fe4aa93260..5e269cda97e2 100755 --- a/bin/jsx +++ b/bin/jsx @@ -34,7 +34,7 @@ require("commoner").resolve(function(id) { // replacing constant expressions with literal (boolean) values. source = propagate(constants, source); - if (constants.__MOCK__) { + if (context.config.mocking) { // Make sure there is exactly one newline at the end of the module. source = source.replace(/\s+$/m, "\n"); diff --git a/grunt/config/jsx/debug.json b/grunt/config/jsx/debug.json index d4099010b2f1..1596c562e857 100644 --- a/grunt/config/jsx/debug.json +++ b/grunt/config/jsx/debug.json @@ -1,7 +1,6 @@ { "debug": true, "constants": { - "__MOCK__": true, "__DEV__": true } } diff --git a/grunt/config/jsx/jsx.js b/grunt/config/jsx/jsx.js index 34fe98c16630..b8a049da92f2 100644 --- a/grunt/config/jsx/jsx.js +++ b/grunt/config/jsx/jsx.js @@ -25,7 +25,7 @@ var test = { "test/all.js", "**/__tests__/*.js" ]), - configFile: debug.configFile, + configFile: "grunt/config/jsx/test.json", sourceDir: "src", outputDir: "build/modules" }; diff --git a/grunt/config/jsx/test.json b/grunt/config/jsx/test.json new file mode 100644 index 000000000000..e897023aabc2 --- /dev/null +++ b/grunt/config/jsx/test.json @@ -0,0 +1,7 @@ +{ + "debug": true, + "mocking": true, + "constants": { + "__DEV__": true + } +}