diff --git a/.gitignore b/.gitignore index a75685f66..74b5a7fe8 100755 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ examples/db_team_bot/ .env .idea .vscode +coverage diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4619447cc..269fef495 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,6 +24,8 @@ possible with your report. If you can, please include: * Create, or link to an existing issue identifying the need driving your PR request. The issue can contain more details of the need for the PR as well as host debate as to which course of action the PR will take that will most serve the common good. * Include screenshots and animated GIFs in your pull request whenever possible. * Follow the JavaScript coding style with details from `.jscsrc` and `.editorconfig` files and use necessary plugins for your text editor. +* Run `npm test` before submitting and fix any issues. +* Add tests to cover any new functionality. Add and/or update tests for any updates to the code. * Write documentation in [Markdown](https://daringfireball.net/projects/markdown). * Please follow, [JSDoc](http://usejsdoc.org/) for proper documentation. * Use short, present tense commit messages. See [Commit Message Styleguide](#git-commit-messages). diff --git a/__test__/lib/Botkit.test.js b/__test__/lib/Botkit.test.js new file mode 100644 index 000000000..cbf8e837c --- /dev/null +++ b/__test__/lib/Botkit.test.js @@ -0,0 +1,24 @@ +let botkit; + +jest.mock('../../lib/CoreBot', () => 'corebot'); +jest.mock('../../lib/SlackBot', () => 'slackbot'); +jest.mock('../../lib/Facebook', () => 'facebook'); +jest.mock('../../lib/TwilioIPMBot', () => 'twilio'); +jest.mock('../../lib/BotFramework', () => 'botframework'); +jest.mock('../../lib/CiscoSparkbot', () => 'spark'); +jest.mock('../../lib/ConsoleBot', () => 'console'); + +beforeEach(() => { + jest.clearAllMocks(); + botkit = require('../../lib/Botkit'); +}); + +test('exports bot interfaces', () => { + expect(botkit.core).toBe('corebot'); + expect(botkit.slackbot).toBe('slackbot'); + expect(botkit.facebookbot).toBe('facebook'); + expect(botkit.twilioipmbot).toBe('twilio'); + expect(botkit.botframeworkbot).toBe('botframework'); + expect(botkit.sparkbot).toBe('spark'); + expect(botkit.consolebot).toBe('console'); +}); diff --git a/__test__/lib/Slack_web_api.test.js b/__test__/lib/Slack_web_api.test.js new file mode 100644 index 000000000..523832250 --- /dev/null +++ b/__test__/lib/Slack_web_api.test.js @@ -0,0 +1,307 @@ +let slackWebApi; +let mockRequest; +let mockResponse; +let mockBot; + +mockRequest = {}; + +jest.mock('request', () => mockRequest); + +beforeEach(() => { + mockResponse = { + statusCode: 200, + body: '{"ok": true}' + }; + + mockBot = { + config: {}, + debug: jest.fn(), + log: jest.fn(), + userAgent: jest.fn().mockReturnValue('jesting') + }; + + mockBot.log.error = jest.fn(); + + mockRequest.post = jest.fn().mockImplementation((params, cb) => { + cb(null, mockResponse, mockResponse.body); + }); + + slackWebApi = require('../../lib/Slack_web_api'); +}); + +describe('config', () => { + test('default api_root', () => { + const instance = slackWebApi(mockBot, {}); + expect(instance.api_url).toBe('https://slack.com/api/'); + }); + + test('setting api_root', () => { + mockBot.config.api_root = 'http://www.somethingelse.com'; + const instance = slackWebApi(mockBot, {}); + expect(instance.api_url).toBe('http://www.somethingelse.com/api/'); + }); +}); + +describe('callApi', () => { + let instance; + + test('uses data.token by default and post', () => { + const data = { + token: 'abc123' + }; + const cb = jest.fn(); + + instance = slackWebApi(mockBot, {}); + instance.callAPI('some.method', data, cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + expect(firstArg.form.token).toBe('abc123'); + }); + + test('uses config.token if data.token is missing', () => { + const data = {}; + const cb = jest.fn(); + + instance = slackWebApi(mockBot, { token: 'abc123' }); + instance.callAPI('some.method', data, cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + expect(firstArg.form.token).toBe('abc123'); + }); + + // this case is specific to callAPI, shared cases will be tested below + test(`handles multipart data`, () => { + const cb = jest.fn(); + instance = slackWebApi(mockBot, {}); + instance.callAPI('some.method', 'data', cb, true); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + + expect(firstArg.formData).toBe('data'); + expect(firstArg.form).toBeUndefined(); + expect(cb).toHaveBeenCalledWith(null, { ok: true }); + }); +}); + +describe('callApiWithoutToken', () => { + let instance; + + test('uses data values by default', () => { + const data = { + client_id: 'id', + client_secret: 'secret', + redirect_uri: 'redirectUri' + }; + const cb = jest.fn(); + + instance = slackWebApi(mockBot, {}); + instance.callAPIWithoutToken('some.method', data, cb); + + expect(mockRequest.post.mock.calls.length).toBe(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + expect(firstArg.form.client_id).toBe('id'); + expect(firstArg.form.client_secret).toBe('secret'); + expect(firstArg.form.redirect_uri).toBe('redirectUri'); + }); + + test('uses config values if not set in data', () => { + const config = { + clientId: 'id', + clientSecret: 'secret', + redirectUri: 'redirectUri' + }; + const cb = jest.fn(); + + // this seems to be an API inconsistency: + // callAPIWithoutToken uses bot.config, but callAPI uses that passed config + mockBot.config = config; + + instance = slackWebApi(mockBot, {}); + instance.callAPIWithoutToken('some.method', {}, cb); + + expect(mockRequest.post.mock.calls.length).toBe(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + expect(firstArg.form.client_id).toBe('id'); + expect(firstArg.form.client_secret).toBe('secret'); + expect(firstArg.form.redirect_uri).toBe('redirectUri'); + }); +}); + +describe('postForm', () => { + + ['callAPI', 'callAPIWithoutToken'].forEach((methodName) => { + let method; + let cb; + + beforeEach(() => { + const instance = slackWebApi(mockBot, {}); + method = instance[methodName]; + cb = jest.fn(); + }); + + test(`${methodName}: handles success`, () => { + method('some.action', 'data', cb); + expect(mockRequest.post).toHaveBeenCalledTimes(1); + const firstArg = mockRequest.post.mock.calls[0][0]; + + // do some thorough assertions here for a baseline + expect(firstArg.url).toMatch(/some.action$/); + expect(firstArg.form).toBe('data'); + expect(firstArg.formData).toBeUndefined(); + expect(firstArg.headers).toEqual({ 'User-Agent': 'jesting' }); + expect(cb).toHaveBeenCalledWith(null, { ok: true }); + }); + + test(`${methodName}: defaults callback`, () => { + method('some.action', 'data'); + expect(mockRequest.post).toHaveBeenCalledTimes(1); + }); + + test(`${methodName}: handles request lib error`, () => { + const error = new Error('WHOOPS!'); + mockRequest.post.mockImplementation((params, callback) => { + callback(error, null, null); + }); + + method('some.action', 'data', cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenCalledWith(error); + }); + + test(`${methodName}: handles non 200 response code`, () => { + mockRequest.post.mockImplementation((params, callback) => { + callback(null, { statusCode: 400 }, null); + }); + + method('some.action', 'data', cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenCalledTimes(1); + const firstArg = cb.mock.calls[0][0]; + expect(firstArg.message).toBe('Invalid response'); + }); + + test(`${methodName}: handles error parsing body`, () => { + mockRequest.post.mockImplementation((params, callback) => { + callback(null, { statusCode: 200 }, '{'); + }); + + method('some.action', 'data', cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenCalledTimes(1); + const firstArg = cb.mock.calls[0][0]; + expect(firstArg).toBeInstanceOf(Error); + }); + + test(`${methodName}: handles ok.false response`, () => { + mockRequest.post.mockImplementation((params, callback) => { + callback(null, { statusCode: 200 }, '{ "ok": false, "error": "not ok"}'); + }); + + method('some.action', 'data', cb); + + expect(mockRequest.post).toHaveBeenCalledTimes(1); + expect(cb).toHaveBeenCalledWith('not ok', { ok: false, error: 'not ok'}); + }); + }); +}); + +describe('api methods', () => { + let instance; + let cb; + + beforeEach(() => { + instance = slackWebApi(mockBot, {}); + cb = jest.fn(); + jest.spyOn(instance, 'callAPI'); + instance.callAPI.mockImplementation(() => {}); + }); + + afterEach(() => { + if (jest.isMockFunction(JSON.stringify)) { + JSON.stringify.mockRestore(); + } + instance.callAPI.mockRestore(); + }); + + test('spot check api methods ', () => { + // testing for all methods seems wasteful, but let's confirm the methods got built correctly and test the following scenarios + + // two levels + expect(instance.auth).toBeDefined(); + expect(instance.auth.test).toBeDefined(); + + instance.auth.test('options', 'cb'); + const firstCallArgs = instance.callAPI.mock.calls[0]; + expect(firstCallArgs).toEqual(['auth.test', 'options', 'cb']); + + // three levels + expect(instance.users).toBeDefined(); + expect(instance.users.profile).toBeDefined(); + expect(instance.users.profile.get).toBeDefined(); + + instance.users.profile.get('options', 'cb'); + const secondCallArgs = instance.callAPI.mock.calls[1]; + expect(secondCallArgs).toEqual(['users.profile.get', 'options', 'cb']); + }); + + describe('special cases', () => { + + test('chat.postMessage stringifies attachments', () => { + instance.chat.postMessage({attachments: []}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {attachments: '[]'}, cb); + }); + + test('chat.postMessage handles attachments as Strings', () => { + jest.spyOn(JSON, 'stringify'); + instance.chat.postMessage({attachments: 'string'}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {attachments: 'string'}, cb); + expect(JSON.stringify).not.toHaveBeenCalled(); + }); + + test('chat.postMessage handles attachments stringification errors', () => { + const error = new Error('WHOOPSIE'); + jest.spyOn(JSON, 'stringify').mockImplementation(() => { throw error; }); + instance.chat.postMessage({attachments: []}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.postMessage', {}, cb); + expect(JSON.stringify).toHaveBeenCalled(); + }); + + test('chat.update stringifies attachments', () => { + instance.chat.update({attachments: []}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {attachments: '[]'}, cb); + }); + + test('chat.update handles attachments as Strings', () => { + jest.spyOn(JSON, 'stringify'); + instance.chat.update({attachments: 'string'}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {attachments: 'string'}, cb); + expect(JSON.stringify).not.toHaveBeenCalled(); + }); + + test('chat.postMessage handles attachments stringification errors', () => { + const error = new Error('WHOOPSIE'); + jest.spyOn(JSON, 'stringify').mockImplementation(() => { throw error; }); + instance.chat.update({attachments: []}, cb); + expect(instance.callAPI).toHaveBeenCalledWith('chat.update', {}, cb); + expect(JSON.stringify).toHaveBeenCalled(); + }); + + test('files.upload should not use multipart if file is false', () => { + const options = { file: false, token: 'abc123' }; + instance.files.upload(options, cb); + expect(instance.callAPI).toHaveBeenCalledWith('files.upload', options, cb, false); + }); + + test('files.upload should use multipart if file is true', () => { + const options = { file: true, token: 'abc123' }; + instance.files.upload(options, cb); + expect(instance.callAPI).toHaveBeenCalledWith('files.upload', options, cb, true); + }); + }); +}); diff --git a/lib/Slackbot_worker.js b/lib/Slackbot_worker.js index 86bd5d879..e3aeb4211 100755 --- a/lib/Slackbot_worker.js +++ b/lib/Slackbot_worker.js @@ -156,7 +156,7 @@ module.exports = function(botkit, config) { } bot.rtm = new Ws(res.url, null, { - agent: agent + agent: agent }); bot.msgcount = 1; diff --git a/package.json b/package.json index 5008ee65a..7df5c9897 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "ws": "^2.2.2" }, "devDependencies": { + "jest-cli": "^20.0.1", "jscs": "^3.0.7", "mocha": "^3.2.0", "should": "^11.2.1", @@ -34,8 +35,10 @@ "winston": "^2.3.1" }, "scripts": { - "pretest": "jscs ./lib/", - "test": "mocha tests/*.js" + "pretest": "jscs ./lib/ ./__test__ --fix", + "test": "jest --coverage", + "test-legacy": "mocha ./tests/*.js", + "test-watch": "jest --watch" }, "repository": { "type": "git", @@ -55,5 +58,8 @@ "microsoft bot framework" ], "author": "ben@howdy.ai", - "license": "MIT" + "license": "MIT", + "jest": { + "testEnvironment": "node" + } } diff --git a/readme.md b/readme.md index c7462a0e7..1973432ba 100644 --- a/readme.md +++ b/readme.md @@ -140,6 +140,27 @@ Use the `--production` flag to skip the installation of devDependencies from Bot npm install --production ``` +## Running Tests + +To run tests, use the npm `test` command. Note: you will need dev dependencies installed using `npm install`. + +```bash +npm test +``` + +To run tests in watch mode run: + +```bash +npm run test-watch +``` + +Tests are run with [Jest](https://facebook.github.io/jest/docs/getting-started.html). You can pass Jest command line options after a `--`. +For example to have Jest bail on the first error you can run + +```bash +npm test -- --bail +``` + ## Documentation * [Get Started](docs/readme.md)