Skip to content

Commit

Permalink
Build: Don't install jsdom 3 on Node.js 0.10 & 0.12 by default
Browse files Browse the repository at this point in the history
jsdom 3 requires Python & Visual Studio on Windows which is a significant
barrier to contributors. Newer jsdom versions don't require pre-compiling
but work only on io.js. This commit installs the new jsdom everywhere (it
does install in old Node.js, it just won't work) and executes Node-related
tests only on newer Nodes or if a working jsdom version is installed. The
latter can be achieved by running the `old_jsdom` task.

Node.js is merging with io.js soon so this will become a smaller problem over
time.

One drawback is our Jenkins setup runs on Node 0.10 so it won't be running
Node tests anymore. We have Travis set up on io.js, though so all PRs
have those tests run. When the new LTS Node.js arrives (as it soon merges
with io.js) we should update our Jenkins infrastructure so that it runs on this
new version.

Fixes gh-2519
Closes gh-2526
  • Loading branch information
mgol committed Sep 8, 2015
1 parent f9ef427 commit dbb2daa
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 35 deletions.
22 changes: 19 additions & 3 deletions Gruntfile.js
Expand Up @@ -14,7 +14,18 @@ module.exports = function( grunt ) {
var fs = require( "fs" ), var fs = require( "fs" ),
stripJSONComments = require( "strip-json-comments" ), stripJSONComments = require( "strip-json-comments" ),
gzip = require( "gzip-js" ), gzip = require( "gzip-js" ),
srcHintOptions = readOptionalJSON( "src/.jshintrc" ); srcHintOptions = readOptionalJSON( "src/.jshintrc" ),
newNode = !/^v0/.test( process.version ),

// Allow to skip jsdom-related tests in Node.js < 1.0.0
runJsdomTests = newNode || ( function() {
try {
require( "jsdom" );
return true;
} catch ( e ) {
return false;
}
} )();


// The concatenated file won't pass onevar // The concatenated file won't pass onevar
// But our modules can // But our modules can
Expand Down Expand Up @@ -182,9 +193,14 @@ module.exports = function( grunt ) {


grunt.registerTask( "lint", [ "jsonlint", "jshint", "jscs" ] ); grunt.registerTask( "lint", [ "jsonlint", "jshint", "jscs" ] );


grunt.registerTask( "test_fast", [ "node_smoke_tests" ] ); // Don't run Node-related tests in Node.js < 1.0.0 as they require an old
// jsdom version that needs compiling, making it harder for people to compile
// jQuery on Windows. (see gh-2519)
grunt.registerTask( "test_fast", runJsdomTests ? [ "node_smoke_tests" ] : [] );


grunt.registerTask( "test", [ "test_fast", "promises_aplus_tests" ] ); grunt.registerTask( "test", [ "test_fast" ].concat(
runJsdomTests ? [ "promises_aplus_tests" ] : []
) );


// Short list as a high frequency watch task // Short list as a high frequency watch task
grunt.registerTask( "dev", [ "build:*:*", "lint", "uglify", "remove_map_comment", "dist:*" ] ); grunt.registerTask( "dev", [ "build:*:*", "lint", "uglify", "remove_map_comment", "dist:*" ] );
Expand Down
27 changes: 0 additions & 27 deletions build/tasks/install_jsdom.js

This file was deleted.

20 changes: 20 additions & 0 deletions build/tasks/install_old_jsdom.js
@@ -0,0 +1,20 @@
module.exports = function( grunt ) {

"use strict";

// Run this task to run jsdom-related tests on Node.js < 1.0.0.
grunt.registerTask( "old_jsdom", function() {
if ( !/^v0/.test( process.version ) ) {
console.warn( "The old_jsdom task doesn\'t need to be run in io.js or new Node.js" );
return;
}

// Use npm on the command-line
// There is no local npm
grunt.util.spawn( {
cmd: "npm",
args: [ "install", "jsdom@3" ],
opts: { stdio: "inherit" }
}, this.async() );
} );
};
2 changes: 1 addition & 1 deletion build/tasks/node_smoke_tests.js
Expand Up @@ -5,7 +5,7 @@ module.exports = function( grunt ) {
var fs = require( "fs" ), var fs = require( "fs" ),
spawnTest = require( "./lib/spawn_test.js" ), spawnTest = require( "./lib/spawn_test.js" ),
testsDir = "./test/node_smoke_tests/", testsDir = "./test/node_smoke_tests/",
nodeSmokeTests = [ "jsdom", "babel:nodeSmokeTests" ]; nodeSmokeTests = [ "babel:nodeSmokeTests" ];


// Fire up all tests defined in test/node_smoke_tests/*.js in spawned sub-processes. // Fire up all tests defined in test/node_smoke_tests/*.js in spawned sub-processes.
// All the files under test/node_smoke_tests/*.js are supposed to exit with 0 code // All the files under test/node_smoke_tests/*.js are supposed to exit with 0 code
Expand Down
5 changes: 1 addition & 4 deletions package.json
Expand Up @@ -39,6 +39,7 @@
"grunt-jsonlint": "1.0.4", "grunt-jsonlint": "1.0.4",
"grunt-npmcopy": "0.1.0", "grunt-npmcopy": "0.1.0",
"gzip-js": "0.3.2", "gzip-js": "0.3.2",
"jsdom": "5.6.1",
"load-grunt-tasks": "1.0.0", "load-grunt-tasks": "1.0.0",
"native-promise-only": "0.7.8-a", "native-promise-only": "0.7.8-a",
"promises-aplus-tests": "2.1.0", "promises-aplus-tests": "2.1.0",
Expand All @@ -52,10 +53,6 @@
"testswarm": "1.1.0", "testswarm": "1.1.0",
"win-spawn": "2.0.0" "win-spawn": "2.0.0"
}, },
"jsdomVersions": {
"node": "3.1.2",
"iojs": "5.3.0"
},
"scripts": { "scripts": {
"build": "npm install && grunt", "build": "npm install && grunt",
"start": "grunt watch", "start": "grunt watch",
Expand Down

1 comment on commit dbb2daa

@mgol
Copy link
Member Author

@mgol mgol commented on dbb2daa Oct 26, 2015

Choose a reason for hiding this comment

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

@timmywil I'd like to backport it to 2.2, otherwise it's not possible to build it using Node.js 4.2.1, the latest version and an LTS at the same time. Any objections?

It's not necessary to backport to 1.11 as we don't support non-browser environments there.

Please sign in to comment.