From 836a40a0b953de25bf733a97c5c9f6acbf2c5d1d Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 17 Apr 2014 14:26:24 +0800 Subject: [PATCH 1/5] Ensure that build fails are logged at the right times Builds should fail with logging during the failure, not at the end. This reverts back to the behaviour prior to #115. Build fails still call the appropriate callback function. --- lib/builder.js | 8 ++++++-- lib/index.js | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/builder.js b/lib/builder.js index 6902580..94c1059 100644 --- a/lib/builder.js +++ b/lib/builder.js @@ -83,8 +83,12 @@ exports.start = function (json, options, buildCallback) { buildStat, end = function (err) { var end = new Date(); - log.info('done racing, the gears are toast'); - log.info('finished in ' + timer.calc(start, end) + ', pretty fast huh?'); + if (err) { + log.err('Build failed: Errors encountered during build'); + } else { + log.info('done racing, the gears are toast'); + log.info('finished in ' + timer.calc(start, end)); + } if (buildCallback) { buildCallback(err); } diff --git a/lib/index.js b/lib/index.js index bc1d7f1..e98ad78 100644 --- a/lib/index.js +++ b/lib/index.js @@ -46,10 +46,16 @@ var runQueue = function() { } }; +function standardExitCallback(err) { + if (err) { + process.exit(1); + } +} + function logAndExit(err) { if (err) { log.err(err); - process.exit(1); + standardExitCallback(err); } } @@ -70,7 +76,7 @@ exports.init = function (opts, initCallback) { log.reset(options); if (!initCallback) { - initCallback = logAndExit; + initCallback = standardExitCallback; } if (options.cwd) { From 2824585544d77f0d3fc46de92bea6787270bdfbb Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 17 Apr 2014 14:28:26 +0800 Subject: [PATCH 2/5] Linting failures are only warnings --- lib/module.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/module.js b/lib/module.js index 9123095..bc13a55 100644 --- a/lib/module.js +++ b/lib/module.js @@ -82,7 +82,7 @@ var Stack = require('./stack').Stack, fn = 'error'; } if (!!lint && lint.length) { - log.err(file + ' contains ' + lint.length + ' lint errors'); + log.warn(file + ' contains ' + lint.length + ' lint errors'); lint.forEach(function (item) { counter = counter + 1; @@ -131,7 +131,7 @@ var Stack = require('./stack').Stack, } if (messages.length) { - log.err(linted.name + ' contains ' + messages.length + ' lint errors'); + log.warn(linted.name + ' contains ' + messages.length + ' lint errors'); messages.forEach(function (item) { if (item && item.reason) { counter = counter + 1; From 582efe99ee5c551fc77d57e0138405d5e7aab9a2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 17 Apr 2014 14:29:36 +0800 Subject: [PATCH 3/5] Ensure consistent logging for build component failures --- lib/module.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/module.js b/lib/module.js index bc13a55..e2d6b5e 100644 --- a/lib/module.js +++ b/lib/module.js @@ -363,6 +363,9 @@ var buildJS = function (mod, name, callback) { .write(path.join(mod.buildDir, fileName, fileName + '-min.js')) .run(function (err, result) { if (err) { + if (typeof err.error !== 'undefined') { + err = err.error; + } if (/file has not changed/.test(err)) { log.warn(name + ': ' + err); } else if (/ENOENT/.test(err)) { @@ -539,14 +542,13 @@ var buildSkin = function (mod, name, callback) { }) .cssmin() .check() - .log('writing skin file with core wrapper') + .log('writing skin file with core wrapper for ' + skinName) .write(path.join(mod.buildDir, name, 'assets', 'skins', skinName, name + '.css')) .run(stack.add(function (err) { if (err) { - log.err(err); - if (err.code === 'ENOENT') { - log.err('skin file is missing: ' + err.path); - return; + if (err.error.code === 'ENOENT') { + log.err('Skin: file is missing: ' + err.error.path); + callback(err.error); } } @@ -567,7 +569,11 @@ var buildSkin = function (mod, name, callback) { }) .log('writing skin file without core wrapper') .write(path.join(mod.buildDir, name, 'assets', 'skins', skinName, name + '-skin.css')) - .run(stack.add(function () { + .run(stack.add(function (err) { + if (err) { + log.err('Skin: ' + err.error); + return callback(err.error); + } })); })); }); From 9ad3b942927716b06fb5f779fd0371756e71593f Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 17 Apr 2014 10:44:36 +0800 Subject: [PATCH 4/5] Check the size of source files in the output file size check --- lib/tasks/check.js | 12 ++++++++++-- tests/assets/copy/out/empty.txt | 0 tests/assets/copy/out/foo.txt | 1 + tests/assets/copy/src/in/files/empty.txt | 0 tests/assets/copy/src/in/files/foo.txt | 1 + tests/general.js | 16 ++++++++++++++-- 6 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 tests/assets/copy/out/empty.txt create mode 100644 tests/assets/copy/src/in/files/empty.txt diff --git a/lib/tasks/check.js b/lib/tasks/check.js index c5ac67c..fca25e1 100644 --- a/lib/tasks/check.js +++ b/lib/tasks/check.js @@ -3,12 +3,20 @@ Copyright (c) 2012, Yahoo! Inc. All rights reserved. Code licensed under the BSD License: http://yuilibrary.com/license/ */ +var fs = require('fs'); exports.check = function (options, blob, done) { - var bail = null; + var bail = null, + sourceFile; if (blob.result.length === 0) { - bail = 'writing zero length file from ' + blob.name; + if (blob.name) { + sourceFile = fs.readFileSync(blob.name).toString(); + } + + if (typeof sourceFile === 'undefined' || sourceFile.length !== 0) { + bail = 'writing zero length file from ' + blob.name; + } } done(bail, new blob.constructor(blob.result, blob)); diff --git a/tests/assets/copy/out/empty.txt b/tests/assets/copy/out/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/tests/assets/copy/out/foo.txt b/tests/assets/copy/out/foo.txt index e69de29..5ddef1f 100644 --- a/tests/assets/copy/out/foo.txt +++ b/tests/assets/copy/out/foo.txt @@ -0,0 +1 @@ +Some sample content to check for a non-null source diff --git a/tests/assets/copy/src/in/files/empty.txt b/tests/assets/copy/src/in/files/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/tests/assets/copy/src/in/files/foo.txt b/tests/assets/copy/src/in/files/foo.txt index e69de29..5ddef1f 100644 --- a/tests/assets/copy/src/in/files/foo.txt +++ b/tests/assets/copy/src/in/files/foo.txt @@ -0,0 +1 @@ +Some sample content to check for a non-null source diff --git a/tests/general.js b/tests/general.js index ef78630..c26b094 100644 --- a/tests/general.js +++ b/tests/general.js @@ -297,11 +297,23 @@ var tests = { assert.isFunction(topic); }, 'and should produce an error on empty results': function(topic) { + var testFile = path.join(__dirname, 'assets/copy/src/in/files/foo.txt'); + + topic({}, { + result: '', + name: testFile + }, function(err) { + assert.equal(err, 'writing zero length file from ' + testFile); + }); + }, + 'and should not produce an error on empty results when source is also empty': function(topic) { + var testFile = path.join(__dirname, 'assets/copy/src/in/files/empty.txt'); + topic({}, { result: '', - name: 'foo.txt' + name: testFile }, function(err) { - assert.equal(err, 'writing zero length file from foo.txt'); + assert.equal(err, null); }); } }, From 92a98814e85cf3ae02d3c1d8408008c3e2e55080 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 17 Apr 2014 15:01:33 +0800 Subject: [PATCH 5/5] Fail on ant failures too --- lib/ant.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ant.js b/lib/ant.js index 1dfdbc5..b0509aa 100644 --- a/lib/ant.js +++ b/lib/ant.js @@ -113,6 +113,7 @@ exports.process = function (options, callback) { fs.readdir(shifter.cwd(), function (err, files) { if (err) { log.err(err); + callback(err); } var props = []; files.forEach(function (file) { @@ -137,7 +138,9 @@ exports.process = function (options, callback) { } }); } else { - callback('no .properties files located, hit the brakes'); + err = 'no .properties files located, hit the brakes'; + log.err(err); + callback(err); } });