From 34eaa1295f02ef1dd073186834516aa2e4bb8441 Mon Sep 17 00:00:00 2001 From: Alexej Yaroshevich Date: Sun, 30 Nov 2014 02:49:46 +0400 Subject: [PATCH 1/4] Update packages, fixup jsdocs, drop commented code --- .jscsrc | 36 ++++++++++--------- lib/pym.js | 9 ++--- package.json | 10 +++--- .../modules/architect-imports-abc/index.js | 6 ++++ .../architect-multiple-registers/index.js | 6 ++++ .../modules/architect-plugin/index.js | 6 ++++ test/fixtures/modules/modules-abc/index.js | 4 +++ test/fixtures/modules/some-config/index.js | 3 ++ test/fixtures/modules/some-config/index.ym.js | 5 +++ 9 files changed, 58 insertions(+), 27 deletions(-) diff --git a/.jscsrc b/.jscsrc index 64613b3..c4fb3c6 100644 --- a/.jscsrc +++ b/.jscsrc @@ -1,19 +1,23 @@ { - "preset": "yandex", + "preset": "yandex", - "additionalRules": [ - "node_modules/jscs-jsdoc/lib/rules/*.js" - ], - "jsDoc": { - "checkParamNames": true, - "checkRedundantParams": true, - "requireParamTypes": true, - "checkReturnTypes": true, - "requireReturnTypes": true, - "checkTypes": true, - "checkRedundantReturns": true, - "checkRedundantAccess": true, - "leadingUnderscoreAccess": "private", - "enforceExistence": true - } + "plugins": [ + "jscs-jsdoc" + ], + "jsDoc": { + "checkAnnotations": { + "preset": "closurecompiler", + "extra": {"api": false, "test": true} + }, + "checkParamNames": true, + "checkRedundantParams": true, + "requireParamTypes": true, + "checkReturnTypes": true, + "requireReturnTypes": true, + "checkTypes": true, + "checkRedundantReturns": true, + "checkRedundantAccess": true, + "leadingUnderscoreAccess": "private", + "enforceExistence": true + } } diff --git a/lib/pym.js b/lib/pym.js index 880dbd4..995d686 100644 --- a/lib/pym.js +++ b/lib/pym.js @@ -15,7 +15,7 @@ App.create = function create(opts) { /** * App constructor - * @constructor + * @class App * @param {Object} opts * @param {String} opts.path */ @@ -40,14 +40,14 @@ function App(opts) { * @api * @param {string[]} dependencies * @param {function(...[resolvedDependency])} successCallbackFunction - * @param {function(error: Error)} [errorCallbackFunction] + * @param {function(Error)} [errorCallbackFunction] */ this.require = ym.require; /** * Loads node package with module provisions * @api - * @param {String|String[]|Object|Function} package - package name, or list of, + * @param {(String|String[]|Object|Function)} package - package name, or list of, * or package object, or package function * @returns {PYM} this */ @@ -60,9 +60,6 @@ function App(opts) { return this; }; - // this.meta = function (package) { - // }; - // load required modules if passed if (opts.uses) { opts.uses.forEach(function (plugin) { diff --git a/package.json b/package.json index 5a78165..c6fb9cb 100644 --- a/package.json +++ b/package.json @@ -30,13 +30,13 @@ "ym": "0.1.x" }, "devDependencies": { - "jscs": "1.6.x", - "jscs-jsdoc": "0.0.12", - "jshint": "~2.5.5", - "mocha": "~1.14.0" + "jscs": "1.8.x", + "jscs-jsdoc": "0.2.x", + "jshint": "~2.5.10", + "mocha": "~2.0.1" }, "scripts": { - "lint": "./node_modules/.bin/jshint lib test && ./node_modules/.bin/jscs lib test", + "lint": "jshint lib test && jscs lib test", "test": "npm run lint && ./node_modules/.bin/mocha" }, "licenses": [ diff --git a/test/fixtures/modules/architect-imports-abc/index.js b/test/fixtures/modules/architect-imports-abc/index.js index 60864cd..ef84357 100644 --- a/test/fixtures/modules/architect-imports-abc/index.js +++ b/test/fixtures/modules/architect-imports-abc/index.js @@ -1,3 +1,9 @@ +/** + * Example of architect api plugin + * @param {Object} options + * @param {Object} imports + * @param {function(Error, Object)} register + */ module.exports = function (options, imports, register) { register(null, { 'imports-abc': { diff --git a/test/fixtures/modules/architect-multiple-registers/index.js b/test/fixtures/modules/architect-multiple-registers/index.js index fc9cb0c..41d1a56 100644 --- a/test/fixtures/modules/architect-multiple-registers/index.js +++ b/test/fixtures/modules/architect-multiple-registers/index.js @@ -1,5 +1,11 @@ var alreadyCalled = false; +/** + * Example of architect api plugin + * @param {Object} options + * @param {Object} imports + * @param {function(Error, Object)} register + */ module.exports = function (options, imports, register) { if (alreadyCalled) { throw new Error('Redundant setup call for architect plugin'); diff --git a/test/fixtures/modules/architect-plugin/index.js b/test/fixtures/modules/architect-plugin/index.js index 4539516..7085a5b 100644 --- a/test/fixtures/modules/architect-plugin/index.js +++ b/test/fixtures/modules/architect-plugin/index.js @@ -1,3 +1,9 @@ +/** + * Example of architect api plugin + * @param {Object} options + * @param {Object} imports + * @param {function(Error, Object)} register + */ module.exports = function (options, imports, register) { register(null, { service: { diff --git a/test/fixtures/modules/modules-abc/index.js b/test/fixtures/modules/modules-abc/index.js index bc84f3d..4af93b3 100644 --- a/test/fixtures/modules/modules-abc/index.js +++ b/test/fixtures/modules/modules-abc/index.js @@ -1,3 +1,7 @@ +/** + * Example of plugin + * @param {YM} ym + */ module.exports = function (ym) { ym.define('a', function (provide) { provide('a'); diff --git a/test/fixtures/modules/some-config/index.js b/test/fixtures/modules/some-config/index.js index 2b0dabb..1f72cae 100644 --- a/test/fixtures/modules/some-config/index.js +++ b/test/fixtures/modules/some-config/index.js @@ -1,3 +1,6 @@ +/** + * Stub + */ module.exports = function () { throw new Error('Shouldn\'t be executed'); }; diff --git a/test/fixtures/modules/some-config/index.ym.js b/test/fixtures/modules/some-config/index.ym.js index 28ab873..8a8080a 100644 --- a/test/fixtures/modules/some-config/index.ym.js +++ b/test/fixtures/modules/some-config/index.ym.js @@ -1,3 +1,8 @@ +/** + * Example of exporting module + * @param {YM} ym + * @param {Object} options + */ module.exports = function (ym, options) { ym.define('config', function (provide) { provide({ From eff866e76846152ac1ca9266c6fb4ac09db1c7b4 Mon Sep 17 00:00:00 2001 From: Alexej Yaroshevich Date: Sun, 30 Nov 2014 07:30:05 +0400 Subject: [PATCH 2/4] jscsrc: strictCasedNatives --- .jscsrc | 2 +- lib/pym.js | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.jscsrc b/.jscsrc index c4fb3c6..3bf3442 100644 --- a/.jscsrc +++ b/.jscsrc @@ -14,7 +14,7 @@ "requireParamTypes": true, "checkReturnTypes": true, "requireReturnTypes": true, - "checkTypes": true, + "checkTypes": "strictNativeCase", "checkRedundantReturns": true, "checkRedundantAccess": true, "leadingUnderscoreAccess": "private", diff --git a/lib/pym.js b/lib/pym.js index 995d686..4bae50f 100644 --- a/lib/pym.js +++ b/lib/pym.js @@ -17,7 +17,7 @@ App.create = function create(opts) { * App constructor * @class App * @param {Object} opts - * @param {String} opts.path + * @param {string} opts.path */ function App(opts) { opts = opts || {}; @@ -47,7 +47,7 @@ function App(opts) { /** * Loads node package with module provisions * @api - * @param {(String|String[]|Object|Function)} package - package name, or list of, + * @param {(string|string[]|Object|Function)} package - package name, or list of, * or package object, or package function * @returns {PYM} this */ @@ -137,7 +137,7 @@ function _wrapToArchitect(setup, metadata) { * @private * @param {Function} provide - `ym` `provide` function in define ctx * @param {Object} cached - cached result of the last `register` call - * @param {String} service - required `service` name (module name in `ym`) + * @param {string} service - required `service` name (module name in `ym`) */ function _provideService(provide, cached, service) { if (cached.err) { @@ -152,7 +152,7 @@ function _wrapToArchitect(setup, metadata) { /** * @private - * @param {String[]} keys + * @param {string[]} keys * @param {Array} values * @returns {Object} * @test [["a","b"],["A","B"]] >>> {"a":"A","b":"B"} @@ -173,13 +173,13 @@ var dirname = PATH.dirname; * Loads a module, getting metadata from either it's package.json * or export object. * @private - * @param {String} base - * @param {String} relativePath + * @param {string} base + * @param {string} relativePath * @returns {Object} - * - {String[]} provides - * - {String[]} consumes - * - {String} packagePath - * - {String} + * - {string[]} provides + * - {string[]} consumes + * - {string} packagePath + * - {string} */ function _resolvePackageSync(base, relativePath) { var packageJsonPath; @@ -246,8 +246,8 @@ var realpathSync = FS.realpathSync; * It's not the full node require system algorithm, but it's the 99% case * This throws, make sure to wrap in try..catch * @private - * @param {String} base - base path - * @param {String} relativePath + * @param {string} base - base path + * @param {string} relativePath */ function _resolvePackagePathSync(base, relativePath) { var originalBase = base; From 96d035b4069d40496099f5817509a3172e40e092 Mon Sep 17 00:00:00 2001 From: Alexej Yaroshevich Date: Sun, 30 Nov 2014 07:17:17 +0400 Subject: [PATCH 3/4] add coverage packages --- .gitignore | 3 ++- index.js | 1 + package.json | 13 ++++++++++--- 3 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 index.js diff --git a/.gitignore b/.gitignore index 0497574..9c0607a 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ test-make-temp/ lib-cov/ coverage.html .DS_Store -*.sublime* \ No newline at end of file +*.sublime* +/html-report diff --git a/index.js b/index.js new file mode 100644 index 0000000..0b1a0db --- /dev/null +++ b/index.js @@ -0,0 +1 @@ +module.exports = require(process.env.PYM_COVER === '1' ? './lib-cov/pym' : './lib/pym'); diff --git a/package.json b/package.json index c6fb9cb..39fc333 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "engines": { "node": ">= 0.10" }, - "main": "lib/pym.js", + "main": "index.js", "files": [ "lib", "LICENSE" @@ -30,14 +30,21 @@ "ym": "0.1.x" }, "devDependencies": { + "istanbul": "^0.3.2", "jscs": "1.8.x", "jscs-jsdoc": "0.2.x", "jshint": "~2.5.10", - "mocha": "~2.0.1" + "mocha": "~2.0.1", + "mocha-istanbul": "*" }, "scripts": { + "cov-clean": "rm -rf lib-cov; rm -rf html-report", + "cov-build": "istanbul instrument --output lib-cov --no-compact --variable global.__coverage__ lib", + "cov": "npm run cov-clean && npm run cov-build && PYM_COVER=1 mocha -R mocha-istanbul", "lint": "jshint lib test && jscs lib test", - "test": "npm run lint && ./node_modules/.bin/mocha" + "test": "npm run lint && mocha", + "travis": "npm run test && npm run coveralls", + "coveralls": "" }, "licenses": [ { From a101e1e33e33e4f042e16dc62793f4935f14c9ba Mon Sep 17 00:00:00 2001 From: Alexej Yaroshevich Date: Sun, 30 Nov 2014 07:20:30 +0400 Subject: [PATCH 4/4] code coverage and minor bug fixes --- .travis.yml | 4 +- lib/pym.js | 18 ++- package.json | 3 +- .../modules/architect-invalid-plugin/index.js | 20 +++ .../architect-invalid-plugin/package.json | 8 + test/mocha.opts | 2 +- test/simple-app.js | 146 ++++++++++++++++++ 7 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/modules/architect-invalid-plugin/index.js create mode 100644 test/fixtures/modules/architect-invalid-plugin/package.json diff --git a/.travis.yml b/.travis.yml index 2da9791..597578f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,4 @@ node_js: - "0.10" - "0.11" -matrix: - allow_failures: - - node_js: "0.11" +script: npm run travis diff --git a/lib/pym.js b/lib/pym.js index 4bae50f..148fa13 100644 --- a/lib/pym.js +++ b/lib/pym.js @@ -51,7 +51,7 @@ function App(opts) { * or package object, or package function * @returns {PYM} this */ - this.usePackage = function use(package, options) { + this.usePackage = function usePackage(package, options) { options = options || {}; var resolvedPackage = _resolvePackageSync(path, package); @@ -62,9 +62,17 @@ function App(opts) { // load required modules if passed if (opts.uses) { - opts.uses.forEach(function (plugin) { - this.use(plugin); - }, this); + if (Array.isArray(opts.uses)) { + opts.uses.forEach(function (plugin) { + this.usePackage(plugin); + }, this); + } else if (typeof opts.uses === 'object') { + Object.keys(opts.uses).forEach(function (key) { + this.usePackage(key, opts.uses[key]); + }, this); + } else { + throw new Error('Unsupported value in uses property'); + } } } @@ -142,7 +150,7 @@ function _wrapToArchitect(setup, metadata) { function _provideService(provide, cached, service) { if (cached.err) { provide(null, cached.err); - } else if (!cached.result[service]) { + } else if (!cached.result || !cached.result[service]) { provide(null, new Error('Invalid architect plugin found on service ' + service)); } else { provide(cached.result[service]); diff --git a/package.json b/package.json index 39fc333..b69c52d 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ }, "devDependencies": { "istanbul": "^0.3.2", + "istanbul-coveralls": "1.0.x", "jscs": "1.8.x", "jscs-jsdoc": "0.2.x", "jshint": "~2.5.10", @@ -44,7 +45,7 @@ "lint": "jshint lib test && jscs lib test", "test": "npm run lint && mocha", "travis": "npm run test && npm run coveralls", - "coveralls": "" + "coveralls": "istanbul cover ./node_modules/.bin/_mocha -- -R spec && istanbul-coveralls" }, "licenses": [ { diff --git a/test/fixtures/modules/architect-invalid-plugin/index.js b/test/fixtures/modules/architect-invalid-plugin/index.js new file mode 100644 index 0000000..b300969 --- /dev/null +++ b/test/fixtures/modules/architect-invalid-plugin/index.js @@ -0,0 +1,20 @@ +/** + * Example of invalid architect api plugin + * @param {Object} options + * @param {Object} imports + * @param {function(Error, Object)} register + */ +module.exports = function (options, imports, register) { + if (options.throwError) { + register(new Error('invalid plugin error')); + } else if (options.returnInvalidString) { + register(null, 'invalid'); + } else if (options.multipleTimes) { + register(null, { + config: '1' + }); + register(null, { + config: '2' + }); + } +}; diff --git a/test/fixtures/modules/architect-invalid-plugin/package.json b/test/fixtures/modules/architect-invalid-plugin/package.json new file mode 100644 index 0000000..ee41f60 --- /dev/null +++ b/test/fixtures/modules/architect-invalid-plugin/package.json @@ -0,0 +1,8 @@ +{ + "name": "architect-imports-abc", + "version": "0.0.1", + "main": "index.js", + "plugin": { + "provides": ["invalid"] + } +} diff --git a/test/mocha.opts b/test/mocha.opts index 6a6182b..a75848f 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1,3 +1,3 @@ --reporter spec ---timeout 10 +--timeout 20 --growl diff --git a/test/simple-app.js b/test/simple-app.js index 1da22ce..82283e5 100644 --- a/test/simple-app.js +++ b/test/simple-app.js @@ -1,8 +1,154 @@ var ASSERT = require('assert'); +var domain = require('domain'); var util = require('util'); var App = require('..'); +describe('Basic tests', function () { + + it('Mono app should load package relative to main file', function (done) { + var app = App.create(); + app.usePackage('../../../test/fixtures/modules/some-config'); + app.require(['config'], function (config) { + ASSERT.equal(config.title, 'I\'m a config module. Trust me!'); + ASSERT.equal(config.options, '{}'); + done(); + }, done); + }); + + it('App with uses should preload package', function (done) { + var app = App.create({ + path: './test/fixtures/simple-app', + uses: ['some-config'] + }); + app.require(['config'], function (config) { + ASSERT.equal(config.title, 'I\'m a config module. Trust me!'); + ASSERT.equal(config.options, '{}'); + done(); + }, done); + }); + + it('App with uses should preload package with config', function (done) { + var app = App.create({ + path: './test/fixtures/simple-app', + uses: {'some-config': {yolo: 'swag'}} + }); + app.require(['config'], function (config) { + ASSERT.equal(config.options, '{"yolo":"swag"}'); + done(); + }, done); + }); + + it('App with uses should throw if not array or object', function () { + ASSERT.throws(function () { + App.create({ + path: './test/fixtures/simple-app', + uses: 'foo-bar' + }); + }); + }); + + it('Provide could resolve in an error', function (done) { + var d = domain.create(); + d.on('error', function () { done(); }); + d.run(function () { + var app = App.create(); + app.define('module', function (provide) { + provide(null, new Error('Oops!')); + }); + app.require('module', function (module) { + console.log('Should not be executed but resolves in ' + module); + }); + }); + }); + + it('should throw if architect plugin is with wrong api', function () { + ASSERT.throws(function () { + var app = App.create(); + app.usePackage('./unknown/'); + }, + /Can\'t find "/); + }); + + it('should throw if module name or path is wrong', function () { + ASSERT.throws(function () { + var app = App.create(); + app.usePackage('./unknown/'); + }, + /Can\'t find "/); + }); + + it('should throw if module is with unknown format', function () { + ASSERT.throws(function () { + var app = App.create(); + app.usePackage('./'); + }, + /Unsupported package/); + }); + + it('should throw if architect register calls with an error', function (done) { + shouldThrow(function () { + var app = App.create(); + app.usePackage('../../../test/fixtures/modules/architect-invalid-plugin', {throwError: true}); + app.require('invalid', function () {}); + }, + /Invalid acrhitect plugin format/, done); + }); + + it('should throw if architect register calls with non-object value', function (done) { + shouldThrow(function () { + var app = App.create(); + app.usePackage('../../../test/fixtures/modules/architect-invalid-plugin', {returnInvalidString: true}); + app.require('invalid', function () {}); + }, + /Invalid acrhitect plugin format/, done); + }); + + it('should throw if architect register called multiple times', function (done) { + shouldThrow(function () { + var app = App.create(); + app.usePackage('../../../test/fixtures/modules/architect-invalid-plugin', {multipleTimes: true}); + app.require('invalid', function () {}); + }, + /Service overloading not supported/, done); + }); + + it('should throw if architect register does not call', function (done) { + shouldThrow(function () { + var app = App.create(); + app.usePackage('../../../test/fixtures/modules/architect-invalid-plugin', {empty: true}); + app.require('invalid', function () {}); + }, + /Invalid architect plugin found on service/, done); + }); + + /** + * Helper for wrapping throws into domain events + * @param {Function} code + * @param {RegExp} [errRe] + * @param {Function} done + */ + function shouldThrow(code, errRe, done) { + if (arguments.length === 2) { + done = errRe; + errRe = null; + } + + ASSERT(typeof code === 'function', 'Expect code callback as first argument'); + ASSERT(typeof done === 'function', 'Expect done callback as last argument'); + + var d = domain.create(); + d.on('error', function (err) { + if (!errRe || errRe.test(err)) { + done(); + } else { + done(err); + } + }); + d.run(code); + } +}); + describe('Simple inherited app', function () { var app;