From ead18c6c8392dd952298f3bb8959fccf48a3d047 Mon Sep 17 00:00:00 2001 From: sttk Date: Sun, 4 Dec 2016 15:41:44 +0900 Subject: [PATCH 1/7] Restrict config properties --- index.js | 48 +++-- lib/shared/config/cfg-specs.js | 40 +++++ lib/shared/config/cli-flags.js | 24 +++ lib/shared/config/env-flags.js | 14 ++ lib/shared/config/loadfiles.js | 33 ++++ lib/versioned/^3.7.0/index.js | 7 +- lib/versioned/^4.0.0-alpha.1/index.js | 7 +- lib/versioned/^4.0.0-alpha.2/index.js | 11 +- lib/versioned/^4.0.0/index.js | 11 +- package.json | 9 +- test/{config.js => config-description.js} | 2 +- test/config-flags-gulpfile.js | 88 +++++++++ test/config-flags-silent.js | 113 ++++++++++++ .../fixtures/config/flags/gulpfile/.gulp.json | 5 + .../config/flags/gulpfile/cwd/gulpfile.js | 8 + .../flags/gulpfile/is/here/mygulpfile.js | 9 + .../fixtures/config/flags/silent/f/.gulp.json | 5 + .../config/flags/silent/f/gulpfile.js | 7 + .../fixtures/config/flags/silent/t/.gulp.json | 5 + .../config/flags/silent/t/gulpfile.js | 7 + test/lib/config-cfg-specs.js | 69 +++++++ test/lib/config-cli-flags.js | 152 ++++++++++++++++ test/lib/config-env-flags.js | 169 ++++++++++++++++++ test/lib/config-loadfiles.js | 38 ++++ 24 files changed, 837 insertions(+), 44 deletions(-) create mode 100644 lib/shared/config/cfg-specs.js create mode 100644 lib/shared/config/cli-flags.js create mode 100644 lib/shared/config/env-flags.js create mode 100644 lib/shared/config/loadfiles.js rename test/{config.js => config-description.js} (97%) create mode 100644 test/config-flags-gulpfile.js create mode 100644 test/config-flags-silent.js create mode 100644 test/fixtures/config/flags/gulpfile/.gulp.json create mode 100644 test/fixtures/config/flags/gulpfile/cwd/gulpfile.js create mode 100644 test/fixtures/config/flags/gulpfile/is/here/mygulpfile.js create mode 100644 test/fixtures/config/flags/silent/f/.gulp.json create mode 100644 test/fixtures/config/flags/silent/f/gulpfile.js create mode 100644 test/fixtures/config/flags/silent/t/.gulp.json create mode 100644 test/fixtures/config/flags/silent/t/gulpfile.js create mode 100644 test/lib/config-cfg-specs.js create mode 100644 test/lib/config-cli-flags.js create mode 100644 test/lib/config-env-flags.js create mode 100644 test/lib/config-loadfiles.js diff --git a/index.js b/index.js index 6bb1f4eb..c8a6ebbd 100644 --- a/index.js +++ b/index.js @@ -3,14 +3,13 @@ var fs = require('fs'); var path = require('path'); var log = require('gulplog'); +var fancyLog = require('fancy-log'); var chalk = require('chalk'); var yargs = require('yargs'); var Liftoff = require('liftoff'); var tildify = require('tildify'); var interpret = require('interpret'); var v8flags = require('v8flags'); -var merge = require('lodash.merge'); -var isString = require('lodash.isstring'); var findRange = require('semver-greatest-satisfied-range'); var exit = require('./lib/shared/exit'); var cliOptions = require('./lib/shared/cliOptions'); @@ -20,6 +19,10 @@ var cliVersion = require('./package.json').version; var getBlacklist = require('./lib/shared/getBlacklist'); var toConsole = require('./lib/shared/log/toConsole'); +var loadConfigFiles = require('./lib/shared/config/loadfiles'); +var mergeToCliFlags = require('./lib/shared/config/cli-flags'); +var mergeToEnvFlags = require('./lib/shared/config/env-flags'); + // Logging functions var logVerify = require('./lib/shared/log/verify'); var logBlacklistError = require('./lib/shared/log/blacklistError'); @@ -65,15 +68,13 @@ if (opts.continue) { process.env.UNDERTAKER_SETTLE = 'true'; } -// Set up event listeners for logging. -toConsole(log, opts); - cli.on('require', function(name) { - log.info('Requiring external module', chalk.magenta(name)); + fancyLog('Requiring external module', chalk.magenta(name)); }); cli.on('requireFail', function(name) { - log.error(chalk.red('Failed to load external module'), chalk.magenta(name)); + fancyLog.error( + chalk.red('Failed to load external module'), chalk.magenta(name)); }); cli.on('respawn', function(flags, child) { @@ -84,26 +85,35 @@ cli.on('respawn', function(flags, child) { }); function run() { - cli.launch({ + var envOpts = { cwd: opts.cwd, configPath: opts.gulpfile, require: opts.require, completion: opts.completion, - }, handleArguments); + }; + + cli.launch(envOpts, function(env) { + var config; + try { + config = loadConfigFiles(env.configFiles['.gulp'], ['home', 'cwd']); + } catch (e) { + log.error(chalk.red(e.message)); + exit(1); + } + + mergeToCliFlags(opts, config, cliOptions); + mergeToEnvFlags(env, config, envOpts); + + handleArguments(env, opts, config); + }); } module.exports = run; // The actual logic -function handleArguments(env) { - - // Map an array of keys to preserve order - var configFilePaths = ['home', 'cwd'].map(function(key) { - return env.configFiles['.gulp'][key]; - }); - configFilePaths.filter(isString).forEach(function(filePath) { - merge(opts, require(filePath)); - }); +function handleArguments(env, opts, cfg) { + // Set up event listeners for logging. + toConsole(log, opts); if (opts.help) { console.log(parser.help()); @@ -169,5 +179,5 @@ function handleArguments(env) { } // Load and execute the CLI version - require(path.join(__dirname, '/lib/versioned/', range, '/'))(opts, env); + require(path.join(__dirname, '/lib/versioned/', range, '/'))(opts, env, cfg); } diff --git a/lib/shared/config/cfg-specs.js b/lib/shared/config/cfg-specs.js new file mode 100644 index 00000000..af0414a7 --- /dev/null +++ b/lib/shared/config/cfg-specs.js @@ -0,0 +1,40 @@ +'use strict'; + +var path = require('path'); + +module.exports = { + + description: { + validate: function(value) { + if (typeof value !== 'string') { + throw new TypeError('config.description must be a string: ' + value); + } + if (!value) { + throw new Error('config.description requires a value.'); + } + return value; + }, + }, + + 'flags.silent': { + validate: function(value) { + if (typeof value !== 'boolean') { + throw new TypeError('config.flags.silent must be a boolean: ' + value); + } + return value; + }, + }, + + 'flags.gulpfile': { + validate: function(value, configFile) { + if (typeof value !== 'string') { + throw new TypeError( + 'config.flags.gulpfile must be a string: ' + value); + } + if (!value) { + throw new Error('config.flags.gulpfile requires a value.'); + } + return path.resolve(path.dirname(configFile), value); + }, + }, +}; diff --git a/lib/shared/config/cli-flags.js b/lib/shared/config/cli-flags.js new file mode 100644 index 00000000..8fc37f8d --- /dev/null +++ b/lib/shared/config/cli-flags.js @@ -0,0 +1,24 @@ +'use strict'; + +var yargs = require('yargs'); +var camelCase = require('lodash.camelcase'); + +function mergeToCliFlags(cliFlags, config, cliOptions) { + var argv = yargs(process.argv.slice(2)).argv; + var cfgFlags = config.flags || {}; + + var optNames = ['silent']; + optNames.filter(excludeArgsByUser).map(camelCase).forEach(copyConfig); + + function excludeArgsByUser(name) { + return !(argv[name] != null || argv[cliOptions[name].alias] != null); + } + + function copyConfig(name) { + if (cfgFlags[name] !== undefined) { + cliFlags[name] = cfgFlags[name]; + } + } +} + +module.exports = mergeToCliFlags; diff --git a/lib/shared/config/env-flags.js b/lib/shared/config/env-flags.js new file mode 100644 index 00000000..973b493b --- /dev/null +++ b/lib/shared/config/env-flags.js @@ -0,0 +1,14 @@ +'use strict'; + +var path = require('path'); + +function mergeToEnvFlags(envFlags, config, envOpts) { + var cfgFlags = config.flags || {}; + + if (!envOpts.configPath && cfgFlags.gulpfile) { + envFlags.configPath = cfgFlags.gulpfile; + envFlags.configBase = path.dirname(cfgFlags.gulpfile); + } +} + +module.exports = mergeToEnvFlags; diff --git a/lib/shared/config/loadfiles.js b/lib/shared/config/loadfiles.js new file mode 100644 index 00000000..47689433 --- /dev/null +++ b/lib/shared/config/loadfiles.js @@ -0,0 +1,33 @@ +'use strict'; + +var get = require('lodash.get'); +var set = require('lodash.set'); +var isString = require('lodash.isstring'); +var configSpecs = require('./cfg-specs'); + +function loadConfigFiles(configFilesBase, keysInOrder) { + var config = {}; + + var configFilePaths = keysInOrder.map(function(key) { + return configFilesBase[key]; + }).filter(isString); + + configFilePaths.forEach(function(filePath) { + var loadedCfg = require(filePath); + + Object.keys(configSpecs).forEach(function(keyPath) { + var spec = configSpecs[keyPath]; + + var value = get(loadedCfg, keyPath); + if (value === undefined) { + return; + } + + set(config, keyPath, spec.validate(value, filePath)); + }); + }); + + return config; +} + +module.exports = loadConfigFiles; diff --git a/lib/versioned/^3.7.0/index.js b/lib/versioned/^3.7.0/index.js index 4879095e..06ffedaf 100644 --- a/lib/versioned/^3.7.0/index.js +++ b/lib/versioned/^3.7.0/index.js @@ -4,7 +4,6 @@ var chalk = require('chalk'); var log = require('gulplog'); var stdout = require('mute-stdout'); var tildify = require('tildify'); -var isString = require('lodash.isstring'); var taskTree = require('./taskTree'); var logTasks = require('../../shared/log/tasks'); @@ -12,7 +11,7 @@ var logEvents = require('./log/events'); var logTasksSimple = require('./log/tasksSimple'); var registerExports = require('../../shared/registerExports'); -function execute(opts, env) { +function execute(opts, env, config) { var tasks = opts._; var toRun = tasks.length ? tasks : ['default']; @@ -39,8 +38,8 @@ function execute(opts, env) { } if (opts.tasks) { var tree = taskTree(gulpInst.tasks); - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); } diff --git a/lib/versioned/^4.0.0-alpha.1/index.js b/lib/versioned/^4.0.0-alpha.1/index.js index 1eb91b06..2de28569 100644 --- a/lib/versioned/^4.0.0-alpha.1/index.js +++ b/lib/versioned/^4.0.0-alpha.1/index.js @@ -6,7 +6,6 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); -var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -16,7 +15,7 @@ var logSyncTask = require('../^4.0.0/log/syncTask'); var logTasksSimple = require('../^4.0.0/log/tasksSimple'); var registerExports = require('../../shared/registerExports'); -function execute(opts, env) { +function execute(opts, env, config) { var tasks = opts._; var toRun = tasks.length ? tasks : ['default']; @@ -44,8 +43,8 @@ function execute(opts, env) { } if (opts.tasks) { var tree = {}; - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); } diff --git a/lib/versioned/^4.0.0-alpha.2/index.js b/lib/versioned/^4.0.0-alpha.2/index.js index 63c85b49..c6694ec4 100644 --- a/lib/versioned/^4.0.0-alpha.2/index.js +++ b/lib/versioned/^4.0.0-alpha.2/index.js @@ -6,7 +6,6 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); -var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -18,7 +17,7 @@ var registerExports = require('../../shared/registerExports'); var getTask = require('../^4.0.0/log/getTask'); -function execute(opts, env) { +function execute(opts, env, config) { var tasks = opts._; var toRun = tasks.length ? tasks : ['default']; @@ -49,8 +48,8 @@ function execute(opts, env) { } if (opts.tasks) { tree = gulpInst.tree({ deep: true }); - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); } @@ -59,8 +58,8 @@ function execute(opts, env) { } if (opts.tasksJson) { tree = gulpInst.tree({ deep: true }); - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + tildify(env.configPath); } diff --git a/lib/versioned/^4.0.0/index.js b/lib/versioned/^4.0.0/index.js index d434827c..aa9a3660 100644 --- a/lib/versioned/^4.0.0/index.js +++ b/lib/versioned/^4.0.0/index.js @@ -6,7 +6,6 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); -var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -18,7 +17,7 @@ var registerExports = require('../../shared/registerExports'); var getTask = require('./log/getTask'); -function execute(opts, env) { +function execute(opts, env, config) { var tasks = opts._; var toRun = tasks.length ? tasks : ['default']; @@ -49,8 +48,8 @@ function execute(opts, env) { } if (opts.tasks) { tree = gulpInst.tree({ deep: true }); - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); } @@ -59,8 +58,8 @@ function execute(opts, env) { } if (opts.tasksJson) { tree = gulpInst.tree({ deep: true }); - if (opts.description && isString(opts.description)) { - tree.label = opts.description; + if (config.description) { + tree.label = config.description; } else { tree.label = 'Tasks for ' + tildify(env.configPath); } diff --git a/package.json b/package.json index 5e316222..cdbdef3a 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "lint": "eslint . && jscs index.js bin/ lib/ test/", "prepublish": "marked-man --name gulp docs/CLI.md > gulp.1", "pretest": "npm run lint", - "test": "mocha --async-only --timeout 3000", + "test": "mocha --async-only --timeout 3000 test/lib test", "cover": "nyc --reporter=lcov --reporter=text-summary npm test", "coveralls": "nyc --reporter=text-lcov npm test | coveralls", "changelog": "github-changes -o gulpjs -r gulp-cli -b master -f ./CHANGELOG.md --order-semver --use-commit-body" @@ -38,10 +38,12 @@ "gulplog": "^1.0.0", "interpret": "^1.0.0", "liftoff": "^2.3.0", + "lodash.camelcase": "^4.3.0", + "lodash.get": "^4.4.2", "lodash.isfunction": "^3.0.8", "lodash.isplainobject": "^4.0.4", "lodash.isstring": "^4.0.1", - "lodash.merge": "^4.5.1", + "lodash.set": "^4.3.2", "lodash.sortby": "^4.5.0", "matchdep": "^1.0.0", "mute-stdout": "^1.0.0", @@ -62,8 +64,7 @@ "fs-extra": "^0.26.1", "github-changes": "^1.0.1", "gulp": "gulpjs/gulp#4.0", - "gulp-test-tools": "^0.6.0", - "istanbul": "^0.4.5", + "gulp-test-tools": "^0.6.1", "jscs": "^2.3.5", "jscs-preset-gulp": "^1.0.0", "marked-man": "^0.1.3", diff --git a/test/config.js b/test/config-description.js similarity index 97% rename from test/config.js rename to test/config-description.js index e2ce6c48..9fd7842e 100644 --- a/test/config.js +++ b/test/config-description.js @@ -11,7 +11,7 @@ var runner = require('gulp-test-tools').gulpRunner; var fixturesDir = path.join(__dirname, 'fixtures', 'config'); var expectedDir = path.join(__dirname, 'expected', 'config'); -describe('gulp configuration', function() { +describe('config: description', function() { it('Should configure with a .gulp.* file in cwd', function(done) { runner({ verbose: false }) diff --git a/test/config-flags-gulpfile.js b/test/config-flags-gulpfile.js new file mode 100644 index 00000000..b04d84e7 --- /dev/null +++ b/test/config-flags-gulpfile.js @@ -0,0 +1,88 @@ +'use strict'; + +var expect = require('expect'); + +var path = require('path'); +var fixturesDir = path.join(__dirname, 'fixtures/config'); + +var headLines = require('gulp-test-tools').headLines; +var runner = require('gulp-test-tools').gulpRunner().basedir(fixturesDir); + +describe('config: flags.gulpfile', function() { + + it('Should configure with a .gulp.* file', function(done) { + runner + .chdir('flags/gulpfile') + .gulp() + .run(cb); + + function cb(err, stdout, stderr) { + stdout = headLines(stdout, 2, 2); + expect(stdout).toEqual( + 'This gulpfile : ' + + path.join(fixturesDir, 'flags/gulpfile/is/here/mygulpfile.js') + + '\n' + + 'The current directory : ' + path.join(fixturesDir, 'flags/gulpfile') + ); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should configure with a .gulp.* file in the directory specified by ' + + '\n\t--cwd', function(done) { + runner + .gulp('--cwd ./flags/gulpfile') + .run(cb); + + function cb(err, stdout, stderr) { + stdout = headLines(stdout, 2, 3); + expect(stdout).toEqual( + 'This gulpfile : ' + + path.join(fixturesDir, 'flags/gulpfile/is/here/mygulpfile.js') + + '\n' + + 'The current directory : ' + path.join(fixturesDir, 'flags/gulpfile') + ); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should ignore a ./gulp.* file if another directory is specified by ' + + '\n\t--cwd', function(done) { + runner + .chdir('./flags/gulpfile') + .gulp('--cwd ./cwd') + .run(cb); + + function cb(err, stdout, stderr) { + stdout = headLines(stdout, 1, 3); + expect(stdout).toEqual( + 'Another gulpfile : ' + + path.join(fixturesDir, 'flags/gulpfile/cwd/gulpfile.js') + ); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should ignore a ./.gulp.* file if another gulpfile is specified by ' + + '\n\t--gulpfile', function(done) { + runner + .chdir('./flags/gulpfile') + .gulp('--gulpfile ./cwd/gulpfile.js') + .run(cb); + + function cb(err, stdout, stderr) { + stdout = headLines(stdout, 1, 3); + expect(stdout).toEqual( + 'Another gulpfile : ' + + path.join(fixturesDir, 'flags/gulpfile/cwd/gulpfile.js') + ); + expect(stderr).toEqual(''); + done(err); + } + }); + +}); + diff --git a/test/config-flags-silent.js b/test/config-flags-silent.js new file mode 100644 index 00000000..13b9ad83 --- /dev/null +++ b/test/config-flags-silent.js @@ -0,0 +1,113 @@ +'use strict'; + +var expect = require('expect'); +var path = require('path'); +var skipLines = require('gulp-test-tools').skipLines; +var eraseTime = require('gulp-test-tools').eraseTime; +var eraseLapse = require('gulp-test-tools').eraseLapse; + +var fixturesDir = path.join(__dirname, 'fixtures/config'); +var runner = require('gulp-test-tools').gulpRunner().basedir(fixturesDir); + +describe('config: flags.silent', function() { + + it('Should be silent if `flags.silent` is true in .gulp.*', + function(done) { + runner + .chdir('flags/silent/t') + .gulp() + .run(cb); + + function cb(err, stdout, stderr) { + expect(stdout).toEqual(''); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should not be silent if `flags.silent` is false in .gulp.*', + function(done) { + runner + .chdir('flags/silent/f') + .gulp() + .run(cb); + + function cb(err, stdout, stderr) { + stdout = eraseLapse(eraseTime(skipLines(stdout, 1))); + expect(stdout).toEqual( + 'Starting \'default\'...\n' + + 'Finished \'default\' after ?\n' + + '' + ); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should be silent with --silent event if `flags.silent` is false in ' + + '\n\t.gulp.*', function(done) { + runner + .chdir('flags/silent/f') + .gulp('--silent') + .run(cb); + + function cb(err, stdout, stderr) { + expect(stdout).toEqual(''); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should be silent with -S event if `flags.silent` is false in ' + + '\n\t.gulp.*', function(done) { + runner + .chdir('flags/silent/f') + .gulp('-S') + .run(cb); + + function cb(err, stdout, stderr) { + expect(stdout).toEqual(''); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should not be silent with --silent=false event if `flags.silent` is ' + + 'true\n\tin .gulp.*', function(done) { + runner + .chdir('flags/silent/t') + .gulp('--silent=false') + .run(cb); + + function cb(err, stdout, stderr) { + stdout = eraseLapse(eraseTime(skipLines(stdout, 1))); + expect(stdout).toEqual( + 'Starting \'default\'...\n' + + 'Finished \'default\' after ?\n' + + '' + ); + expect(stderr).toEqual(''); + done(err); + } + }); + + it('Should not be silent with -S false event if `flags.silent` is true in ' + + '\n\t.gulp.*', function(done) { + runner + .chdir('flags/silent/t') + .gulp('-S false') + .run(cb); + + function cb(err, stdout, stderr) { + stdout = eraseLapse(eraseTime(skipLines(stdout, 1))); + expect(stdout).toEqual( + 'Starting \'default\'...\n' + + 'Finished \'default\' after ?\n' + + '' + ); + expect(stderr).toEqual(''); + done(err); + } + }); + +}); diff --git a/test/fixtures/config/flags/gulpfile/.gulp.json b/test/fixtures/config/flags/gulpfile/.gulp.json new file mode 100644 index 00000000..b2089dc7 --- /dev/null +++ b/test/fixtures/config/flags/gulpfile/.gulp.json @@ -0,0 +1,5 @@ +{ + "flags": { + "gulpfile": "./is/here/mygulpfile.js" + } +} diff --git a/test/fixtures/config/flags/gulpfile/cwd/gulpfile.js b/test/fixtures/config/flags/gulpfile/cwd/gulpfile.js new file mode 100644 index 00000000..3b6476b6 --- /dev/null +++ b/test/fixtures/config/flags/gulpfile/cwd/gulpfile.js @@ -0,0 +1,8 @@ +'use strict'; + +var gulp = require('gulp'); + +gulp.task('default', function(done) { + console.log('Another gulpfile : ' + __filename); + done(); +}); diff --git a/test/fixtures/config/flags/gulpfile/is/here/mygulpfile.js b/test/fixtures/config/flags/gulpfile/is/here/mygulpfile.js new file mode 100644 index 00000000..61b4199a --- /dev/null +++ b/test/fixtures/config/flags/gulpfile/is/here/mygulpfile.js @@ -0,0 +1,9 @@ +'use strict'; + +var gulp = require('gulp'); + +gulp.task('default', function(done) { + console.log('This gulpfile : ' + __filename); + console.log('The current directory : ' + process.cwd()); + done(); +}); diff --git a/test/fixtures/config/flags/silent/f/.gulp.json b/test/fixtures/config/flags/silent/f/.gulp.json new file mode 100644 index 00000000..4f3822ab --- /dev/null +++ b/test/fixtures/config/flags/silent/f/.gulp.json @@ -0,0 +1,5 @@ +{ + "flags": { + "silent": false + } +} diff --git a/test/fixtures/config/flags/silent/f/gulpfile.js b/test/fixtures/config/flags/silent/f/gulpfile.js new file mode 100644 index 00000000..42466b03 --- /dev/null +++ b/test/fixtures/config/flags/silent/f/gulpfile.js @@ -0,0 +1,7 @@ +'use strict'; + +var gulp = require('gulp'); + +gulp.task('default', function(done) { + done(); +}); diff --git a/test/fixtures/config/flags/silent/t/.gulp.json b/test/fixtures/config/flags/silent/t/.gulp.json new file mode 100644 index 00000000..9917bd8c --- /dev/null +++ b/test/fixtures/config/flags/silent/t/.gulp.json @@ -0,0 +1,5 @@ +{ + "flags": { + "silent": true + } +} diff --git a/test/fixtures/config/flags/silent/t/gulpfile.js b/test/fixtures/config/flags/silent/t/gulpfile.js new file mode 100644 index 00000000..42466b03 --- /dev/null +++ b/test/fixtures/config/flags/silent/t/gulpfile.js @@ -0,0 +1,7 @@ +'use strict'; + +var gulp = require('gulp'); + +gulp.task('default', function(done) { + done(); +}); diff --git a/test/lib/config-cfg-specs.js b/test/lib/config-cfg-specs.js new file mode 100644 index 00000000..a35b6fae --- /dev/null +++ b/test/lib/config-cfg-specs.js @@ -0,0 +1,69 @@ +'use strict'; + +var expect = require('expect'); +var path = require('path'); + +var configSpecs = require('../../lib/shared/config/cfg-specs'); + +describe('lib: config/cfg-specs', function() { + + it('description', function(done) { + var validate = configSpecs.description.validate; + expect(validate).toBeA('function'); + expect(validate('abc')).toEqual('abc'); + + expect(function() { + validate(null); + }).toThrow(TypeError, 'config.description must be a string: null'); + + expect(function() { + validate(0); + }).toThrow(TypeError, 'config.description must be a string: 0'); + + expect(function() { + validate(''); + }).toThrow(Error, 'config.description requires a value.'); + done(); + }); + + it('flags.silent', function(done) { + var validate = configSpecs['flags.silent'].validate; + expect(validate).toBeA('function'); + expect(validate(true)).toEqual(true); + expect(validate(false)).toEqual(false); + + expect(function() { + validate(null); + }).toThrow(TypeError, 'config.flags.silent must be a boolean: null'); + + expect(function() { + validate(0); + }).toThrow(TypeError, 'config.flags.silent must be a boolean: 0'); + + expect(function() { + validate(''); + }).toThrow(TypeError, 'config.flags.silent must be a boolean: '); + done(); + }); + + it('flags.gulpfile', function(done) { + var validate = configSpecs['flags.gulpfile'].validate; + expect(validate).toBeA('function'); + expect(validate('abc', 'path/to/config')).toEqual( + path.join(process.cwd(), 'path/to/abc')); + + expect(function() { + validate(null); + }).toThrow(TypeError, 'config.flags.gulpfile must be a string: null'); + + expect(function() { + validate(0); + }).toThrow(TypeError, 'config.flags.gulpfile must be a string: 0'); + + expect(function() { + validate(''); + }).toThrow(Error, 'config.flags.gulpfile requires a value.'); + done(); + }); + +}); diff --git a/test/lib/config-cli-flags.js b/test/lib/config-cli-flags.js new file mode 100644 index 00000000..e54c2c42 --- /dev/null +++ b/test/lib/config-cli-flags.js @@ -0,0 +1,152 @@ +'use strict'; + +var expect = require('expect'); +var mergeToCliFlags = require('../../lib/shared/config/cli-flags'); +var cliOptions = require('../../lib/shared/cliOptions'); + +var originalArgv = process.argv; + +describe('lib: config/cli-flag', function() { + + afterEach(function(done) { + process.argv = originalArgv; + done(); + }); + + describe('config.flags is not empty', function() { + + it('Should merge flags which is not specified by user', function(done) { + process.argv = originalArgv.slice(); + var config = { flags: { silent: true } }; + + var cliFlags = {}; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + silent: true, + }); + + cliFlags = { + help: false, + depth: 4, + tasks: false, + silent: false, + }; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + help: false, + depth: 4, + tasks: false, + silent: true, + }); + done(); + }); + + it('Should not merge flags which is specified by user', function(done) { + process.argv = originalArgv.concat([ + '--silent', + ]); + var config = { flags: { silent: true } }; + + var cliFlags = {}; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({}); + + cliFlags = { + help: false, + depth: 4, + tasks: false, + silent: false, + }; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + help: false, + depth: 4, + tasks: false, + silent: false, + }); + done(); + }); + + it('Should not merge flags of which alias is specified by user', + function(done) { + process.argv = originalArgv.concat([ + '-S', + ]); + var config = { flags: { silent: true } }; + + var cliFlags = {}; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({}); + + cliFlags = { + help: false, + depth: 4, + tasks: false, + silent: false, + }; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + help: false, + depth: 4, + tasks: false, + silent: false, + }); + done(); + }); + + }); + + describe('config.flags is empty', function() { + it('Should not cause error when user specified no arg', function(done) { + process.argv = originalArgv.slice(); + var config = {}; + + var cliFlags = {}; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({}); + + cliFlags = { + help: false, + depth: 4, + tasks: false, + silent: false, + }; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + help: false, + depth: 4, + tasks: false, + silent: false, + }); + done(); + }); + + it('Should not cause error when user specified args', function(done) { + process.argv = originalArgv.concat([ + '--silent', + ]); + var config = {}; + + var cliFlags = {}; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({}); + + cliFlags = { + help: false, + depth: 4, + tasks: false, + silent: false, + }; + mergeToCliFlags(cliFlags, config, cliOptions); + expect(cliFlags).toEqual({ + help: false, + depth: 4, + tasks: false, + silent: false, + }); + done(); + }); + }); + +}); + diff --git a/test/lib/config-env-flags.js b/test/lib/config-env-flags.js new file mode 100644 index 00000000..e2c01182 --- /dev/null +++ b/test/lib/config-env-flags.js @@ -0,0 +1,169 @@ +'use strict'; + +var expect = require('expect'); + +var mergeToEnvFlags = require('../../lib/shared/config/env-flags'); + +describe('lib: config/env-flags', function() { + + describe('config.flags is not empty', function() { + it('Should merge target flags which is not specified by user', + function(done) { + var envOpts = { + cwd: null, + configPath: null, + require: null, + completion: null, + }; + var config = { + flags: { gulpfile: 'path/to/gulpfile.js' }, + }; + + var envFlags = {}; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({ + configPath: 'path/to/gulpfile.js', + configBase: 'path/to', + }); + + envFlags = { + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({ + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'path/to/gulpfile.js', + configBase: 'path/to', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }); + done(); + }); + + it('Should not merge target flags which is specified by user', + function(done) { + var envOpts = { + cwd: '.', + configPath: 'x', + require: 'y', + completion: 'z', + }; + var config = { + flags: { gulpfile: 'path/to/gulpfile.js' }, + }; + + var envFlags = {}; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({}); + + envFlags = { + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({ + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }); + done(); + }); + }); + + describe('config.flags is empty', function() { + it('Should not case error when user specified no arg', function(done) { + var envOpts = { + cwd: null, + configPath: null, + require: null, + completion: null, + }; + var config = {}; + + var envFlags = {}; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({}); + + envFlags = { + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({ + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }); + done(); + }); + + it('Should not case error when user specified args', function(done) { + var envOpts = { + cwd: '.', + configPath: 'x', + require: 'y', + completion: 'z', + }; + var config = {}; + + var envFlags = {}; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({}); + + envFlags = { + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }; + mergeToEnvFlags(envFlags, config, envOpts); + expect(envFlags).toEqual({ + cwd: 'c/w/d', + require: ['r/e/q'], + configNameSearch: ['config/name/search'], + configPath: 'config/path', + configBase: 'config/base', + modulePath: 'module/path', + modulePackage: 'module/package', + configFiles: {}, + }); + done(); + }); + }); +}); diff --git a/test/lib/config-loadfiles.js b/test/lib/config-loadfiles.js new file mode 100644 index 00000000..8e2beee6 --- /dev/null +++ b/test/lib/config-loadfiles.js @@ -0,0 +1,38 @@ +'use strict'; + +var expect = require('expect'); +var path = require('path'); +var loadConfigFiles = require('../../lib/shared/config/loadfiles'); + +var fixturesDir = path.join(__dirname, '../fixtures/config'); + +describe('lib: config/loadfiles', function() { + + it('Should load config from files', function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'foo/bar/.gulp.json'), + b: path.join(fixturesDir, 'qux/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['a', 'b']); + + expect(config).toEqual({ + description: 'description by .gulp.js in directory qux', + }); + done(); + }); + + it('Should load config files in specified order', function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'foo/bar/.gulp.json'), + b: path.join(fixturesDir, 'qux/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['b', 'a']); + + expect(config).toEqual({ + description: 'Description by .gulp.json in directory foo/bar', + }); + done(); + }); +}); From 298426599884f8241d0ab8e31a89540539ff1add Mon Sep 17 00:00:00 2001 From: sttk Date: Mon, 23 Jan 2017 06:33:38 +0900 Subject: [PATCH 2/7] Modify matters pointed out. --- index.js | 32 ++-- lib/shared/config/cfg-specs.js | 40 ----- lib/shared/config/cli-flags.js | 26 +-- lib/shared/config/convert-config.js | 45 +++++ lib/shared/config/env-flags.js | 21 ++- lib/shared/config/exclude-cli-args.js | 34 ++++ lib/shared/config/load-files.js | 33 ++++ lib/shared/config/loadfiles.js | 33 ---- package.json | 5 +- test/fixtures/config/err/case1/.gulp.js | 4 + test/fixtures/config/err/case2/.gulp.js | 4 + test/fixtures/config/err/case3/.gulp.js | 7 + test/lib/config-cfg-specs.js | 69 -------- test/lib/config-cli-flags.js | 192 +++++++------------- test/lib/config-convert-config.js | 102 +++++++++++ test/lib/config-env-flags.js | 221 ++++++++---------------- test/lib/config-exclude-cli-args.js | 136 +++++++++++++++ test/lib/config-load-files.js | 122 +++++++++++++ test/lib/config-loadfiles.js | 38 ---- 19 files changed, 654 insertions(+), 510 deletions(-) delete mode 100644 lib/shared/config/cfg-specs.js create mode 100644 lib/shared/config/convert-config.js create mode 100644 lib/shared/config/exclude-cli-args.js create mode 100644 lib/shared/config/load-files.js delete mode 100644 lib/shared/config/loadfiles.js create mode 100644 test/fixtures/config/err/case1/.gulp.js create mode 100644 test/fixtures/config/err/case2/.gulp.js create mode 100644 test/fixtures/config/err/case3/.gulp.js delete mode 100644 test/lib/config-cfg-specs.js create mode 100644 test/lib/config-convert-config.js create mode 100644 test/lib/config-exclude-cli-args.js create mode 100644 test/lib/config-load-files.js delete mode 100644 test/lib/config-loadfiles.js diff --git a/index.js b/index.js index c8a6ebbd..94e47f12 100644 --- a/index.js +++ b/index.js @@ -19,9 +19,9 @@ var cliVersion = require('./package.json').version; var getBlacklist = require('./lib/shared/getBlacklist'); var toConsole = require('./lib/shared/log/toConsole'); -var loadConfigFiles = require('./lib/shared/config/loadfiles'); -var mergeToCliFlags = require('./lib/shared/config/cli-flags'); -var mergeToEnvFlags = require('./lib/shared/config/env-flags'); +var loadConfigFiles = require('./lib/shared/config/load-files'); +var mergeConfigToCliFlags = require('./lib/shared/config/cli-flags'); +var mergeConfigToEnvFlags = require('./lib/shared/config/env-flags'); // Logging functions var logVerify = require('./lib/shared/log/verify'); @@ -85,31 +85,23 @@ cli.on('respawn', function(flags, child) { }); function run() { - var envOpts = { + cli.launch({ cwd: opts.cwd, configPath: opts.gulpfile, require: opts.require, completion: opts.completion, - }; - - cli.launch(envOpts, function(env) { - var config; - try { - config = loadConfigFiles(env.configFiles['.gulp'], ['home', 'cwd']); - } catch (e) { - log.error(chalk.red(e.message)); - exit(1); - } - - mergeToCliFlags(opts, config, cliOptions); - mergeToEnvFlags(env, config, envOpts); - - handleArguments(env, opts, config); - }); + }, configureAndInvoke); } module.exports = run; +function configureAndInvoke(env) { + var config = loadConfigFiles(env.configFiles['.gulp'], ['home', 'cwd']); + opts = mergeConfigToCliFlags(opts, config); + env = mergeConfigToEnvFlags(env, config); + handleArguments(env, opts, config); +} + // The actual logic function handleArguments(env, opts, cfg) { // Set up event listeners for logging. diff --git a/lib/shared/config/cfg-specs.js b/lib/shared/config/cfg-specs.js deleted file mode 100644 index af0414a7..00000000 --- a/lib/shared/config/cfg-specs.js +++ /dev/null @@ -1,40 +0,0 @@ -'use strict'; - -var path = require('path'); - -module.exports = { - - description: { - validate: function(value) { - if (typeof value !== 'string') { - throw new TypeError('config.description must be a string: ' + value); - } - if (!value) { - throw new Error('config.description requires a value.'); - } - return value; - }, - }, - - 'flags.silent': { - validate: function(value) { - if (typeof value !== 'boolean') { - throw new TypeError('config.flags.silent must be a boolean: ' + value); - } - return value; - }, - }, - - 'flags.gulpfile': { - validate: function(value, configFile) { - if (typeof value !== 'string') { - throw new TypeError( - 'config.flags.gulpfile must be a string: ' + value); - } - if (!value) { - throw new Error('config.flags.gulpfile requires a value.'); - } - return path.resolve(path.dirname(configFile), value); - }, - }, -}; diff --git a/lib/shared/config/cli-flags.js b/lib/shared/config/cli-flags.js index 8fc37f8d..786df38e 100644 --- a/lib/shared/config/cli-flags.js +++ b/lib/shared/config/cli-flags.js @@ -1,24 +1,14 @@ 'use strict'; -var yargs = require('yargs'); -var camelCase = require('lodash.camelcase'); +var copyProps = require('copy-props'); -function mergeToCliFlags(cliFlags, config, cliOptions) { - var argv = yargs(process.argv.slice(2)).argv; - var cfgFlags = config.flags || {}; +var fromto = { + 'flags.silent': 'silent', +}; - var optNames = ['silent']; - optNames.filter(excludeArgsByUser).map(camelCase).forEach(copyConfig); - - function excludeArgsByUser(name) { - return !(argv[name] != null || argv[cliOptions[name].alias] != null); - } - - function copyConfig(name) { - if (cfgFlags[name] !== undefined) { - cliFlags[name] = cfgFlags[name]; - } - } +function mergeConfigToCliFlags(opt, config) { + opt = copyProps(opt, {}); + return copyProps(config, opt, fromto); } -module.exports = mergeToCliFlags; +module.exports = mergeConfigToCliFlags; diff --git a/lib/shared/config/convert-config.js b/lib/shared/config/convert-config.js new file mode 100644 index 00000000..7c859d79 --- /dev/null +++ b/lib/shared/config/convert-config.js @@ -0,0 +1,45 @@ +'use strict'; + +var path = require('path'); +var logger = require('fancy-log'); +var chalk = require('chalk'); + +module.exports = { + + description: function(value, configFile) { + if (typeof value !== 'string') { + logger.error(chalk.red( + configFile + ': config.description must be a string: ' + value)); + return undefined; + } + if (!value.length) { + logger.error(chalk.red( + configFile + ': config.description requires a value.')); + return undefined; + } + return value; + }, + + 'flags.silent': function(value, configFile) { + if (typeof value !== 'boolean') { + logger.error(chalk.red( + configFile + ': config.flags.silent must be a boolean: ' + value)); + return undefined; + } + return value; + }, + + 'flags.gulpfile': function(value, configFile) { + if (typeof value !== 'string') { + logger.error(chalk.red( + configFile + ': config.flags.gulpfile must be a string: ' + value)); + return undefined; + } + if (!value.length) { + logger.error(chalk.red( + configFile + ': config.flags.gulpfile requires a value.')); + return undefined; + } + return path.resolve(path.dirname(configFile), value); + }, +}; diff --git a/lib/shared/config/env-flags.js b/lib/shared/config/env-flags.js index 973b493b..2e5a6f28 100644 --- a/lib/shared/config/env-flags.js +++ b/lib/shared/config/env-flags.js @@ -1,14 +1,23 @@ 'use strict'; var path = require('path'); +var copyProps = require('copy-props'); -function mergeToEnvFlags(envFlags, config, envOpts) { - var cfgFlags = config.flags || {}; +var fromto = { + configPath: 'flags.gulpfile', + configBase: 'flags.gulpfile', +}; - if (!envOpts.configPath && cfgFlags.gulpfile) { - envFlags.configPath = cfgFlags.gulpfile; - envFlags.configBase = path.dirname(cfgFlags.gulpfile); +function mergeConfigToEnvFlags(env, config) { + env = copyProps(env, {}); + return copyProps(env, config, fromto, convert, true); +} + +function convert(value, configKey, envKey) { + if (envKey === 'configBase') { + return path.dirname(value); } + return value; } -module.exports = mergeToEnvFlags; +module.exports = mergeConfigToEnvFlags; diff --git a/lib/shared/config/exclude-cli-args.js b/lib/shared/config/exclude-cli-args.js new file mode 100644 index 00000000..b5a3a9eb --- /dev/null +++ b/lib/shared/config/exclude-cli-args.js @@ -0,0 +1,34 @@ +'use struct'; + +var yargs = require('yargs'); +var camelCase = require('camelcase'); +var copyProps = require('copy-props'); +var cliOptions = require('../cliOptions'); + +function excludeCliArgs(config) { + var argv = yargs(process.argv.slice(2)).argv; + + var excludedNames = Object.keys(cliOptions) + .filter(isSpecified) + .map(toConfigName); + + return copyProps(config, {}, exclude); + + + function isSpecified(name) { + return (argv[name] != null || argv[cliOptions[name].alias] != null); + } + + function exclude(value, name) { + if (excludedNames.indexOf(name) >= 0) { + return undefined; + } + return value; + } +} + +function toConfigName(name) { + return 'flags.' + camelCase(name); +} + +module.exports = excludeCliArgs; diff --git a/lib/shared/config/load-files.js b/lib/shared/config/load-files.js new file mode 100644 index 00000000..22f7d36f --- /dev/null +++ b/lib/shared/config/load-files.js @@ -0,0 +1,33 @@ +'use strict'; + +var copyProps = require('copy-props'); +var convertConfig = require('./convert-config'); +var excludeCliArgs = require('./exclude-cli-args'); + +function loadConfigFiles(configFilesBase, configFileTitles) { + var config = {}; + + configFileTitles.forEach(function(fileTitle) { + var filePath = configFilesBase[fileTitle]; + if (!filePath) { + return; + } + + copyProps(require(filePath), config, function(value, name) { + if (value == null) { + return undefined; + } + var convert = convertConfig[name]; + if (typeof convert !== 'function') { + return value; + } + return convert(value, filePath); + }); + }); + + config = excludeCliArgs(config); + + return config; +} + +module.exports = loadConfigFiles; diff --git a/lib/shared/config/loadfiles.js b/lib/shared/config/loadfiles.js deleted file mode 100644 index 47689433..00000000 --- a/lib/shared/config/loadfiles.js +++ /dev/null @@ -1,33 +0,0 @@ -'use strict'; - -var get = require('lodash.get'); -var set = require('lodash.set'); -var isString = require('lodash.isstring'); -var configSpecs = require('./cfg-specs'); - -function loadConfigFiles(configFilesBase, keysInOrder) { - var config = {}; - - var configFilePaths = keysInOrder.map(function(key) { - return configFilesBase[key]; - }).filter(isString); - - configFilePaths.forEach(function(filePath) { - var loadedCfg = require(filePath); - - Object.keys(configSpecs).forEach(function(keyPath) { - var spec = configSpecs[keyPath]; - - var value = get(loadedCfg, keyPath); - if (value === undefined) { - return; - } - - set(config, keyPath, spec.validate(value, filePath)); - }); - }); - - return config; -} - -module.exports = loadConfigFiles; diff --git a/package.json b/package.json index cdbdef3a..63ab5b5a 100644 --- a/package.json +++ b/package.json @@ -33,17 +33,16 @@ }, "dependencies": { "archy": "^1.0.0", + "camelcase": "^3.0.0", "chalk": "^1.1.0", + "copy-props": "^1.4.1", "fancy-log": "^1.1.0", "gulplog": "^1.0.0", "interpret": "^1.0.0", "liftoff": "^2.3.0", - "lodash.camelcase": "^4.3.0", - "lodash.get": "^4.4.2", "lodash.isfunction": "^3.0.8", "lodash.isplainobject": "^4.0.4", "lodash.isstring": "^4.0.1", - "lodash.set": "^4.3.2", "lodash.sortby": "^4.5.0", "matchdep": "^1.0.0", "mute-stdout": "^1.0.0", diff --git a/test/fixtures/config/err/case1/.gulp.js b/test/fixtures/config/err/case1/.gulp.js new file mode 100644 index 00000000..5e3b7545 --- /dev/null +++ b/test/fixtures/config/err/case1/.gulp.js @@ -0,0 +1,4 @@ +module.exports = { + description: null, + 'non-exist-in-converter': null, +}; diff --git a/test/fixtures/config/err/case2/.gulp.js b/test/fixtures/config/err/case2/.gulp.js new file mode 100644 index 00000000..932f15cc --- /dev/null +++ b/test/fixtures/config/err/case2/.gulp.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'DESC.', + 'non-exist-in-converter': 'ABC', +}; diff --git a/test/fixtures/config/err/case3/.gulp.js b/test/fixtures/config/err/case3/.gulp.js new file mode 100644 index 00000000..e0e3e51a --- /dev/null +++ b/test/fixtures/config/err/case3/.gulp.js @@ -0,0 +1,7 @@ +module.exports = { + description: true, + flags: { + silent: 123, + gulpfile: '', + }, +}; diff --git a/test/lib/config-cfg-specs.js b/test/lib/config-cfg-specs.js deleted file mode 100644 index a35b6fae..00000000 --- a/test/lib/config-cfg-specs.js +++ /dev/null @@ -1,69 +0,0 @@ -'use strict'; - -var expect = require('expect'); -var path = require('path'); - -var configSpecs = require('../../lib/shared/config/cfg-specs'); - -describe('lib: config/cfg-specs', function() { - - it('description', function(done) { - var validate = configSpecs.description.validate; - expect(validate).toBeA('function'); - expect(validate('abc')).toEqual('abc'); - - expect(function() { - validate(null); - }).toThrow(TypeError, 'config.description must be a string: null'); - - expect(function() { - validate(0); - }).toThrow(TypeError, 'config.description must be a string: 0'); - - expect(function() { - validate(''); - }).toThrow(Error, 'config.description requires a value.'); - done(); - }); - - it('flags.silent', function(done) { - var validate = configSpecs['flags.silent'].validate; - expect(validate).toBeA('function'); - expect(validate(true)).toEqual(true); - expect(validate(false)).toEqual(false); - - expect(function() { - validate(null); - }).toThrow(TypeError, 'config.flags.silent must be a boolean: null'); - - expect(function() { - validate(0); - }).toThrow(TypeError, 'config.flags.silent must be a boolean: 0'); - - expect(function() { - validate(''); - }).toThrow(TypeError, 'config.flags.silent must be a boolean: '); - done(); - }); - - it('flags.gulpfile', function(done) { - var validate = configSpecs['flags.gulpfile'].validate; - expect(validate).toBeA('function'); - expect(validate('abc', 'path/to/config')).toEqual( - path.join(process.cwd(), 'path/to/abc')); - - expect(function() { - validate(null); - }).toThrow(TypeError, 'config.flags.gulpfile must be a string: null'); - - expect(function() { - validate(0); - }).toThrow(TypeError, 'config.flags.gulpfile must be a string: 0'); - - expect(function() { - validate(''); - }).toThrow(Error, 'config.flags.gulpfile requires a value.'); - done(); - }); - -}); diff --git a/test/lib/config-cli-flags.js b/test/lib/config-cli-flags.js index e54c2c42..4a5e8a3d 100644 --- a/test/lib/config-cli-flags.js +++ b/test/lib/config-cli-flags.js @@ -1,151 +1,75 @@ 'use strict'; var expect = require('expect'); -var mergeToCliFlags = require('../../lib/shared/config/cli-flags'); -var cliOptions = require('../../lib/shared/cliOptions'); +var mergeConfig = require('../../lib/shared/config/cli-flags'); -var originalArgv = process.argv; +describe('lib: config/cli-flags', function() { -describe('lib: config/cli-flag', function() { + it('Should copy only config props specified to cli flags', function(done) { + var opts = {}; - afterEach(function(done) { - process.argv = originalArgv; - done(); - }); - - describe('config.flags is not empty', function() { - - it('Should merge flags which is not specified by user', function(done) { - process.argv = originalArgv.slice(); - var config = { flags: { silent: true } }; - - var cliFlags = {}; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - silent: true, - }); - - cliFlags = { - help: false, - depth: 4, - tasks: false, - silent: false, - }; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - help: false, - depth: 4, - tasks: false, + var config = { + description: 'DESCRIPTION.', + flags: { silent: true, - }); - done(); - }); - - it('Should not merge flags which is specified by user', function(done) { - process.argv = originalArgv.concat([ - '--silent', - ]); - var config = { flags: { silent: true } }; - - var cliFlags = {}; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({}); - - cliFlags = { - help: false, - depth: 4, - tasks: false, - silent: false, - }; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - help: false, - depth: 4, - tasks: false, - silent: false, - }); - done(); - }); - - it('Should not merge flags of which alias is specified by user', - function(done) { - process.argv = originalArgv.concat([ - '-S', - ]); - var config = { flags: { silent: true } }; - - var cliFlags = {}; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({}); - - cliFlags = { - help: false, - depth: 4, - tasks: false, - silent: false, - }; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - help: false, - depth: 4, - tasks: false, - silent: false, - }); - done(); + gulpfile: '/path/to/gulpfile', + }, + }; + + var result = mergeConfig(opts, config); + expect(result).toNotBe(opts); + expect(result).toEqual({ + silent: true, }); - + done(); }); - describe('config.flags is empty', function() { - it('Should not cause error when user specified no arg', function(done) { - process.argv = originalArgv.slice(); - var config = {}; - - var cliFlags = {}; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({}); - - cliFlags = { - help: false, - depth: 4, - tasks: false, + it('Should override cli flags with config props', function(done) { + var opts = { + help: false, + depth: 4, + silent: true, + tasks: false, + }; + + var config = { + description: 'DESCRIPTION.', + flags: { silent: false, - }; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - help: false, - depth: 4, - tasks: false, - silent: false, - }); - done(); + gulpfile: '/path/to/gulpfile', + }, + }; + + var result = mergeConfig(opts, config); + expect(result).toNotBe(opts); + expect(result).toEqual({ + help: false, + depth: 4, + silent: false, + tasks: false, }); + done(); + }); - it('Should not cause error when user specified args', function(done) { - process.argv = originalArgv.concat([ - '--silent', - ]); - var config = {}; - - var cliFlags = {}; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({}); - - cliFlags = { - help: false, - depth: 4, - tasks: false, - silent: false, - }; - mergeToCliFlags(cliFlags, config, cliOptions); - expect(cliFlags).toEqual({ - help: false, - depth: 4, - tasks: false, - silent: false, - }); - done(); + it('Should not cause error if config is empty', function(done) { + var opts = { + help: false, + depth: 4, + silent: true, + tasks: false, + }; + + var config = {}; + + var result = mergeConfig(opts, config); + expect(result).toNotBe(opts); + expect(result).toEqual({ + help: false, + depth: 4, + silent: true, + tasks: false, }); + done(); }); }); diff --git a/test/lib/config-convert-config.js b/test/lib/config-convert-config.js new file mode 100644 index 00000000..d02f648e --- /dev/null +++ b/test/lib/config-convert-config.js @@ -0,0 +1,102 @@ +'use strict'; + +var expect = require('expect'); +var path = require('path'); +var convertConfig = require('../../lib/shared/config/convert-config'); + +var chalk = require('chalk'); +var red = chalk.red; + +var logger = require('fancy-log'); +var origLogError = logger.error; +var logs; + +describe('lib: config/convert-config', function() { + + beforeEach(function() { + logs = []; + logger.error = function(log) { + logs.push(log); + }; + }); + + afterEach(function() { + logs.error = origLogError; + }); + + it('description', function(done) { + var convert = convertConfig.description; + expect(convert).toBeA('function'); + + expect(convert('abc', 'config/file/path')).toEqual('abc'); + expect(logs.length).toEqual(0); + + expect(convert(null, 'config/file/path')).toBeA('undefined'); + expect(logs.length).toEqual(1); + + expect(convert(0, 'config/file/path')).toBeA('undefined'); + expect(logs.length).toEqual(2); + + expect(convert('', 'config/file/path')).toBeA('undefined'); + expect(logs.length).toEqual(3); + + expect(logs).toEqual([ + red('config/file/path: config.description must be a string: null'), + red('config/file/path: config.description must be a string: 0'), + red('config/file/path: config.description requires a value.'), + ]); + + done(); + }); + + it('flags.silent', function(done) { + var convert = convertConfig['flags.silent']; + expect(convert).toBeA('function'); + + expect(convert(true, 'file/path')).toEqual(true); + expect(convert(false, 'file/path')).toEqual(false); + expect(logs.length).toEqual(0); + + expect(convert(null, 'file/path')).toBeA('undefined'); + expect(logs.length).toEqual(1); + + expect(convert(0, 'file/path')).toBeA('undefined'); + expect(logs.length).toEqual(2); + + expect(convert('', 'file/path')).toBeA('undefined'); + expect(logs.length).toEqual(3); + + expect(logs).toEqual([ + red('file/path: config.flags.silent must be a boolean: null'), + red('file/path: config.flags.silent must be a boolean: 0'), + red('file/path: config.flags.silent must be a boolean: '), + ]); + done(); + }); + + it('flags.gulpfile', function(done) { + var convert = convertConfig['flags.gulpfile']; + expect(convert).toBeA('function'); + + expect(convert('abc', 'path/to/config')).toEqual( + path.join(process.cwd(), 'path/to/abc')); + expect(logs.length).toEqual(0); + + expect(convert(null, 'path/to/config')).toBeA('undefined'); + expect(logs.length).toEqual(1); + + expect(convert(0, 'path/to/config')).toBeA('undefined'); + expect(logs.length).toEqual(2); + + expect(convert('', 'path/to/config')).toBeA('undefined'); + expect(logs.length).toEqual(3); + + expect(logs).toEqual([ + red('path/to/config: config.flags.gulpfile must be a string: null'), + red('path/to/config: config.flags.gulpfile must be a string: 0'), + red('path/to/config: config.flags.gulpfile requires a value.'), + ]); + done(); + }); + +}); diff --git a/test/lib/config-env-flags.js b/test/lib/config-env-flags.js index e2c01182..349a34d2 100644 --- a/test/lib/config-env-flags.js +++ b/test/lib/config-env-flags.js @@ -1,169 +1,92 @@ 'use strict'; var expect = require('expect'); - -var mergeToEnvFlags = require('../../lib/shared/config/env-flags'); +var mergeConfig = require('../../lib/shared/config/env-flags'); describe('lib: config/env-flags', function() { - describe('config.flags is not empty', function() { - it('Should merge target flags which is not specified by user', - function(done) { - var envOpts = { - cwd: null, - configPath: null, - require: null, - completion: null, - }; - var config = { - flags: { gulpfile: 'path/to/gulpfile.js' }, - }; + it('Should copy only config props specified to env flags', function(done) { + var env = {}; - var envFlags = {}; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({ - configPath: 'path/to/gulpfile.js', - configBase: 'path/to', - }); + var config = { + description: 'DESCRIPTION.', + flags: { + silent: true, + gulpfile: '/path/to/gulpfile', + }, + }; - envFlags = { - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({ - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'path/to/gulpfile.js', - configBase: 'path/to', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }); - done(); - }); - - it('Should not merge target flags which is specified by user', - function(done) { - var envOpts = { - cwd: '.', - configPath: 'x', - require: 'y', - completion: 'z', - }; - var config = { - flags: { gulpfile: 'path/to/gulpfile.js' }, - }; - - var envFlags = {}; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({}); - - envFlags = { - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({ - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }); - done(); + var result = mergeConfig(env, config); + expect(result).toNotBe(env); + expect(result).toEqual({ + configPath: '/path/to/gulpfile', + configBase: '/path/to', }); + done(); }); - describe('config.flags is empty', function() { - it('Should not case error when user specified no arg', function(done) { - var envOpts = { - cwd: null, - configPath: null, - require: null, - completion: null, - }; - var config = {}; + it('Should override env flags with config props', function(done) { + var env = { + cwd: '/path/to/cwd', + require: 'preload', + configNameSearch: 'configNameSearch', + configPath: '/path/of/config/path', + configBase: '/path/of/config/base', + modulePath: '/path/of/module/path', + modulePackage: { name: 'modulePackage' }, + configFiles: { aaa: {} }, + }; - var envFlags = {}; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({}); + var config = { + description: 'DESCRIPTION.', + flags: { + silent: false, + gulpfile: '/path/to/gulpfile', + }, + }; - envFlags = { - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({ - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }); - done(); + var result = mergeConfig(env, config); + expect(result).toNotBe(env); + expect(result).toEqual({ + cwd: '/path/to/cwd', + require: 'preload', + configNameSearch: 'configNameSearch', + configPath: '/path/to/gulpfile', + configBase: '/path/to', + modulePath: '/path/of/module/path', + modulePackage: { name: 'modulePackage' }, + configFiles: { aaa: {} }, }); + done(); + }); - it('Should not case error when user specified args', function(done) { - var envOpts = { - cwd: '.', - configPath: 'x', - require: 'y', - completion: 'z', - }; - var config = {}; + it('Should not cause error if config is empty', function(done) { + var env = { + cwd: '/path/to/cwd', + require: 'preload', + configNameSearch: 'configNameSearch', + configPath: '/path/of/config/path', + configBase: '/path/of/config/base', + modulePath: '/path/of/module/path', + modulePackage: { name: 'modulePackage' }, + configFiles: { aaa: {} }, + }; - var envFlags = {}; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({}); + var config = {}; - envFlags = { - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }; - mergeToEnvFlags(envFlags, config, envOpts); - expect(envFlags).toEqual({ - cwd: 'c/w/d', - require: ['r/e/q'], - configNameSearch: ['config/name/search'], - configPath: 'config/path', - configBase: 'config/base', - modulePath: 'module/path', - modulePackage: 'module/package', - configFiles: {}, - }); - done(); + var result = mergeConfig(env, config); + expect(result).toNotBe(env); + expect(result).toEqual({ + cwd: '/path/to/cwd', + require: 'preload', + configNameSearch: 'configNameSearch', + configPath: '/path/of/config/path', + configBase: '/path/of/config/base', + modulePath: '/path/of/module/path', + modulePackage: { name: 'modulePackage' }, + configFiles: { aaa: {} }, }); + done(); }); + }); diff --git a/test/lib/config-exclude-cli-args.js b/test/lib/config-exclude-cli-args.js new file mode 100644 index 00000000..a9a8199c --- /dev/null +++ b/test/lib/config-exclude-cli-args.js @@ -0,0 +1,136 @@ +'use strict'; + +var expect = require('expect'); +var excludeCliArgs = require('../../lib/shared/config/exclude-cli-args'); + +var originalArgv = process.argv; + +describe('lib: config/exclude-cli-args', function() { + + afterEach(function(done) { + process.argv = originalArgv; + done(); + }); + + describe('config.flags is not empty', function() { + + it('Should remain config flags which is not specified by cli args', + function(done) { + process.argv = originalArgv.concat(); + + var config = { flags: { silent: true } }; + var result = excludeCliArgs(config); + expect(result).toEqual({ flags: { silent: true } }); + + config = { + flags: { + help: false, + depth: 4, + tasks: false, + silent: true, + }, + }; + result = excludeCliArgs(config); + expect(result).toEqual({ + flags: { + help: false, + depth: 4, + tasks: false, + silent: true, + }, + }); + done(); + }); + + it('Should remove config flags which is specified by cli args', + function(done) { + process.argv = originalArgv.concat([ + '--silent', + ]); + var config = { flags: { silent: true } }; + + var result = excludeCliArgs(config); + expect(result).toEqual({ flags: {} }); + + config = { + flags: { + help: false, + depth: 4, + tasks: false, + silent: true, + }, + }; + result = excludeCliArgs(config); + expect(result).toEqual({ + flags: { + help: false, + depth: 4, + tasks: false, + }, + }); + done(); + }); + + it('Should remove config flags of which alias is specified by cli args', + function(done) { + process.argv = originalArgv.concat([ + '-S', + ]); + var config = { flags: { silent: true } }; + + var result = excludeCliArgs(config); + expect(result).toEqual({ flags: {} }); + + config = { + flags: { + help: false, + depth: 4, + tasks: false, + silent: true, + }, + }; + result = excludeCliArgs(config); + expect(result).toEqual({ + flags: { + help: false, + depth: 4, + tasks: false, + }, + }); + done(); + }); + + }); + + describe('config.flags is empty', function() { + + it('Should cause nothing when user specified no arg', function(done) { + process.argv = originalArgv.concat(); + + var config = {}; + var result = excludeCliArgs(config); + expect(result).toEqual({}); + + config = null; + var result = excludeCliArgs(config); + expect(result).toEqual({}); + done(); + }); + + it('Should cause nothing when user specified args', function(done) { + process.argv = originalArgv.concat([ + '--silent', + ]); + + var config = {}; + var result = excludeCliArgs(config); + expect(result).toEqual({}); + + config = null; + var result = excludeCliArgs(config); + expect(result).toEqual({}); + done(); + }); + }); + +}); diff --git a/test/lib/config-load-files.js b/test/lib/config-load-files.js new file mode 100644 index 00000000..2e7a916f --- /dev/null +++ b/test/lib/config-load-files.js @@ -0,0 +1,122 @@ +'use strict'; + +var expect = require('expect'); +var path = require('path'); +var loadConfigFiles = require('../../lib/shared/config/load-files'); + +var chalk = require('chalk'); +var red = chalk.red; + +var fixturesDir = path.join(__dirname, '../fixtures/config'); + +var logger = require('fancy-log'); +var origLogError = logger.error; +var logs; + +describe('lib: config/load-files', function() { + + beforeEach(function() { + logs = []; + logger.error = function(log) { + logs.push(log); + }; + }); + + afterEach(function() { + logger.error = origLogError; + }); + + it('Should load config from files', function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'foo/bar/.gulp.json'), + b: null, + c: path.join(fixturesDir, 'qux/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['a', 'b', 'c']); + + expect(config).toEqual({ + description: 'description by .gulp.js in directory qux', + }); + expect(logs.length).toEqual(0); + done(); + }); + + it('Should load config files in specified order', function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'foo/bar/.gulp.json'), + b: null, + c: path.join(fixturesDir, 'qux/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['b', 'c', 'a']); + + expect(config).toEqual({ + description: 'Description by .gulp.json in directory foo/bar', + }); + expect(logs.length).toEqual(0); + done(); + }); + + it('Should load and convert a config prop if it is a path', function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'flags/gulpfile/.gulp.json'), + }; + + var config = loadConfigFiles(configFilesBase, ['a']); + + expect(config).toEqual({ + flags: { + gulpfile: path.join(fixturesDir, + 'flags/gulpfile/is/here/mygulpfile.js'), + }, + }); + expect(logs.length).toEqual(0); + done(); + }); + + it('Should ignore if a config prop value is null or undefined', + function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'err/case1/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['a']); + + expect(config).toEqual({}); + expect(logs.length).toEqual(0); + done(); + }); + + it('Should let pass if a config prop is not defined in converter', + function(done) { + var configFilesBase = { + a: path.join(fixturesDir, 'err/case2/.gulp.js'), + }; + + var config = loadConfigFiles(configFilesBase, ['a']); + + expect(config).toEqual({ + description: 'DESC.', + 'non-exist-in-converter': 'ABC', + }); + expect(logs.length).toEqual(0); + done(); + }); + + it('Should output an error log and ignore if a config prop value is invalid', + function(done) { + var fp = path.join(fixturesDir, 'err/case3/.gulp.js'); + var configFilesBase = { a: fp }; + var config = loadConfigFiles(configFilesBase, ['a']); + + expect(config).toEqual({ flags: {} }); + expect(logs).toEqual([ + red(fp + ': config.description must be a string: true'), + red(fp + ': config.flags.silent must be a boolean: 123'), + red(fp + ': config.flags.gulpfile requires a value.'), + ]); + done(); + }); + +}); diff --git a/test/lib/config-loadfiles.js b/test/lib/config-loadfiles.js deleted file mode 100644 index 8e2beee6..00000000 --- a/test/lib/config-loadfiles.js +++ /dev/null @@ -1,38 +0,0 @@ -'use strict'; - -var expect = require('expect'); -var path = require('path'); -var loadConfigFiles = require('../../lib/shared/config/loadfiles'); - -var fixturesDir = path.join(__dirname, '../fixtures/config'); - -describe('lib: config/loadfiles', function() { - - it('Should load config from files', function(done) { - var configFilesBase = { - a: path.join(fixturesDir, 'foo/bar/.gulp.json'), - b: path.join(fixturesDir, 'qux/.gulp.js'), - }; - - var config = loadConfigFiles(configFilesBase, ['a', 'b']); - - expect(config).toEqual({ - description: 'description by .gulp.js in directory qux', - }); - done(); - }); - - it('Should load config files in specified order', function(done) { - var configFilesBase = { - a: path.join(fixturesDir, 'foo/bar/.gulp.json'), - b: path.join(fixturesDir, 'qux/.gulp.js'), - }; - - var config = loadConfigFiles(configFilesBase, ['b', 'a']); - - expect(config).toEqual({ - description: 'Description by .gulp.json in directory foo/bar', - }); - done(); - }); -}); From 9337fd69a6c6d9368420f913b19e742cb075bbf5 Mon Sep 17 00:00:00 2001 From: sttk Date: Sat, 22 Apr 2017 05:22:47 +0900 Subject: [PATCH 3/7] Rename argument names of load-files --- lib/shared/config/load-files.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/shared/config/load-files.js b/lib/shared/config/load-files.js index 22f7d36f..d2ccc3f8 100644 --- a/lib/shared/config/load-files.js +++ b/lib/shared/config/load-files.js @@ -4,11 +4,11 @@ var copyProps = require('copy-props'); var convertConfig = require('./convert-config'); var excludeCliArgs = require('./exclude-cli-args'); -function loadConfigFiles(configFilesBase, configFileTitles) { +function loadConfigFiles(configFiles, configFileOrder) { var config = {}; - configFileTitles.forEach(function(fileTitle) { - var filePath = configFilesBase[fileTitle]; + configFileOrder.forEach(function(key) { + var filePath = configFiles[key]; if (!filePath) { return; } From e5bafdd6ea699d6fc055249eac3773e66a8fb9e1 Mon Sep 17 00:00:00 2001 From: sttk Date: Sat, 22 Apr 2017 13:20:42 +0900 Subject: [PATCH 4/7] Remove exclude-cli-args --- lib/shared/config/exclude-cli-args.js | 34 ------- lib/shared/config/load-files.js | 3 - test/config-flags-silent.js | 66 ------------- test/lib/config-exclude-cli-args.js | 136 -------------------------- 4 files changed, 239 deletions(-) delete mode 100644 lib/shared/config/exclude-cli-args.js delete mode 100644 test/lib/config-exclude-cli-args.js diff --git a/lib/shared/config/exclude-cli-args.js b/lib/shared/config/exclude-cli-args.js deleted file mode 100644 index b5a3a9eb..00000000 --- a/lib/shared/config/exclude-cli-args.js +++ /dev/null @@ -1,34 +0,0 @@ -'use struct'; - -var yargs = require('yargs'); -var camelCase = require('camelcase'); -var copyProps = require('copy-props'); -var cliOptions = require('../cliOptions'); - -function excludeCliArgs(config) { - var argv = yargs(process.argv.slice(2)).argv; - - var excludedNames = Object.keys(cliOptions) - .filter(isSpecified) - .map(toConfigName); - - return copyProps(config, {}, exclude); - - - function isSpecified(name) { - return (argv[name] != null || argv[cliOptions[name].alias] != null); - } - - function exclude(value, name) { - if (excludedNames.indexOf(name) >= 0) { - return undefined; - } - return value; - } -} - -function toConfigName(name) { - return 'flags.' + camelCase(name); -} - -module.exports = excludeCliArgs; diff --git a/lib/shared/config/load-files.js b/lib/shared/config/load-files.js index d2ccc3f8..3b42d7fc 100644 --- a/lib/shared/config/load-files.js +++ b/lib/shared/config/load-files.js @@ -2,7 +2,6 @@ var copyProps = require('copy-props'); var convertConfig = require('./convert-config'); -var excludeCliArgs = require('./exclude-cli-args'); function loadConfigFiles(configFiles, configFileOrder) { var config = {}; @@ -25,8 +24,6 @@ function loadConfigFiles(configFiles, configFileOrder) { }); }); - config = excludeCliArgs(config); - return config; } diff --git a/test/config-flags-silent.js b/test/config-flags-silent.js index 13b9ad83..8f22f402 100644 --- a/test/config-flags-silent.js +++ b/test/config-flags-silent.js @@ -44,70 +44,4 @@ describe('config: flags.silent', function() { } }); - it('Should be silent with --silent event if `flags.silent` is false in ' + - '\n\t.gulp.*', function(done) { - runner - .chdir('flags/silent/f') - .gulp('--silent') - .run(cb); - - function cb(err, stdout, stderr) { - expect(stdout).toEqual(''); - expect(stderr).toEqual(''); - done(err); - } - }); - - it('Should be silent with -S event if `flags.silent` is false in ' + - '\n\t.gulp.*', function(done) { - runner - .chdir('flags/silent/f') - .gulp('-S') - .run(cb); - - function cb(err, stdout, stderr) { - expect(stdout).toEqual(''); - expect(stderr).toEqual(''); - done(err); - } - }); - - it('Should not be silent with --silent=false event if `flags.silent` is ' + - 'true\n\tin .gulp.*', function(done) { - runner - .chdir('flags/silent/t') - .gulp('--silent=false') - .run(cb); - - function cb(err, stdout, stderr) { - stdout = eraseLapse(eraseTime(skipLines(stdout, 1))); - expect(stdout).toEqual( - 'Starting \'default\'...\n' + - 'Finished \'default\' after ?\n' + - '' - ); - expect(stderr).toEqual(''); - done(err); - } - }); - - it('Should not be silent with -S false event if `flags.silent` is true in ' + - '\n\t.gulp.*', function(done) { - runner - .chdir('flags/silent/t') - .gulp('-S false') - .run(cb); - - function cb(err, stdout, stderr) { - stdout = eraseLapse(eraseTime(skipLines(stdout, 1))); - expect(stdout).toEqual( - 'Starting \'default\'...\n' + - 'Finished \'default\' after ?\n' + - '' - ); - expect(stderr).toEqual(''); - done(err); - } - }); - }); diff --git a/test/lib/config-exclude-cli-args.js b/test/lib/config-exclude-cli-args.js deleted file mode 100644 index a9a8199c..00000000 --- a/test/lib/config-exclude-cli-args.js +++ /dev/null @@ -1,136 +0,0 @@ -'use strict'; - -var expect = require('expect'); -var excludeCliArgs = require('../../lib/shared/config/exclude-cli-args'); - -var originalArgv = process.argv; - -describe('lib: config/exclude-cli-args', function() { - - afterEach(function(done) { - process.argv = originalArgv; - done(); - }); - - describe('config.flags is not empty', function() { - - it('Should remain config flags which is not specified by cli args', - function(done) { - process.argv = originalArgv.concat(); - - var config = { flags: { silent: true } }; - var result = excludeCliArgs(config); - expect(result).toEqual({ flags: { silent: true } }); - - config = { - flags: { - help: false, - depth: 4, - tasks: false, - silent: true, - }, - }; - result = excludeCliArgs(config); - expect(result).toEqual({ - flags: { - help: false, - depth: 4, - tasks: false, - silent: true, - }, - }); - done(); - }); - - it('Should remove config flags which is specified by cli args', - function(done) { - process.argv = originalArgv.concat([ - '--silent', - ]); - var config = { flags: { silent: true } }; - - var result = excludeCliArgs(config); - expect(result).toEqual({ flags: {} }); - - config = { - flags: { - help: false, - depth: 4, - tasks: false, - silent: true, - }, - }; - result = excludeCliArgs(config); - expect(result).toEqual({ - flags: { - help: false, - depth: 4, - tasks: false, - }, - }); - done(); - }); - - it('Should remove config flags of which alias is specified by cli args', - function(done) { - process.argv = originalArgv.concat([ - '-S', - ]); - var config = { flags: { silent: true } }; - - var result = excludeCliArgs(config); - expect(result).toEqual({ flags: {} }); - - config = { - flags: { - help: false, - depth: 4, - tasks: false, - silent: true, - }, - }; - result = excludeCliArgs(config); - expect(result).toEqual({ - flags: { - help: false, - depth: 4, - tasks: false, - }, - }); - done(); - }); - - }); - - describe('config.flags is empty', function() { - - it('Should cause nothing when user specified no arg', function(done) { - process.argv = originalArgv.concat(); - - var config = {}; - var result = excludeCliArgs(config); - expect(result).toEqual({}); - - config = null; - var result = excludeCliArgs(config); - expect(result).toEqual({}); - done(); - }); - - it('Should cause nothing when user specified args', function(done) { - process.argv = originalArgv.concat([ - '--silent', - ]); - - var config = {}; - var result = excludeCliArgs(config); - expect(result).toEqual({}); - - config = null; - var result = excludeCliArgs(config); - expect(result).toEqual({}); - done(); - }); - }); - -}); From be189e35a9172b1c89818648b9adf817ca36af60 Mon Sep 17 00:00:00 2001 From: sttk Date: Sat, 22 Apr 2017 13:36:30 +0900 Subject: [PATCH 5/7] Remove validations of config files --- lib/shared/config/cli-flags.js | 1 - lib/shared/config/convert-config.js | 45 ----------- lib/shared/config/env-flags.js | 5 +- lib/shared/config/load-files.js | 12 +-- lib/versioned/^3.7.0/index.js | 3 +- lib/versioned/^4.0.0-alpha.1/index.js | 3 +- lib/versioned/^4.0.0-alpha.2/index.js | 5 +- lib/versioned/^4.0.0/index.js | 5 +- test/fixtures/config/err/case1/.gulp.js | 4 - test/fixtures/config/err/case2/.gulp.js | 4 - test/fixtures/config/err/case3/.gulp.js | 7 -- test/lib/config-cli-flags.js | 3 - test/lib/config-convert-config.js | 102 ------------------------ test/lib/config-env-flags.js | 3 - test/lib/config-load-files.js | 80 ++----------------- 15 files changed, 24 insertions(+), 258 deletions(-) delete mode 100644 lib/shared/config/convert-config.js delete mode 100644 test/fixtures/config/err/case1/.gulp.js delete mode 100644 test/fixtures/config/err/case2/.gulp.js delete mode 100644 test/fixtures/config/err/case3/.gulp.js delete mode 100644 test/lib/config-convert-config.js diff --git a/lib/shared/config/cli-flags.js b/lib/shared/config/cli-flags.js index 786df38e..4c837bb0 100644 --- a/lib/shared/config/cli-flags.js +++ b/lib/shared/config/cli-flags.js @@ -7,7 +7,6 @@ var fromto = { }; function mergeConfigToCliFlags(opt, config) { - opt = copyProps(opt, {}); return copyProps(config, opt, fromto); } diff --git a/lib/shared/config/convert-config.js b/lib/shared/config/convert-config.js deleted file mode 100644 index 7c859d79..00000000 --- a/lib/shared/config/convert-config.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict'; - -var path = require('path'); -var logger = require('fancy-log'); -var chalk = require('chalk'); - -module.exports = { - - description: function(value, configFile) { - if (typeof value !== 'string') { - logger.error(chalk.red( - configFile + ': config.description must be a string: ' + value)); - return undefined; - } - if (!value.length) { - logger.error(chalk.red( - configFile + ': config.description requires a value.')); - return undefined; - } - return value; - }, - - 'flags.silent': function(value, configFile) { - if (typeof value !== 'boolean') { - logger.error(chalk.red( - configFile + ': config.flags.silent must be a boolean: ' + value)); - return undefined; - } - return value; - }, - - 'flags.gulpfile': function(value, configFile) { - if (typeof value !== 'string') { - logger.error(chalk.red( - configFile + ': config.flags.gulpfile must be a string: ' + value)); - return undefined; - } - if (!value.length) { - logger.error(chalk.red( - configFile + ': config.flags.gulpfile requires a value.')); - return undefined; - } - return path.resolve(path.dirname(configFile), value); - }, -}; diff --git a/lib/shared/config/env-flags.js b/lib/shared/config/env-flags.js index 2e5a6f28..c404e5f2 100644 --- a/lib/shared/config/env-flags.js +++ b/lib/shared/config/env-flags.js @@ -3,14 +3,13 @@ var path = require('path'); var copyProps = require('copy-props'); -var fromto = { +var toFrom = { configPath: 'flags.gulpfile', configBase: 'flags.gulpfile', }; function mergeConfigToEnvFlags(env, config) { - env = copyProps(env, {}); - return copyProps(env, config, fromto, convert, true); + return copyProps(env, config, toFrom, convert, true); } function convert(value, configKey, envKey) { diff --git a/lib/shared/config/load-files.js b/lib/shared/config/load-files.js index 3b42d7fc..52374901 100644 --- a/lib/shared/config/load-files.js +++ b/lib/shared/config/load-files.js @@ -1,7 +1,7 @@ 'use strict'; var copyProps = require('copy-props'); -var convertConfig = require('./convert-config'); +var path = require('path'); function loadConfigFiles(configFiles, configFileOrder) { var config = {}; @@ -13,14 +13,10 @@ function loadConfigFiles(configFiles, configFileOrder) { } copyProps(require(filePath), config, function(value, name) { - if (value == null) { - return undefined; + if (name === 'flags.gulpfile') { + return path.resolve(path.dirname(filePath), value); } - var convert = convertConfig[name]; - if (typeof convert !== 'function') { - return value; - } - return convert(value, filePath); + return value; }); }); diff --git a/lib/versioned/^3.7.0/index.js b/lib/versioned/^3.7.0/index.js index 06ffedaf..d6ccc326 100644 --- a/lib/versioned/^3.7.0/index.js +++ b/lib/versioned/^3.7.0/index.js @@ -4,6 +4,7 @@ var chalk = require('chalk'); var log = require('gulplog'); var stdout = require('mute-stdout'); var tildify = require('tildify'); +var isString = require('lodash.isstring'); var taskTree = require('./taskTree'); var logTasks = require('../../shared/log/tasks'); @@ -38,7 +39,7 @@ function execute(opts, env, config) { } if (opts.tasks) { var tree = taskTree(gulpInst.tasks); - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); diff --git a/lib/versioned/^4.0.0-alpha.1/index.js b/lib/versioned/^4.0.0-alpha.1/index.js index 2de28569..9788e8be 100644 --- a/lib/versioned/^4.0.0-alpha.1/index.js +++ b/lib/versioned/^4.0.0-alpha.1/index.js @@ -6,6 +6,7 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); +var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -43,7 +44,7 @@ function execute(opts, env, config) { } if (opts.tasks) { var tree = {}; - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); diff --git a/lib/versioned/^4.0.0-alpha.2/index.js b/lib/versioned/^4.0.0-alpha.2/index.js index c6694ec4..783071d6 100644 --- a/lib/versioned/^4.0.0-alpha.2/index.js +++ b/lib/versioned/^4.0.0-alpha.2/index.js @@ -6,6 +6,7 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); +var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -48,7 +49,7 @@ function execute(opts, env, config) { } if (opts.tasks) { tree = gulpInst.tree({ deep: true }); - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); @@ -58,7 +59,7 @@ function execute(opts, env, config) { } if (opts.tasksJson) { tree = gulpInst.tree({ deep: true }); - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + tildify(env.configPath); diff --git a/lib/versioned/^4.0.0/index.js b/lib/versioned/^4.0.0/index.js index aa9a3660..042cb04e 100644 --- a/lib/versioned/^4.0.0/index.js +++ b/lib/versioned/^4.0.0/index.js @@ -6,6 +6,7 @@ var log = require('gulplog'); var chalk = require('chalk'); var stdout = require('mute-stdout'); var tildify = require('tildify'); +var isString = require('lodash.isstring'); var exit = require('../../shared/exit'); @@ -48,7 +49,7 @@ function execute(opts, env, config) { } if (opts.tasks) { tree = gulpInst.tree({ deep: true }); - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath)); @@ -58,7 +59,7 @@ function execute(opts, env, config) { } if (opts.tasksJson) { tree = gulpInst.tree({ deep: true }); - if (config.description) { + if (config.description && isString(config.description)) { tree.label = config.description; } else { tree.label = 'Tasks for ' + tildify(env.configPath); diff --git a/test/fixtures/config/err/case1/.gulp.js b/test/fixtures/config/err/case1/.gulp.js deleted file mode 100644 index 5e3b7545..00000000 --- a/test/fixtures/config/err/case1/.gulp.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = { - description: null, - 'non-exist-in-converter': null, -}; diff --git a/test/fixtures/config/err/case2/.gulp.js b/test/fixtures/config/err/case2/.gulp.js deleted file mode 100644 index 932f15cc..00000000 --- a/test/fixtures/config/err/case2/.gulp.js +++ /dev/null @@ -1,4 +0,0 @@ -module.exports = { - description: 'DESC.', - 'non-exist-in-converter': 'ABC', -}; diff --git a/test/fixtures/config/err/case3/.gulp.js b/test/fixtures/config/err/case3/.gulp.js deleted file mode 100644 index e0e3e51a..00000000 --- a/test/fixtures/config/err/case3/.gulp.js +++ /dev/null @@ -1,7 +0,0 @@ -module.exports = { - description: true, - flags: { - silent: 123, - gulpfile: '', - }, -}; diff --git a/test/lib/config-cli-flags.js b/test/lib/config-cli-flags.js index 4a5e8a3d..c541095d 100644 --- a/test/lib/config-cli-flags.js +++ b/test/lib/config-cli-flags.js @@ -17,7 +17,6 @@ describe('lib: config/cli-flags', function() { }; var result = mergeConfig(opts, config); - expect(result).toNotBe(opts); expect(result).toEqual({ silent: true, }); @@ -41,7 +40,6 @@ describe('lib: config/cli-flags', function() { }; var result = mergeConfig(opts, config); - expect(result).toNotBe(opts); expect(result).toEqual({ help: false, depth: 4, @@ -62,7 +60,6 @@ describe('lib: config/cli-flags', function() { var config = {}; var result = mergeConfig(opts, config); - expect(result).toNotBe(opts); expect(result).toEqual({ help: false, depth: 4, diff --git a/test/lib/config-convert-config.js b/test/lib/config-convert-config.js deleted file mode 100644 index d02f648e..00000000 --- a/test/lib/config-convert-config.js +++ /dev/null @@ -1,102 +0,0 @@ -'use strict'; - -var expect = require('expect'); -var path = require('path'); -var convertConfig = require('../../lib/shared/config/convert-config'); - -var chalk = require('chalk'); -var red = chalk.red; - -var logger = require('fancy-log'); -var origLogError = logger.error; -var logs; - -describe('lib: config/convert-config', function() { - - beforeEach(function() { - logs = []; - logger.error = function(log) { - logs.push(log); - }; - }); - - afterEach(function() { - logs.error = origLogError; - }); - - it('description', function(done) { - var convert = convertConfig.description; - expect(convert).toBeA('function'); - - expect(convert('abc', 'config/file/path')).toEqual('abc'); - expect(logs.length).toEqual(0); - - expect(convert(null, 'config/file/path')).toBeA('undefined'); - expect(logs.length).toEqual(1); - - expect(convert(0, 'config/file/path')).toBeA('undefined'); - expect(logs.length).toEqual(2); - - expect(convert('', 'config/file/path')).toBeA('undefined'); - expect(logs.length).toEqual(3); - - expect(logs).toEqual([ - red('config/file/path: config.description must be a string: null'), - red('config/file/path: config.description must be a string: 0'), - red('config/file/path: config.description requires a value.'), - ]); - - done(); - }); - - it('flags.silent', function(done) { - var convert = convertConfig['flags.silent']; - expect(convert).toBeA('function'); - - expect(convert(true, 'file/path')).toEqual(true); - expect(convert(false, 'file/path')).toEqual(false); - expect(logs.length).toEqual(0); - - expect(convert(null, 'file/path')).toBeA('undefined'); - expect(logs.length).toEqual(1); - - expect(convert(0, 'file/path')).toBeA('undefined'); - expect(logs.length).toEqual(2); - - expect(convert('', 'file/path')).toBeA('undefined'); - expect(logs.length).toEqual(3); - - expect(logs).toEqual([ - red('file/path: config.flags.silent must be a boolean: null'), - red('file/path: config.flags.silent must be a boolean: 0'), - red('file/path: config.flags.silent must be a boolean: '), - ]); - done(); - }); - - it('flags.gulpfile', function(done) { - var convert = convertConfig['flags.gulpfile']; - expect(convert).toBeA('function'); - - expect(convert('abc', 'path/to/config')).toEqual( - path.join(process.cwd(), 'path/to/abc')); - expect(logs.length).toEqual(0); - - expect(convert(null, 'path/to/config')).toBeA('undefined'); - expect(logs.length).toEqual(1); - - expect(convert(0, 'path/to/config')).toBeA('undefined'); - expect(logs.length).toEqual(2); - - expect(convert('', 'path/to/config')).toBeA('undefined'); - expect(logs.length).toEqual(3); - - expect(logs).toEqual([ - red('path/to/config: config.flags.gulpfile must be a string: null'), - red('path/to/config: config.flags.gulpfile must be a string: 0'), - red('path/to/config: config.flags.gulpfile requires a value.'), - ]); - done(); - }); - -}); diff --git a/test/lib/config-env-flags.js b/test/lib/config-env-flags.js index 349a34d2..31d67076 100644 --- a/test/lib/config-env-flags.js +++ b/test/lib/config-env-flags.js @@ -17,7 +17,6 @@ describe('lib: config/env-flags', function() { }; var result = mergeConfig(env, config); - expect(result).toNotBe(env); expect(result).toEqual({ configPath: '/path/to/gulpfile', configBase: '/path/to', @@ -46,7 +45,6 @@ describe('lib: config/env-flags', function() { }; var result = mergeConfig(env, config); - expect(result).toNotBe(env); expect(result).toEqual({ cwd: '/path/to/cwd', require: 'preload', @@ -75,7 +73,6 @@ describe('lib: config/env-flags', function() { var config = {}; var result = mergeConfig(env, config); - expect(result).toNotBe(env); expect(result).toEqual({ cwd: '/path/to/cwd', require: 'preload', diff --git a/test/lib/config-load-files.js b/test/lib/config-load-files.js index 2e7a916f..9d94b6fd 100644 --- a/test/lib/config-load-files.js +++ b/test/lib/config-load-files.js @@ -4,66 +4,47 @@ var expect = require('expect'); var path = require('path'); var loadConfigFiles = require('../../lib/shared/config/load-files'); -var chalk = require('chalk'); -var red = chalk.red; - var fixturesDir = path.join(__dirname, '../fixtures/config'); -var logger = require('fancy-log'); -var origLogError = logger.error; -var logs; - describe('lib: config/load-files', function() { - beforeEach(function() { - logs = []; - logger.error = function(log) { - logs.push(log); - }; - }); - - afterEach(function() { - logger.error = origLogError; - }); - it('Should load config from files', function(done) { - var configFilesBase = { + var configFiles = { a: path.join(fixturesDir, 'foo/bar/.gulp.json'), b: null, c: path.join(fixturesDir, 'qux/.gulp.js'), }; - var config = loadConfigFiles(configFilesBase, ['a', 'b', 'c']); + var config = loadConfigFiles(configFiles, ['a', 'b', 'c']); expect(config).toEqual({ description: 'description by .gulp.js in directory qux', }); - expect(logs.length).toEqual(0); done(); }); it('Should load config files in specified order', function(done) { - var configFilesBase = { + var configFiles = { a: path.join(fixturesDir, 'foo/bar/.gulp.json'), b: null, c: path.join(fixturesDir, 'qux/.gulp.js'), }; - var config = loadConfigFiles(configFilesBase, ['b', 'c', 'a']); + var config = loadConfigFiles(configFiles, ['b', 'c', 'a']); expect(config).toEqual({ description: 'Description by .gulp.json in directory foo/bar', }); - expect(logs.length).toEqual(0); done(); }); - it('Should load and convert a config prop if it is a path', function(done) { - var configFilesBase = { + it('Should convert a value of `flags.gulpfile` to absolute path', + function(done) { + var configFiles = { a: path.join(fixturesDir, 'flags/gulpfile/.gulp.json'), }; - var config = loadConfigFiles(configFilesBase, ['a']); + var config = loadConfigFiles(configFiles, ['a']); expect(config).toEqual({ flags: { @@ -71,51 +52,6 @@ describe('lib: config/load-files', function() { 'flags/gulpfile/is/here/mygulpfile.js'), }, }); - expect(logs.length).toEqual(0); - done(); - }); - - it('Should ignore if a config prop value is null or undefined', - function(done) { - var configFilesBase = { - a: path.join(fixturesDir, 'err/case1/.gulp.js'), - }; - - var config = loadConfigFiles(configFilesBase, ['a']); - - expect(config).toEqual({}); - expect(logs.length).toEqual(0); - done(); - }); - - it('Should let pass if a config prop is not defined in converter', - function(done) { - var configFilesBase = { - a: path.join(fixturesDir, 'err/case2/.gulp.js'), - }; - - var config = loadConfigFiles(configFilesBase, ['a']); - - expect(config).toEqual({ - description: 'DESC.', - 'non-exist-in-converter': 'ABC', - }); - expect(logs.length).toEqual(0); - done(); - }); - - it('Should output an error log and ignore if a config prop value is invalid', - function(done) { - var fp = path.join(fixturesDir, 'err/case3/.gulp.js'); - var configFilesBase = { a: fp }; - var config = loadConfigFiles(configFilesBase, ['a']); - - expect(config).toEqual({ flags: {} }); - expect(logs).toEqual([ - red(fp + ': config.description must be a string: true'), - red(fp + ': config.flags.silent must be a boolean: 123'), - red(fp + ': config.flags.gulpfile requires a value.'), - ]); done(); }); From 4758c8673ca6848e9d5e81d9331923480dfd512a Mon Sep 17 00:00:00 2001 From: sttk Date: Sat, 22 Apr 2017 14:13:25 +0900 Subject: [PATCH 6/7] Update toConsole to enable to call twice and remove calls of fancyLog in index.js --- index.js | 29 +++++++++++++++-------------- lib/shared/log/toConsole.js | 3 +++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index 94e47f12..6f2a154a 100644 --- a/index.js +++ b/index.js @@ -1,9 +1,10 @@ 'use strict'; +/* eslint max-statements: [1, 40] */ + var fs = require('fs'); var path = require('path'); var log = require('gulplog'); -var fancyLog = require('fancy-log'); var chalk = require('chalk'); var yargs = require('yargs'); var Liftoff = require('liftoff'); @@ -68,13 +69,15 @@ if (opts.continue) { process.env.UNDERTAKER_SETTLE = 'true'; } +// Set up event listeners for logging temporarily. +toConsole(log, opts); + cli.on('require', function(name) { - fancyLog('Requiring external module', chalk.magenta(name)); + log.info('Requiring external module', chalk.magenta(name)); }); cli.on('requireFail', function(name) { - fancyLog.error( - chalk.red('Failed to load external module'), chalk.magenta(name)); + log.error(chalk.red('Failed to load external module'), chalk.magenta(name)); }); cli.on('respawn', function(flags, child) { @@ -90,21 +93,19 @@ function run() { configPath: opts.gulpfile, require: opts.require, completion: opts.completion, - }, configureAndInvoke); + }, handleArguments); } module.exports = run; -function configureAndInvoke(env) { - var config = loadConfigFiles(env.configFiles['.gulp'], ['home', 'cwd']); - opts = mergeConfigToCliFlags(opts, config); - env = mergeConfigToEnvFlags(env, config); - handleArguments(env, opts, config); -} - // The actual logic -function handleArguments(env, opts, cfg) { - // Set up event listeners for logging. +function handleArguments(env) { + + var cfg = loadConfigFiles(env.configFiles['.gulp'], ['home', 'cwd']); + opts = mergeConfigToCliFlags(opts, cfg); + env = mergeConfigToEnvFlags(env, cfg); + + // Set up event listeners for logging again after configuring. toConsole(log, opts); if (opts.help) { diff --git a/lib/shared/log/toConsole.js b/lib/shared/log/toConsole.js index 39483bd6..1dd2ee15 100644 --- a/lib/shared/log/toConsole.js +++ b/lib/shared/log/toConsole.js @@ -14,6 +14,9 @@ var levels = [ ]; function toConsole(log, opts) { + // Remove previous listeners to enable to call this twice. + log.removeAllListeners(); + // Return immediately if logging is // not desired. if (opts.tasksSimple || opts.silent) { From 23677c962e42f2265cedd569e37238c59e5d636f Mon Sep 17 00:00:00 2001 From: sttk Date: Sat, 22 Apr 2017 16:55:45 +0900 Subject: [PATCH 7/7] Remove camelcase from dependencies --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 63ab5b5a..eeb8857f 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,6 @@ }, "dependencies": { "archy": "^1.0.0", - "camelcase": "^3.0.0", "chalk": "^1.1.0", "copy-props": "^1.4.1", "fancy-log": "^1.1.0",