From 8baa3d6601306e33ef058765d83602bf470f73e0 Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Mon, 25 Nov 2013 12:01:48 -0700 Subject: [PATCH] more tests, added contributing.md and license.md --- CONTRIBUTING.md | 47 ++++++++++++++++++++++ LICENSE.md | 13 ++++++ package.json | 1 + scripts/run_tests.js | 78 +++++++++++++++++++++++++----------- src/lib/client.js | 7 ++++ src/lib/client_action.js | 6 ++- src/lib/transport.js | 41 +++++++++---------- src/lib/utils.js | 16 ++++++-- test/unit/test_transport.js | 79 +++++++++++++++++++++++++++++++++++++ test/unit/test_utils.js | 35 ++++++++++++++++ 10 files changed, 275 insertions(+), 48 deletions(-) create mode 100644 test/unit/test_transport.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e69de29bb..c06c8793f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -0,0 +1,47 @@ +If you have a bugfix or new feature that you would like to contribute to elasticsearch.js please feel free to submit a pull request, or open an issue for discussion. If your change is adding new functionality, you should open an issue ***before*** writing any code. + +The process for contributing to any of the Elasticsearch repositories is similar. + +1. Lint your codes + + While developing, be sure to run JSHint on the files you modify. This is simple with most IDE's and the project has .jshintrc files in the proper places, make sure jshint is using them. + + We use the `"white": true` jshint config option to enforce style, but the + +2. Write tests + + Please write test cases to exercise your changes. + +2. When you're ready to run the tests + + Call `npm test` which executes `scripts/run_tests.js`. + + ```sh + $ node scripts/run_tests.js --help + + Runner for the Elasticsearch.js Unit and Integration tests in both node and the browser. + Specify --no-{{flag}} to negate it. + + Options: + --server [true] + --browser [true] + --unit [true] + --integration [false] + --port [9200] + --host ["localhost"] hostname for elasticsearch instance used in integration tests + --check-upstream [false] check for remote updates to the yaml test suite + ``` + +3. Submit a pull request + + Push your local changes to your forked copy of the repository and submit a pull request. In the pull request, describe what your changes do and be sure to link to any conversations regarding this implementation, eg "Closes #123". + +4. Sign the CLA + + Please make sure you have signed the [Contributor License Agreement](http://www.elasticsearch.org/contributor-agreement/). We are not asking you to assign copyright to us, but to give us the right to distribute your code without restriction. We ask this of all contributors in order to assure our users of the origin and continuing existence of the code. You only need to sign the CLA once. + +5. Grab a beer + + There will probably be discussion about the pull request and we will work with you if any changes are needed. + +6. Know that we greatly apreciate your help!! :D diff --git a/LICENSE.md b/LICENSE.md index e69de29bb..b2ba7010b 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -0,0 +1,13 @@ +Copyright 2012-2013 Elasticsearch BV + +Licensed under the Apache License, Version 2.0 (the "License"); you +may not use this file except in compliance with the License. You may +obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +implied. See the License for the specific language governing +permissions and limitations under the License. diff --git a/package.json b/package.json index ea0f3a753..a4005067b 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ }, "scripts": { "test": "node scripts/run_tests.js", + "coverage": "mocha test/unit/test_*.js --require blanket -R html-cov > coverage.html && open -a \"Google Chrome\" ./coverage.html", "build": "grunt", "generate": "node scripts/generate/js_api && node scripts/generate/yaml_tests", "export_client": "node scripts/export_client.js", diff --git a/scripts/run_tests.js b/scripts/run_tests.js index 9bce02273..578252a7b 100644 --- a/scripts/run_tests.js +++ b/scripts/run_tests.js @@ -2,59 +2,91 @@ var async = require('async'); var cp = require('child_process'); var chalk = require('chalk'); var argv = require('optimist') + .usage([ + 'Runner for the Elasticsearch.js unit and integration tests in both node and the browser.', + 'Specify --no-{{flag}} to negate it.' + ].join('\n')) .default({ - 'check-upstream': false, - 'in-node': true, - 'in-browser': true, - 'not-in-node': false, - 'not-in-browser': false, - 'unit': true, - 'integration': true + server: true, + browser: true, + unit: true, + integration: false, + host: 'localhost', + port: 9200, + 'check-upstream': false + }) + .describe({ + host: 'hostname for elasticsearch instance used in integration tests', + 'check-upstream': 'check for remote updates to the yaml test suite' }) .alias({ u: 'unit', i: 'integration', - b: 'in-browser', - n: 'in-node', - }) - .parse(JSON.parse(process.env.npm_config_argv).original); + b: 'browser', + s: 'server', + }); -if (argv['not-in-browser']) { - argv.b = argv['in-browser'] = false; +if (process.argv.indexOf('help') + process.argv.indexOf('--help') + process.argv.indexOf('-h') !== -3) { + argv.showHelp(); + process.exit(1); } -if (argv['not-in-node']) { - argv.n = argv['in-node'] = false; + +if (process.env.npm_config_argv) { + // when called by NPM + argv = argv.parse(JSON.parse(process.env.npm_config_argv).original); +} else { + // when called by NPM - via `npm test` + argv = argv.argv; } var commands = []; +if (argv['just-browser']) { + argv.server = false; + argv.browser = true; +} + if (argv['check-upstream']) { commands.push(['node', 'scripts/generate/yaml_tests/index.js']); } if (argv.unit) { - if (argv['in-node']) { + if (argv.server) { commands.push(['mocha', 'test/unit/test_*.js', '--require=should']); } - if (argv['in-browser']) { + if (argv.browser) { commands.push(['testling', '.']); } } if (argv.integration) { - if (argv['in-node']) { - commands.push(['mocha', 'test/integration/yaml_suite/index.js', '-b', '--require=should']); + if (argv.server) { + commands.push([ + 'mocha', + 'test/integration/yaml_suite/index.js', + '-b', + '--require=should', + '--host=' + argv.host, + '--port=' + argv.port + ]); } - if (argv['in-browser']) { + if (argv.browser) { commands.push(['node', 'scripts/run_browser_integration_suite/index.js']); } } +var proc = null; +process.on('exit', function () { + if (proc && proc.kill) { + proc.kill(); + } +}); + if (commands.length) { async.forEachSeries(commands, function (args, done) { var command = args.shift(); - console.log(chalk.gray('\n\n' + '# ' + command + ' ' + args.join(' '))); - var proc = cp.spawn(command, args, { + console.log(chalk.gray.bold('\n\n' + '# ' + command + ' ' + args.join(' '))); + proc = cp.spawn(command, args, { stdio: 'inherit' }); @@ -69,9 +101,11 @@ if (commands.length) { }); }, function (err) { if (err) { + console.log(chalk.red('\n\n⚑⚑⚑︎ Error! ⚑⚑⚑')); console.error(err.message); process.exit(1); } else { + console.log(chalk.green('\n\n✔︎ looks good\n\n')); process.exit(0); } }); diff --git a/src/lib/client.js b/src/lib/client.js index 54b67192a..6d728750f 100755 --- a/src/lib/client.js +++ b/src/lib/client.js @@ -31,6 +31,13 @@ var ca = require('./client_action'); var Transport = require('./transport'); function Client(config) { + config = config || {}; + + // our client will log minimally by default + if (!config.hasOwnProperty('log')) { + config.log = 'warning'; + } + this.transport = new Transport(config); // instantiate the api's namespaces diff --git a/src/lib/client_action.js b/src/lib/client_action.js index 1db455ef5..caf179f5c 100644 --- a/src/lib/client_action.js +++ b/src/lib/client_action.js @@ -45,7 +45,11 @@ var castType = { return param.options[i]; } } - throw new TypeError('Invalid ' + name + ': expected one of ' + param.options.join(',')); + throw new TypeError('Invalid ' + name + ': expected ' + ( + param.options.length > 1 + ? 'one of ' + param.options.join(',') + : param.options[0] + )); }, duration: function (param, val, name) { if (_.isNumeric(val) || _.isInterval(val)) { diff --git a/src/lib/transport.js b/src/lib/transport.js index bc7cf2fa9..a72acd0f5 100644 --- a/src/lib/transport.js +++ b/src/lib/transport.js @@ -7,25 +7,12 @@ module.exports = Transport; var _ = require('./utils'); var errors = require('./errors'); var Host = require('./host'); -var Log = require('./log'); var when = require('when'); function Transport(config) { config = config || {}; - var LogClass; - // setup the log - switch (typeof config.log) { - case 'function': - LogClass = config.log; - break; - case 'undefined': - config.log = 'warning'; - /* fall through */ - default: - LogClass = Log; - } - + var LogClass = _.funcEnum(config, 'logClass', Transport.logs, 'main'); config.log = this.log = new LogClass(config); // overwrite the createDefer method if a new implementation is provided @@ -37,25 +24,31 @@ function Transport(config) { var ConnectionPool = _.funcEnum(config, 'connectionPool', Transport.connectionPools, 'main'); this.connectionPool = new ConnectionPool(config); + // setup the serializer + var Serializer = _.funcEnum(config, 'serializer', Transport.serializers, 'json'); + this.serializer = new Serializer(config); + if (config.hosts) { - var hosts = _.createArray(config.hosts, function (val) { + var hostsConfig = _.createArray(config.hosts, function (val) { if (_.isPlainObject(val) || _.isString(val)) { return val; } }); - if (!hosts) { + if (!hostsConfig) { throw new Error('Invalid hosts config. Expected a URL, an array of urls, a host config object, or an array of ' + 'host config objects.'); } - this.connectionPool.setHosts(_.map(hosts, function (conf) { + var hosts = _.map(hostsConfig, function (conf) { return new Host(conf); - })); - } + }); - // setup the serializer - var Serializer = _.funcEnum(config, 'serializer', Transport.serializers, 'json'); - this.serializer = new Serializer(config); + if (config.randomizeHosts) { + hosts = _.shuffle(hosts); + } + + this.connectionPool.setHosts(hosts); + } } Transport.connectionPools = { @@ -66,6 +59,10 @@ Transport.serializers = { json: require('./serializers/json') }; +Transport.logs = { + main: require('./log') +}; + /** * Perform a request with the client's transport * diff --git a/src/lib/utils.js b/src/lib/utils.js index 1fef1521d..1170ad50e 100644 --- a/src/lib/utils.js +++ b/src/lib/utils.js @@ -399,13 +399,23 @@ _.funcEnum = function (config, name, opts, def) { case 'function': return val; case 'string': - if (opts[val]) { + if (opts.hasOwnProperty(val)) { return opts[val]; } /* falls through */ default: - throw new TypeError('Invalid ' + name + ' "' + val + '", expected a function or one of ' + - _.keys(opts).join(', ')); + var err = 'Invalid ' + name + ' "' + val + '", expected a function'; + switch (_.size(opts)) { + case 0: + break; + case 1: + err += 'or ' + _.keys(opts)[0]; + break; + default: + err += 'or one of ' + _.keys(opts).join(', '); + break; + } + throw new TypeError(err); } }; diff --git a/test/unit/test_transport.js b/test/unit/test_transport.js new file mode 100644 index 000000000..f75fa318e --- /dev/null +++ b/test/unit/test_transport.js @@ -0,0 +1,79 @@ +var Transport = require('../../src/lib/transport'); + +describe('Transport Class', function () { + + describe('Constructor', function () { + + it('Accepts a log class and intanciates it at this.log', function () { + function CustomLogClass() {} + var trans = new Transport({ + logClass: CustomLogClass + }); + + trans.log.should.be.an.instanceOf(CustomLogClass); + }); + + it('Accepts the name of a log class that is defined on Transport.logs', function () { + Transport.logs.custom = function () { + // custom logger class! + }; + + var trans = new Transport({ + logClass: 'custom' + }); + + trans.log.should.be.an.instanceOf(Transport.logs.custom); + delete Transport.logs.custom; + }); + + it('Accepts a "createDefer" function, which can be used to tie into other promise libs.', function () { + function CustomPromise() { + this.then = function () {}; + } + + var trans = new Transport({ + createDefer: function () { + return new CustomPromise(); + } + }); + + trans.createDefer().should.be.an.instanceOf(CustomPromise); + }); + + it('Accepts a connection pool class and intanciates it at this.connectionPool', function () { + function CustomConnectionPool() {} + var trans = new Transport({ + connectionPool: CustomConnectionPool + }); + + trans.connectionPool.should.be.an.instanceOf(CustomConnectionPool); + }); + + it('Accepts the name of a connectionPool class that is defined on Transport.connectionPools', function () { + Transport.connectionPools.custom = function () {}; + + var trans = new Transport({ + connectionPool: 'custom' + }); + + trans.connectionPool.should.be.an.instanceOf(Transport.connectionPools.custom); + delete Transport.connectionPools.custom; + }); + + it('Throws an error when the logClass or connectionPool configs are set wrong', function () { + (function () { + var trans = new Transport({ + connectionPool: 'pasta' + }); + }).should.throw(/invalid connectionpool/i); + + (function () { + var trans = new Transport({ + logClass: 'pasta' + }); + }).should.throw(/invalid logclass/i); + }); + + }); + +}); diff --git a/test/unit/test_utils.js b/test/unit/test_utils.js index 04c43f2c9..ade066df7 100644 --- a/test/unit/test_utils.js +++ b/test/unit/test_utils.js @@ -291,4 +291,39 @@ describe('Utils', function () { }).should.be.exactly(false); }); }); + + describe('#funcEnum', function () { + /* + * _.funcEnum(object, key, opts, default); + */ + it('tests if the value at key in object is a function, returns it if so', function () { + var config = { + func: function () {} + }; + _.funcEnum(config, 'func', {}, 'toString') + .should.be.exactly(config.func); + }); + it('tests if the value at key in object is undefined, returns the option at key default if so', function () { + var config = { + func: undefined + }; + _.funcEnum(config, 'func', {}, 'toString') + .should.be.exactly(Object.prototype.toString); + }); + it('tests if the value at key in object is a string, returns the option at that key if so', function () { + var config = { + 'config key name': 'toString' + }; + _.funcEnum(config, 'config key name', { toString: 'pizza' }, 'toJSON') + .should.be.exactly('pizza'); + }); + it('throws an error if the selection if invalid', function () { + var config = { + 'config': 'val' + }; + (function () { + _.funcEnum(config, 'config', { main: 'default' }, 'main'); + }).should.throw(/invalid config/i); + }); + }); });