From 3c65cda20ab752698ba71026dd34e7e30f7c99fe Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Mon, 22 Jan 2018 15:39:08 +0100 Subject: [PATCH 1/5] First stab at fixing a bug that wouldn't send bot messages to recipients' private messages. --- package.json | 5 +-- test/help_message_visibility_test.js | 51 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 test/help_message_visibility_test.js diff --git a/package.json b/package.json index b0941e5..b6dd7ae 100644 --- a/package.json +++ b/package.json @@ -33,13 +33,14 @@ "grunt-release": "^0.14.0", "hubot": "^3.0.0", "hubot-mock-adapter-v3": "^1.0.0", + "hubot-test-helper": "^1.8.1", "matchdep": "^0.1.2", "mocha": "^3.0.2", "nyc": "^11.0.3", + "semantic-release": "^6.3.6", "sinon": "^1.4.2", "sinon-chai": "^2.8.0", - "standard": "^10.0.2", - "semantic-release": "^6.3.6" + "standard": "^10.0.2" }, "main": "index.coffee", "scripts": { diff --git a/test/help_message_visibility_test.js b/test/help_message_visibility_test.js new file mode 100644 index 0000000..44c2981 --- /dev/null +++ b/test/help_message_visibility_test.js @@ -0,0 +1,51 @@ +'use strict' + +/* global describe, beforeEach, afterEach, it, context */ + +const chai = require('chai') +const expect = chai.expect + +chai.use(require('sinon-chai')) + +const Helper = require('hubot-test-helper') + +const helper = new Helper('../src/help.js') + +describe('help', () => describe('message visibility', () => { + beforeEach(function () { + this.room = helper.createRoom() + }) + + afterEach(function () { + this.room.destroy() + }) + + context('when HUBOT_HELP_REPLY_IN_PRIVATE is set', () => it('replies in a private message', function (done) { + process.env.HUBOT_HELP_REPLY_IN_PRIVATE = true + this.room.user.say('john', '@hubot help help').then(() => { + expect(this.room.messages).to.eql([ + ['john', '@hubot help help'], + ['hubot', '@john replied to you in private!'] + ]) + expect(this.room.privateMessages).to.eql({ + john: [ + ['hubot', 'hubot help - Displays all of the help commands that this bot knows about.\nhubot help - Displays all help commands that match .'] + ] + }) + }) + })) + + context('when HUBOT_HELP_REPLY_IN_PRIVATE is unset', () => it('replies in the same room', function (done) { + this.room.user.say('john', '@hubot help help').then(() => { + expect(this.room.messages).to.eql([ + ['john', '@hubot help help'], + ['hubot', '@john replied to you in private!'] + ]) + expect(this.room.privateMessages).to.eql({ + john: [ + ['hubot', 'hubot help - Displays all of the help commands that this bot knows about.\nhubot help - Displays all help commands that match .'] + ] + }) + }) + })) +})) From 72d6c288ad08fe762cdc44c134384d4c4d748466 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Mon, 22 Jan 2018 15:57:25 +0100 Subject: [PATCH 2/5] Fix the tests, and fix the problem. --- src/help.js | 6 ++---- test/help_message_visibility_test.js | 28 +++++++++++++++++----------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/help.js b/src/help.js index 33ee91c..cef482a 100644 --- a/src/help.js +++ b/src/help.js @@ -58,8 +58,6 @@ const helpContents = (name, commands) => `\ ` module.exports = (robot) => { - const replyInPrivate = process.env.HUBOT_HELP_REPLY_IN_PRIVATE - robot.respond(/help(?:\s+(.*))?$/i, (msg) => { let cmds = getHelpCommands(robot) const filter = msg.match[1] @@ -74,9 +72,9 @@ module.exports = (robot) => { const emit = cmds.join('\n') - if (replyInPrivate && msg.message && msg.message.user && msg.message.user.name && msg.message.user.name !== msg.message.room) { + if (process.env.HUBOT_HELP_REPLY_IN_PRIVATE && msg.message && msg.message.user && msg.message.user.name && msg.message.user.name !== msg.message.room) { msg.reply('replied to you in private!') - return robot.send({ room: msg.message.user.name }, emit) + return msg.sendPrivate(emit) } else { return msg.send(emit) } diff --git a/test/help_message_visibility_test.js b/test/help_message_visibility_test.js index 44c2981..8a3dc4b 100644 --- a/test/help_message_visibility_test.js +++ b/test/help_message_visibility_test.js @@ -20,22 +20,28 @@ describe('help', () => describe('message visibility', () => { this.room.destroy() }) - context('when HUBOT_HELP_REPLY_IN_PRIVATE is set', () => it('replies in a private message', function (done) { - process.env.HUBOT_HELP_REPLY_IN_PRIVATE = true + context('when HUBOT_HELP_REPLY_IN_PRIVATE is unset', () => it('replies in the same room', function (done) { this.room.user.say('john', '@hubot help help').then(() => { expect(this.room.messages).to.eql([ ['john', '@hubot help help'], - ['hubot', '@john replied to you in private!'] + ['hubot', 'hubot help - Displays all of the help commands that this bot knows about.\nhubot help - Displays all help commands that match .'] ]) - expect(this.room.privateMessages).to.eql({ - john: [ - ['hubot', 'hubot help - Displays all of the help commands that this bot knows about.\nhubot help - Displays all help commands that match .'] - ] - }) - }) + }).then(done, done) })) +})) - context('when HUBOT_HELP_REPLY_IN_PRIVATE is unset', () => it('replies in the same room', function (done) { +describe('help', () => describe('message visibility', () => { + beforeEach(function () { + process.env.HUBOT_HELP_REPLY_IN_PRIVATE = true + this.room = helper.createRoom() + }) + + afterEach(function () { + delete process.env.HUBOT_HELP_REPLY_IN_PRIVATE + this.room.destroy() + }) + + context('when HUBOT_HELP_REPLY_IN_PRIVATE is set', () => it('replies in a private message', function (done) { this.room.user.say('john', '@hubot help help').then(() => { expect(this.room.messages).to.eql([ ['john', '@hubot help help'], @@ -46,6 +52,6 @@ describe('help', () => describe('message visibility', () => { ['hubot', 'hubot help - Displays all of the help commands that this bot knows about.\nhubot help - Displays all help commands that match .'] ] }) - }) + }).then(done, done) })) })) From 3b5cf0964685aa739cd5c0b94c99616983d054a4 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Fri, 23 Feb 2018 12:14:28 +0100 Subject: [PATCH 3/5] Make Hubot's PM message a bit friendlier. --- src/help.js | 2 +- test/help_message_visibility_test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/help.js b/src/help.js index cef482a..323bad3 100644 --- a/src/help.js +++ b/src/help.js @@ -73,7 +73,7 @@ module.exports = (robot) => { const emit = cmds.join('\n') if (process.env.HUBOT_HELP_REPLY_IN_PRIVATE && msg.message && msg.message.user && msg.message.user.name && msg.message.user.name !== msg.message.room) { - msg.reply('replied to you in private!') + msg.reply('I just replied to you in private.') return msg.sendPrivate(emit) } else { return msg.send(emit) diff --git a/test/help_message_visibility_test.js b/test/help_message_visibility_test.js index 8a3dc4b..395203b 100644 --- a/test/help_message_visibility_test.js +++ b/test/help_message_visibility_test.js @@ -45,7 +45,7 @@ describe('help', () => describe('message visibility', () => { this.room.user.say('john', '@hubot help help').then(() => { expect(this.room.messages).to.eql([ ['john', '@hubot help help'], - ['hubot', '@john replied to you in private!'] + ['hubot', '@john I just replied to you in private.'] ]) expect(this.room.privateMessages).to.eql({ john: [ From a3b619716acabdf0c5e7875c46f72999f07a9c48 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Tue, 27 Feb 2018 12:40:20 +0100 Subject: [PATCH 4/5] Increate the test timeout, because CI builds seem to hit them. --- test/help_message_visibility_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/help_message_visibility_test.js b/test/help_message_visibility_test.js index 395203b..8d598eb 100644 --- a/test/help_message_visibility_test.js +++ b/test/help_message_visibility_test.js @@ -13,6 +13,7 @@ const helper = new Helper('../src/help.js') describe('help', () => describe('message visibility', () => { beforeEach(function () { + this.timeout(5000) this.room = helper.createRoom() }) From f00994a8c3dfade861a4005fd177568bd10e1921 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Tue, 27 Feb 2018 12:40:20 +0100 Subject: [PATCH 5/5] Increase the test timeout, because CI builds seem to hit them. --- test/help_message_visibility_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/help_message_visibility_test.js b/test/help_message_visibility_test.js index 395203b..8d598eb 100644 --- a/test/help_message_visibility_test.js +++ b/test/help_message_visibility_test.js @@ -13,6 +13,7 @@ const helper = new Helper('../src/help.js') describe('help', () => describe('message visibility', () => { beforeEach(function () { + this.timeout(5000) this.room = helper.createRoom() })