From 97b19fbd1981c72bef06fe2d99cbf1f69c2910a7 Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Fri, 25 May 2018 22:46:06 -0400 Subject: [PATCH 1/8] test/slush.js: Refactor runSlush, capture stderr Eliminates a lot of common boilerplate from each test case, and fixes other issues. Also adds assertions to ensure events are triggered as expected. Previously any assertion errors would technically go uncaught. The more proper way to report them in async tests is to set up a try/catch and pass the error to `done()`. Both stdout and stderr are now captured, and the "should fail when running a generator without slushfile" now fails. Previously, when slush failed in this test, stdout was empty while stderr was ignored due to a failure in liftoff (`cli.launch()`). The `data` variable used to capture stdout was empty, which caused `should.match` to always pass. All output is now written to stderr when a test fails to facilitate stack trace debugging (as with the `cli.launch()` failure). --- test/slush.js | 124 +++++++++++++++++++++----------------------------- 1 file changed, 52 insertions(+), 72 deletions(-) diff --git a/test/slush.js b/test/slush.js index c9a6a5f..655b46e 100644 --- a/test/slush.js +++ b/test/slush.js @@ -4,116 +4,96 @@ var spawn = require('child_process').spawn, describe('slush', function () { it('should list installed generators', function (done) { - var slush = runSlush(); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { + runSlush(undefined, function(code, data) { code.should.equal(0); data.should.match(/\[slush\] ├── bad/); data.should.match(/\[slush\] └── test/); - done(); - }); + }, done); }); it('should list tasks for given generator', function (done) { - var slush = runSlush(['test', '--tasks']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { + runSlush(['test', '--tasks'], function(code, data) { code.should.equal(0); data.should.match(/├── default/); data.should.match(/└── app/); - done(); - }); + }, done); }); it('should run `default` task in generator, when task is not provided', function (done) { - var slush = runSlush(['test']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { + runSlush(['test'], function(code, data) { code.should.equal(0); + data.should.match(/ Starting 'test:default'\.\.\./); data.should.match(/\ndefault\n/); - done(); - }); + data.should.match(/ Finished 'test:default' after /); + data.should.match(/Scaffolding done/); + }, done); }); it('should run provided task in generator', function (done) { - var slush = runSlush(['test:app']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { + runSlush(['test:app'], function(code, data) { code.should.equal(0); + data.should.match(/ Starting 'test:app'\.\.\./); data.should.match(/\napp\n/); - done(); - }); + data.should.match(/ Finished 'test:app' after /); + data.should.match(/Scaffolding done/); + }, done); }); it('should run provided task with arguments in generator', function (done) { - var slush = runSlush(['test:app', 'arg1', 'arg2']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { - code.should.equal(0); + runSlush(['test:app', 'arg1', 'arg2'], function(code, data) { + code.should.match(0); + data.should.match(/ Starting 'test:app'\.\.\./); data.should.match(/\napp \(arg1, arg2\)\n/); - done(); - }); + data.should.match(/ Finished 'test:app' after /); + data.should.match(/Scaffolding done/); + }, done); }); it('should fail when running a non-existing task in a generator', function (done) { - var slush = runSlush(['test:noexist']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { - code.should.equal(1); + runSlush(['test:noexist'], function(code, data) { + code.should.match(1); data.should.match(/\[slush\] Task 'noexist' was not defined in `slush-test`/); - done(); - }); + data.should.not.match(/Scaffolding done/); + }, done); }); it('should fail when running a generator without slushfile', function (done) { - var slush = runSlush(['bad']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { - code.should.equal(1); + runSlush(['bad'], function(code, data) { + code.should.match(1); data.should.match(/\[slush\] No slushfile found/); data.should.match(/\[slush\].+issue with.+`slush-bad`/); - done(); - }); + }, done); }); it('should fail trying to run a non-existing generator', function (done) { - var slush = runSlush(['noexist']); - var data = ''; - slush.stdout.on('data', function (chunk) { - data += chunk; - }); - slush.on('close', function (code) { - code.should.equal(1); + runSlush(['noexist'], function(code, data) { + code.should.match(1); data.should.match(/\[slush\] No generator by name: "noexist" was found/); - done(); - }); + }, done); }); }); -function runSlush (args) { - args = args || []; - var slush = spawn('node', [path.join(__dirname, '..', 'bin', 'slush.js')].concat(args), {cwd: __dirname}); +function runSlush (args, doAssertions, done) { + var slush = spawn('node', + [path.join(__dirname, '..', 'bin', 'slush.js')].concat(args || []), + {cwd: __dirname}); + var data = ''; + slush.stdout.setEncoding('utf8'); - return slush; + + slush.stdout.on('data', function (chunk) { + data += chunk; + }); + slush.stderr.on('data', function (chunk) { + data += chunk; + }); + slush.on('close', function (code) { + try { + doAssertions(code, data); + done(); + } catch (e) { + process.stderr.write(data); + done(e); + } + }); } From c739af6aa0db84b362228fb936c9d1b254b6f3cf Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Sat, 26 May 2018 00:23:49 -0400 Subject: [PATCH 2/8] Update gulpfile to support Gulp 4 --- gulpfile.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index a0dd0ba..ac2f563 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -7,6 +7,10 @@ var jshint = require('gulp-jshint'); var codeFiles = ['**/*.js', '!node_modules/**']; +function taskSpec(tasks) { + return gulp.parallel ? gulp.parallel(tasks) : tasks; +} + gulp.task('lint', function() { log('Linting Files'); return gulp.src(codeFiles) @@ -16,7 +20,7 @@ gulp.task('lint', function() { gulp.task('watch', function() { log('Watching Files'); - gulp.watch(codeFiles, ['lint']); + gulp.watch(codeFiles, taskSpec(['lint'])); }); -gulp.task('default', ['lint', 'watch']); +gulp.task('default', taskSpec(['lint', 'watch'])); From d92d2271ddea3b8de25f3e61d02a1c8773a6771b Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Tue, 29 May 2018 13:31:21 -0400 Subject: [PATCH 3/8] Support both Gulp 3.9.1 and 4.0.0 Gulp 4.0.0 switched its task execution engine from `orchestrator` to `undertaker`. As a result, certain methods and events from Gulp 3.9.1 upon which `slush` depended disappeared: https://github.com/gulpjs/gulp/issues/755 Supporting Gulp 4.0.0 is important because Node 10 broke the `graceful-fs` package upon which Gulp 3.9.1 depends. While there's a workaround (updating the `natives` package), it places a burden on users that still doesn't guarantee that Gulp 3.9.1 will remain future-proof: https://github.com/gulpjs/gulp/issues/2146#issuecomment-379583238 https://github.com/gulpjs/gulp/issues/2162#issuecomment-385197164 https://github.com/nodejs/node/issues/19786#issuecomment-379567469 As it turned out, the changes required to support both versions were fairly straighforward, and should ensure that Slush remains future-proof until the next major Gulp update. NOTE: The test tasks are now all asynchronous via a `done` callback, since Gulp 4 doesn't support synchronous tasks. Any synchronous slushfile task will need to be updated. --- bin/slush.js | 81 +++++++++++++++++++++++++++++------- test/slush-test/slushfile.js | 6 ++- test/slush.js | 23 ++++++++++ 3 files changed, 94 insertions(+), 16 deletions(-) diff --git a/bin/slush.js b/bin/slush.js index 1b1a538..d2efb48 100755 --- a/bin/slush.js +++ b/bin/slush.js @@ -102,8 +102,31 @@ function handleArguments(env) { process.nextTick(function() { if (tasksFlag) { return logTasks(generator.name, gulpInst); + } else if (gulpInst.start) { + // Gulp <= 3.9.1 (gulp.start() is unsupported and breaks under Node 10) + return gulpInst.start.apply(gulpInst, toRun); } - gulpInst.start.apply(gulpInst, toRun); + runGulpV4Tasks(gulpInst, toRun); + }); +} + +// For Gulp 4, we have to bind gulpInst to task functions individually. We +// trigger our own `task_not_found` and `finished` events to maintain the same +// Slush interface between Slush versions (rather than relying on the default +// Gulp 4 behavior). +function runGulpV4Tasks(gulpInst, toRun) { + toRun.forEach(function(task) { + var gulpTask = gulpInst.task(task); + if (gulpTask === undefined) { + gulpInst.emit('task_not_found', { task: task }); + } + gulpInst.task(task, gulpTask.bind(gulpInst)); + }); + gulpInst.parallel(toRun)(function(err) { + if (err) { + process.exit(1); + } + gulpInst.emit('finished'); }); } @@ -121,7 +144,8 @@ function logGenerators(generators) { } function logTasks(name, localGulp) { - var tree = taskTree(localGulp.tasks); + // Gulp <= 3.9.1 uses gulp.tasks; Gulp >= 4.0.0 uses gulp.tree(). + var tree = localGulp.tasks ? taskTree(localGulp.tasks) : localGulp.tree(); tree.label = 'Tasks for generator ' + chalk.magenta(name); archy(tree).split('\n').forEach(function(v) { if (v.trim().length === 0) return; @@ -131,26 +155,28 @@ function logTasks(name, localGulp) { // format orchestrator errors function formatError(e) { - if (!e.err) return e.message; - if (e.err.message) return e.err.message; - return JSON.stringify(e.err); + var err = e.err || e.error; + if (!err) return e.message; + if (err.message) return err.message; + return JSON.stringify(err); } // wire up logging events function logEvents(name, gulpInst) { - gulpInst.on('task_start', function(e) { - gutil.log('Starting', "'" + chalk.cyan(name + ':' + e.task) + "'..."); + var names = getEventNames(gulpInst); + + gulpInst.on(names.task_start, function(e) { + gutil.log('Starting', "'" + chalk.cyan(name + ':' + taskName(e)) + "'..."); }); - gulpInst.on('task_stop', function(e) { - var time = prettyTime(e.hrDuration); - gutil.log('Finished', "'" + chalk.cyan(name + ':' + e.task) + "'", 'after', chalk.magenta(time)); + gulpInst.on(names.task_stop, function(e) { + gutil.log('Finished', "'" + chalk.cyan(name + ':' + taskName(e)) + "'", 'after', chalk.magenta(taskDuration(e))); }); - gulpInst.on('task_err', function(e) { + gulpInst.on(names.task_error, function(e) { + console.error('ERR', e); var msg = formatError(e); - var time = prettyTime(e.hrDuration); - gutil.log("'" + chalk.cyan(name + ':' + e.task) + "'", 'errored after', chalk.magenta(time), chalk.red(msg)); + gutil.log("'" + chalk.cyan(name + ':' + taskName(e)) + "'", 'errored after', chalk.magenta(taskDuration(e)), chalk.red(msg)); }); gulpInst.on('task_not_found', function(err) { @@ -158,11 +184,38 @@ function logEvents(name, gulpInst) { process.exit(1); }); - gulpInst.on('stop', function () { + gulpInst.on(names.finished, function () { log('Scaffolding done'); }); } +function taskName(event) { + return event.task || event.name; +} + +function taskDuration(event) { + return prettyTime(event.hrDuration || event.duration); +} + +function getEventNames(gulpInst) { + if (gulpInst.tasks) { + // Gulp v3.9.1 and earlier + return { + task_start: 'task_start', + task_stop: 'task_stop', + task_error: 'task_err', + finished: 'stop' + }; + } + // Gulp v4.0.0 and later + return { + task_start: 'start', + task_stop: 'stop', + task_error: 'error', + finished: 'finished' + }; +} + function getGenerator (name) { return getAllGenerators().filter(function (gen) { return gen.name === name; diff --git a/test/slush-test/slushfile.js b/test/slush-test/slushfile.js index 88246c5..c151260 100644 --- a/test/slush-test/slushfile.js +++ b/test/slush-test/slushfile.js @@ -1,10 +1,12 @@ 'use strict'; var gulp = require('gulp'); -gulp.task('default', function () { +gulp.task('default', function (done) { console.log('default'); + done(); }); -gulp.task('app', function () { +gulp.task('app', function (done) { console.log('app' + (this.args.length ? ' (' + this.args.join(', ') + ')' : '')); + done(this.args[0] === 'fail' ? new Error('forced error') : undefined); }); diff --git a/test/slush.js b/test/slush.js index 655b46e..c072c28 100644 --- a/test/slush.js +++ b/test/slush.js @@ -39,6 +39,19 @@ describe('slush', function () { }, done); }); + it('should run multiple tasks in generator', function (done) { + runSlush(['test:app:default'], function(code, data) { + code.should.equal(0); + data.should.match(/ Starting 'test:app'\.\.\./); + data.should.match(/\napp\n/); + data.should.match(/ Finished 'test:app' after /); + data.should.match(/ Starting 'test:default'\.\.\./); + data.should.match(/\ndefault\n/); + data.should.match(/ Finished 'test:default' after /); + data.should.match(/Scaffolding done/); + }, done); + }); + it('should run provided task with arguments in generator', function (done) { runSlush(['test:app', 'arg1', 'arg2'], function(code, data) { code.should.match(0); @@ -49,6 +62,16 @@ describe('slush', function () { }, done); }); + it('should fail when a task fails in generator', function (done) { + runSlush(['test:app', 'fail'], function(code, data) { + code.should.match(1); + data.should.match(/ Starting 'test:app'\.\.\./); + data.should.match(/\napp \(fail\)\n/); + data.should.match(/ 'test:app' errored after .* forced error\n/); + data.should.not.match(/Scaffolding done/); + }, done); + }); + it('should fail when running a non-existing task in a generator', function (done) { runSlush(['test:noexist'], function(code, data) { code.should.match(1); From d37ae3a8958fc48f9a2a041a63e66fd540ef9d54 Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Sat, 26 May 2018 01:21:20 -0400 Subject: [PATCH 4/8] package.json: Update most deps, bump to Gulp 4.0.0 liftoff will take some work, as the API has changed between 0.10.0 and 2.5.0. --- package.json | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index c4b569e..4e11465 100644 --- a/package.json +++ b/package.json @@ -23,20 +23,20 @@ "slush": "./bin/slush.js" }, "dependencies": { - "glob": "~4.0.0", - "minimist": "~0.1.0", + "glob": "^7.1.2", + "minimist": "^1.2.0", "gulp-util": "^2.2.0", - "pretty-hrtime": "^0.2.0", - "archy": "^0.0.2", + "pretty-hrtime": "^1.0.3", + "archy": "^1.0.0", "liftoff": "~0.10.0", - "chalk": "^0.4.0" + "chalk": "^2.4.1" }, "devDependencies": { - "mocha": "^1.17.0", - "should": "^3.1.0", - "jshint": "^2.4.1", - "gulp-jshint": "^1.4.0", - "gulp": "~3.6.2" + "mocha": "^5.2.0", + "should": "^13.2.1", + "jshint": "^2.9.5", + "gulp-jshint": "^2.1.0", + "gulp": "^4.0.0" }, "scripts": { "test": "node gulpfile.js && NODE_ENV=test mocha --reporter spec" From b85d5728a21ce58a86967829e24b84bf286468b2 Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Mon, 28 May 2018 11:27:26 -0400 Subject: [PATCH 5/8] package.json,bin/slush.js: Bump to liftoff 2.5.0 Fixes the "should fail when running a generator without slushfile" test case. As noted in the commit that updated runSlush() from test/slush.js and caused the failure, slush was actually failing without the expected "No slushfile found" message. It was failing with a stack trace that started with: path.js:39 throw new ERR_INVALID_ARG_TYPE('path', 'string', path); ^ TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type object at assertPath (path.js:39:11) at Object.extname (path.js:1375:5) at Liftoff.buildEnvironment (/Users/mbland/src/mbland/slush/node_modules/liftoff/index.js:111:50) at Liftoff.launch (/Users/mbland/src/mbland/slush/node_modules/liftoff/index.js:154:22) at Object. (/Users/mbland/src/mbland/slush/bin/slush.js:57:5) This points to a bug in the previous version of liftoff whereby the the search for `slushfile.js` found nothing, resulting in a `configPath` of `null`. liftoff passed this `null` to `Object.extname`, resulting in this stack trace. liftoff v2.5.0 doesn't have this problem, but it does have a slightly different API. With the changes to `bin/slush.js`, this test case now passes along with the rest of the suite. --- bin/slush.js | 15 ++++++--------- package.json | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/bin/slush.js b/bin/slush.js index d2efb48..b9da9c0 100755 --- a/bin/slush.js +++ b/bin/slush.js @@ -16,6 +16,7 @@ var versionFlag = argv.v || argv.version; var params = argv._.slice(); var generatorAndTasks = params.length ? params.shift().split(':') : []; var generatorName = generatorAndTasks.shift(); +var tasks = generatorAndTasks; if (!generatorName) { if (versionFlag) { @@ -34,11 +35,6 @@ if (!generator) { process.exit(1); } -// Setting cwd and slushfile dir: -argv.cwd = process.cwd(); -argv.slushfile = path.join(generator.path, 'slushfile.js'); -argv._ = generatorAndTasks; - var cli = new Liftoff({ processTitle: 'slush', moduleName: 'gulp', @@ -54,13 +50,14 @@ cli.on('requireFail', function(name) { gutil.log(chalk.red('Failed to load external module'), chalk.magenta(name)); }); -cli.launch(handleArguments, argv); +cli.launch({ + // Setting cwd and slushfile dir: + cwd: process.cwd(), + configPath: path.join(generator.path, 'slushfile.js') +}, handleArguments); function handleArguments(env) { - - var argv = env.argv; var tasksFlag = argv.T || argv.tasks; - var tasks = argv._; var toRun = tasks.length ? tasks : ['default']; var args = params; diff --git a/package.json b/package.json index 4e11465..8858899 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "gulp-util": "^2.2.0", "pretty-hrtime": "^1.0.3", "archy": "^1.0.0", - "liftoff": "~0.10.0", + "liftoff": "^2.5.0", "chalk": "^2.4.1" }, "devDependencies": { From 9f2f01de2439aaf4e1bd575937888e1b4a2bbace Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Tue, 29 May 2018 17:11:45 -0400 Subject: [PATCH 6/8] .travis.yml: Use latest stable and LTS versions The previous versions, 0.10 and 0.11, are years out of date. Using the latest versions per the Travis docs should fix the build: https://docs.travis-ci.com/user/languages/javascript-with-nodejs/#Specifying-Node.js-versions --- .travis.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 05d299e..8f17669 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,7 @@ language: node_js node_js: - - "0.10" - - "0.11" + - "node" + - "lts/*" +script: + - "npm test" + - "printf 'Finished testing with Gulp v4; now testing with Gulp v3\n' && npm install gulp@3 && npm test" From 77bbf97381759e44d080d8f799f49e8f9dcca2c2 Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Wed, 30 May 2018 08:38:11 -0400 Subject: [PATCH 7/8] Fix comment, test assertions No behaviorial changes. --- bin/slush.js | 2 +- test/slush.js | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/slush.js b/bin/slush.js index b9da9c0..70d0829 100755 --- a/bin/slush.js +++ b/bin/slush.js @@ -109,7 +109,7 @@ function handleArguments(env) { // For Gulp 4, we have to bind gulpInst to task functions individually. We // trigger our own `task_not_found` and `finished` events to maintain the same -// Slush interface between Slush versions (rather than relying on the default +// Slush interface between Gulp versions (rather than relying on the default // Gulp 4 behavior). function runGulpV4Tasks(gulpInst, toRun) { toRun.forEach(function(task) { diff --git a/test/slush.js b/test/slush.js index c072c28..958f8ef 100644 --- a/test/slush.js +++ b/test/slush.js @@ -54,7 +54,7 @@ describe('slush', function () { it('should run provided task with arguments in generator', function (done) { runSlush(['test:app', 'arg1', 'arg2'], function(code, data) { - code.should.match(0); + code.should.equal(0); data.should.match(/ Starting 'test:app'\.\.\./); data.should.match(/\napp \(arg1, arg2\)\n/); data.should.match(/ Finished 'test:app' after /); @@ -64,7 +64,7 @@ describe('slush', function () { it('should fail when a task fails in generator', function (done) { runSlush(['test:app', 'fail'], function(code, data) { - code.should.match(1); + code.should.equal(1); data.should.match(/ Starting 'test:app'\.\.\./); data.should.match(/\napp \(fail\)\n/); data.should.match(/ 'test:app' errored after .* forced error\n/); @@ -74,7 +74,7 @@ describe('slush', function () { it('should fail when running a non-existing task in a generator', function (done) { runSlush(['test:noexist'], function(code, data) { - code.should.match(1); + code.should.equal(1); data.should.match(/\[slush\] Task 'noexist' was not defined in `slush-test`/); data.should.not.match(/Scaffolding done/); }, done); @@ -82,7 +82,7 @@ describe('slush', function () { it('should fail when running a generator without slushfile', function (done) { runSlush(['bad'], function(code, data) { - code.should.match(1); + code.should.equal(1); data.should.match(/\[slush\] No slushfile found/); data.should.match(/\[slush\].+issue with.+`slush-bad`/); }, done); @@ -90,7 +90,7 @@ describe('slush', function () { it('should fail trying to run a non-existing generator', function (done) { runSlush(['noexist'], function(code, data) { - code.should.match(1); + code.should.equal(1); data.should.match(/\[slush\] No generator by name: "noexist" was found/); }, done); }); From 8a78ed7beca8e94195aefbd595c86deffed3fb5b Mon Sep 17 00:00:00 2001 From: "Bland, Mike" Date: Wed, 30 May 2018 08:38:36 -0400 Subject: [PATCH 8/8] .travis.yml: Add lint command to test scripts --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8f17669..f0566d2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,5 +3,6 @@ node_js: - "node" - "lts/*" script: + - "node_modules/.bin/gulp lint" - "npm test" - "printf 'Finished testing with Gulp v4; now testing with Gulp v3\n' && npm install gulp@3 && npm test"