diff --git a/README.md b/README.md index ccfce9e..10df35e 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ concise, simple test files with strong flexibility and extensibility. - Parallel execution of tests - Test against multiple acceptable answers - Wildcard matching (``, ``, `<*>`) -- Support for testing rich attachments (``, ``) +- Support for testing rich attachments (``, ``, ``) - Support for regular expressions (`<(st|nt|nd)>`) - Show diffs on error - Conversation branches @@ -71,6 +71,28 @@ Dialogue: - Bot: ``` +Handling multiple attachments + +```YAML +Title: Conversation with rich messages +Dialogue: + - Bot: Hi + - Human: Hello + - Human: How are we doing this month? + - Bot: "Month-to-date: " + - Human: Show me ad spend compared to last month + - Bot: + - Human: How is the campaign doing overall? + - Bot: According to and , it's going great! +``` + +Currently, the order and type of the attachments are checked, as is the text +that surrounds them, but not the position of the attachments within the text. + +> **NOTE**: In versions `0.0.11` and earlier, the text was not checked at all +> when an attachment was present. If you were sending text along with the +> attachment, you should add it to your dialogue when upgrading. + ## Special Tags You can use the following tags in your test dialogue files: @@ -80,7 +102,8 @@ Tag | Meaning `<*>` | Matches anything, including whitespaces `` | A single word without whitespaces `` | An image attachment -`` | A card attachment +`` | A card attachment (this includes buttons) +`` | Any other type of attachment (video, etc.) `<(REGEX)>` | Any regex expression, i.e. `<([0-9]{2})>` `<$VARNAME>` | An expression from the locale/translation file diff --git a/dialogues/examples/multiple-attachments.yml b/dialogues/examples/multiple-attachments.yml new file mode 100644 index 0000000..ac75dd7 --- /dev/null +++ b/dialogues/examples/multiple-attachments.yml @@ -0,0 +1,6 @@ +Title: Multiple attachments +Dialogue: + - Human: Hello + - Bot: Hi + - Human: How is my campaign doing? + - Bot: "Here are the stats for this week: " diff --git a/dialogues/examples/simple-attachment.yml b/dialogues/examples/simple-attachment.yml new file mode 100644 index 0000000..95965c5 --- /dev/null +++ b/dialogues/examples/simple-attachment.yml @@ -0,0 +1,6 @@ +Title: Simple attachment +Dialogue: + - Human: Hello + - Bot: Hi + - Human: How is my campaign doing? + - Bot: diff --git a/src/clients/botframework_client.ts b/src/clients/botframework_client.ts index 041d607..5692568 100644 --- a/src/clients/botframework_client.ts +++ b/src/clients/botframework_client.ts @@ -6,7 +6,7 @@ import { Activity, Message as BotMessage } from 'botframework-directlinejs'; import { Client } from './client_interface'; -import { Message, MessageType } from '../spec/message'; +import { Message, Attachment } from '../spec/message'; import * as program from 'commander'; import * as chalk from 'chalk'; @@ -78,16 +78,14 @@ export class BotFrameworkClient extends Client { } const message: Message = { user, - messageTypes: [ - dlMessage.text && MessageType.Text, - dlMessage.attachments && dlMessage.attachments.some(a => - a.contentType.includes('image')) && MessageType.Image, - dlMessage.attachments && dlMessage.attachments.some(a => - a.contentType.includes('hero')) && MessageType.Card, - ].filter(m => m), + attachments: dlMessage.attachments.map((a) => { + if (a.contentType.includes('image')) return Attachment.Image; + if (a.contentType.includes('hero')) return Attachment.Cards; + return Attachment.Other; + }), text: dlMessage.text || null, }; - if (message.messageTypes.length) { + if (message.text || message.attachments.length) { callback(message); return; } diff --git a/src/runner.ts b/src/runner.ts index 794d053..3b4ef54 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -1,7 +1,7 @@ 'use strict'; import { Client } from './clients/client_interface'; import { Dialogue } from './spec/dialogue'; -import { Message, MessageType } from './spec/message'; +import { Message, Attachment } from './spec/message'; import { Response } from './spec/response'; import { Turn, TurnType } from './spec/turn'; import { Config } from './config'; @@ -156,12 +156,12 @@ export class Runner { if (response !== null) { if (program.verbose) { const timing = (new Date().getTime() - this.timing) || 0; - if (response.messageTypes.includes(MessageType.Text)) { + if (response.text) { const texts = response.text.split('\n'); const truncatedText = texts.length > 5 ? texts.slice(0, 5).join('\n') + '...' : response.text; console.log(chalk.blue('\tBOT', response.user, ':', - truncatedText), + truncatedText, ...response.attachments.map(a => `<${a}>`)), chalk.magenta(` (${timing}ms)`)); } else { console.log(chalk.blue('\tBOT:', response.user, ':', JSON.stringify(response)), @@ -192,12 +192,15 @@ export class Runner { expected = `\t\t${matchArray[0]}`; } + let actual = response.text ? [response.text] : []; + actual = actual.concat(response.attachments.map(a => `<${a}>`)); + this.results.set(test.dialogue, { dialogue: test.dialogue, passed: false, errorMessage: chalk.red(`\t${Runner.getUsername(test)}\n`) + `\tExpected:\n${expected}` + - `\n\tGot:\n\t\t${response.text}`, + `\n\tGot:\n\t\t${actual.join(' ') || null}`, }); this.terminateInstance(test); return; @@ -261,7 +264,7 @@ export class Runner { } this.client.send({ user, - messageTypes: [MessageType.Text], + attachments: [], text: next.query, }); } diff --git a/src/spec/message.ts b/src/spec/message.ts index 909345b..2523356 100644 --- a/src/spec/message.ts +++ b/src/spec/message.ts @@ -1,11 +1,18 @@ -export enum MessageType { - Text = 'Text', - Image = 'Image', - Card = 'Card', +/* + * An enumeration of the possible attachment types. + * + * `Other` is a catch-all for any attachment type that is not (yet) explicitly + * supported. The enum values are used to build the regexes for the attachment + * wildcards, e.g. is derived from 'IMAGE'. + */ +export enum Attachment { + Image = 'IMAGE', + Cards = 'CARDS', + Other = 'OTHER', } export interface Message { - messageTypes: MessageType[]; + attachments: Attachment[]; text: string; user: string; } diff --git a/src/spec/response.ts b/src/spec/response.ts index 7863127..025ba76 100644 --- a/src/spec/response.ts +++ b/src/spec/response.ts @@ -1,50 +1,83 @@ -import { Message, MessageType } from './message'; +import { Message, Attachment } from './message'; import { Translator } from '../translator'; import * as program from 'commander'; const wildcardRegex: RegExp = /<\*>/g; const wordRegex: RegExp = //g; const numberRegex: RegExp = //g; -const imageRegex: RegExp = //g; -const cardsRegex: RegExp = //g; const regexRegex: RegExp = /<\((.+?)\)>/g; +// Use the Attachment enum values to create a regex for any attachment wildcard. +const attachmentAlternatives = Object.values(Attachment).join('|'); +const attachmentsRegex = new RegExp(`<(?:${attachmentAlternatives})>`, 'g'); + export class Response { - responseType: MessageType; + attachments: Attachment[]; private textMatchChecker: RegExp; + private bodyText: string; original: string; constructor(responseData: string) { this.original = responseData.trim(); - switch (this.original) { - case '': - this.responseType = MessageType.Image; - break; - case '': - this.responseType = MessageType.Card; - break; - default: - this.responseType = MessageType.Text; - this.textMatchChecker = new RegExp(Response.transformTags(this.original)); - } + + // Get a list of the Attachments in the response data. The map removes the + // angle brackets so that the resulting elements are valid enum values. + this.attachments = (this.original.match(attachmentsRegex) || []) + .map(match => match.slice(1, -1)); + + // Get what's left of the body text once the attachment wildcards have been + // removed. Each attachment consumes the surrounding whitespace, which in + // the end we replace with a single space. This is done so that you can + // write e.g. ' ' as a message spec and _not_ have the + // matcher try to match the message text with the space in between the + // wildcards in the spec. + // + // TODO: Use the array just after the map() to create an _array of match + // checkers_ so that we can validate the interleaving of attachments and + // message text, rather than ignoring where the attachments come within + // the text. + this.bodyText = this.original // 'Here you go: and ' + .split(attachmentsRegex) // ['Here you go: ', ' and ', ''] + .map(s => s.trim()) // ['Here you go:', 'and', ''] + .filter(s => s) // ['Here you go:', 'and'] + .join(' '); // 'Here you go: and' + + // Construct a regex to match the remaining body text. + this.textMatchChecker = new RegExp(Response.transformTags(this.bodyText)); } matches(message: Message): boolean { - if (!message.messageTypes.includes(this.responseType)) { + + // Check that the message body is present or absent, as expected. + if ((this.bodyText && !message.text) || (message.text && !this.bodyText)) { return false; - } else if (this.responseType === MessageType.Text) { - return message.text.trim().match(this.textMatchChecker) !== null; - } else { - return true; } + + // If there is a message body, check that its content is correct. + if (this.bodyText && message.text.trim().match(this.textMatchChecker) === null) { + return false; + } + + // Check that we have the right number of attachments. + if (this.attachments.length !== message.attachments.length) { + return false; + } + + // Check that we have the right types of attachments. + for (let i = 0; i < this.attachments.length; i += 1) { + if (this.attachments[i] !== message.attachments[i]) { + return false; + } + } + + // Everything appears to be in order. + return true; } static transformTags(text: string): string { regexRegex.lastIndex = 0; const taggedText = text - .replace(imageRegex, '') - .replace(cardsRegex, '') .replace(wildcardRegex, '<([\\s\\S]*?)>') .replace(wordRegex, '<([^ ]+?)>') .replace(numberRegex, '<([0-9\.,-]+)>'); diff --git a/src/test/runner.ts b/src/test/runner.ts index 48fb928..3b0d1d2 100644 --- a/src/test/runner.ts +++ b/src/test/runner.ts @@ -3,7 +3,7 @@ import { Runner } from '../runner'; import { MockClient } from '../clients/mock_client'; import { defaultConfig } from '../config'; import { Dialogue } from '../spec/dialogue'; -import { MessageType } from '../spec/message'; +import { Attachment } from '../spec/message'; import * as fs from 'fs'; import * as path from 'path'; @@ -27,19 +27,19 @@ describe('runner.ts', () => { }); expect(client.read(username)).to.equal('Hello'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Hi', user: username, }); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'How are you?', user: username, }); expect(client.read(username)).to.equal('I\'m good'); expect(client.read(username)).to.equal('Yourself?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'I\'m good too', user: username, }); @@ -66,19 +66,19 @@ describe('runner.ts', () => { }); expect(client.read(username)).to.equal('Hello'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: '123', user: username, }); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Hi', user: username, }); expect(client.read(username)).to.equal('I\'m good'); expect(client.read(username)).to.equal('Yourself?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'I\'m good too', user: username, }); @@ -97,11 +97,15 @@ describe('runner.ts', () => { client, getDialoguePath('examples/'), Object.assign({}, defaultConfig, { timeout: 5 })); - expect(runner.dialogues).to.have.length(2); + expect(runner.dialogues).to.have.length(4); expect(runner.dialogues).to.deep.include( new Dialogue(getDialoguePath('examples/simple.yml')), ).and.to.deep.include( new Dialogue(getDialoguePath('examples/branching.yml')), + ).and.to.deep.include( + new Dialogue(getDialoguePath('examples/simple-attachment.yml')), + ).and.to.deep.include( + new Dialogue(getDialoguePath('examples/multiple-attachments.yml')), ); }); @@ -118,7 +122,7 @@ describe('runner.ts', () => { }); }); - it('should fail a simple conversation if not matching', () => { + it('should fail a simple conversation if the text doesn\'t match', () => { const client = new MockClient(); const runner = new Runner(client, getDialoguePath('examples/simple.yml'), defaultConfig); setTimeout( @@ -130,7 +134,7 @@ describe('runner.ts', () => { terminated: false, }); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Bye', user: username, }); @@ -149,6 +153,82 @@ describe('runner.ts', () => { }); }); + it('should fail a simple conversation if the attachment doesn\'t match', () => { + const client = new MockClient(); + const runner = new Runner(client, getDialoguePath('examples/simple-attachment.yml'), + defaultConfig); + setTimeout( + () => { + const username = Runner.getUsername({ + branchNumber: 0, + dialogue: new Dialogue(getDialoguePath('examples/simple-attachment.yml')), + lastMessage: 0, + terminated: false, + }); + + expect(client.read(username)).to.equal('Hello'); + client.reply({ + attachments: [], + text: 'Hi', + user: username, + }); + + expect(client.read(username)).to.equal('How is my campaign doing?'); + client.reply({ + attachments: [Attachment.Cards], + text: null, + user: username, + }); + }, + 0); + + return runner.start().then((results) => { + const testResult = results[0]; + expect(testResult.passed).to.be.false; + expect(testResult.errorMessage) + .to.contain('Expected:\n\t\t') + .and.to.contain('Got:\n\t\t'); + }); + }); + + it('should fail a conversation if attachment order doesn\'t match', () => { + const client = new MockClient(); + const runner = new Runner(client, getDialoguePath('examples/multiple-attachments.yml'), + defaultConfig); + setTimeout( + () => { + const username = Runner.getUsername({ + branchNumber: 0, + dialogue: new Dialogue(getDialoguePath('examples/multiple-attachments.yml')), + lastMessage: 0, + terminated: false, + }); + + expect(client.read(username)).to.equal('Hello'); + client.reply({ + attachments: [], + text: 'Hi', + user: username, + }); + + expect(client.read(username)).to.equal('How is my campaign doing?'); + client.reply({ + attachments: [Attachment.Cards, Attachment.Image], + text: 'Here are the stats for this week:', + user: username, + }); + }, + 0); + + return runner.start().then((results) => { + const testResult = results[0]; + expect(testResult.passed).to.be.false; + expect(testResult.errorMessage) + .to.contain('Expected:\n\t\tHere are the stats for this week: ') + .and.to.contain('Got:\n\t\tHere are the stats for this week: '); + }); + }); + it('should handle branching conversations correctly', () => { const client = new MockClient(); const runner = new Runner(client, getDialoguePath('examples/branching.yml'), defaultConfig); @@ -174,14 +254,14 @@ describe('runner.ts', () => { expect(client.read(users[0])).to.equal('Can you assist me?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Ok', user: users[0], }); // This ends user0 client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Howdy', user: users[1], }); @@ -189,21 +269,21 @@ describe('runner.ts', () => { expect(client.read(users[1])).to.equal('Are you a cowboy?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Yes', user: users[1], }); expect(client.read(users[1])).to.equal('Really?'); expect(client.read(users[1])).to.equal('Can you assist me?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'No', user: users[1], }); // This ends user1 client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Howdy', user: users[2], }); @@ -211,7 +291,7 @@ describe('runner.ts', () => { expect(client.read(users[2])).to.equal('Are you a cowboy?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'No', user: users[2], }); @@ -251,13 +331,13 @@ describe('runner.ts', () => { expect(client.read(users[0])).to.equal('Can you assist me?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Random text', user: users[0], }); // user0 fails now client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Howdy', user: users[1], }); @@ -266,7 +346,7 @@ describe('runner.ts', () => { // user1 shortcircuits client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Howdy', user: users[2], }); @@ -309,26 +389,26 @@ describe('runner.ts', () => { expect(client.read(username)).to.equal('Hi there'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Hey', user: username, }); expect(client.read(username)).to.equal('Hello'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Hi', user: username, }); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'How are you?', user: username, }); expect(client.read(username)).to.equal('I\'m good'); expect(client.read(username)).to.equal('Yourself?'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'I\'m good too', user: username, }); @@ -361,14 +441,14 @@ describe('runner.ts', () => { expect(client.read(username)).to.equal('Show me some cats'); client.reply({ - messageTypes: [MessageType.Image], + attachments: [Attachment.Image], text: null, user: username, }); expect(client.read(username)).to.equal('What can you do?'); client.reply({ - messageTypes: [MessageType.Card], + attachments: [Attachment.Cards], text: null, user: username, }); @@ -401,7 +481,7 @@ describe('runner.ts', () => { expect(client.read(username)).to.equal('Hey there'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'How are you?', user: username, }); @@ -412,7 +492,7 @@ describe('runner.ts', () => { setTimeout(() => { expect(client.read(username)).to.equal('Hola'); client.reply({ - messageTypes: [MessageType.Text], + attachments: [], text: 'Como estas?', user: username, }); diff --git a/src/test/spec/response.ts b/src/test/spec/response.ts index d87df03..2f43037 100644 --- a/src/test/spec/response.ts +++ b/src/test/spec/response.ts @@ -1,11 +1,11 @@ import { expect } from 'chai'; import { Response } from '../../spec/response'; -import { Message, MessageType } from '../../spec/message'; +import { Message, Attachment } from '../../spec/message'; export function createTextMessage(text: string): Message { return { text, - messageTypes: [MessageType.Text], + attachments: [], user: null, }; } @@ -19,29 +19,111 @@ describe('response.ts', () => { expect(Response.transformTags('Hey <*>')).to.equal('^Hey ([\\s\\S]*?)$'); expect(Response.transformTags('Hey there')).to.equal('^Hey ([^ ]+?) there$'); }); - it('should instantiate correctly', () => { - expect(new Response(' ').responseType).to.equal(MessageType.Card); - }); - it('should instantiate correctly', () => { - expect(new Response(' ').responseType).to.equal(MessageType.Image); - }); - it('should match correctly', () => { - expect(new Response(' ').responseType).to.equal(MessageType.Card); - }); it('should match correctly', () => { expect(new Response(' ').matches({ user: null, text: null, - messageTypes: [MessageType.Image], + attachments: [Attachment.Image], })).to.be.true; }); it('should match correctly', () => { expect(new Response(' ').matches({ user: null, text: null, - messageTypes: [MessageType.Card], + attachments: [Attachment.Cards], + })).to.be.true; + }); + it('should not match text with if no text is expected', () => { + expect(new Response(' ').matches({ + user: null, + text: ' Here is your image: ', + attachments: [Attachment.Image], + })).to.be.false; + }); + it('should not match text with if no text is expected', () => { + expect(new Response(' ').matches({ + user: null, + text: ' Here are your options: ', + attachments: [Attachment.Cards], + })).to.be.false; + }); + it('should match correct text with an attachment if text is expected', () => { + expect(new Response('Here are your options: ').matches({ + user: null, + text: ' Here are your options: ', + attachments: [Attachment.Cards], })).to.be.true; }); + it('should not match incorrect text with an attachment if text is expected', () => { + expect(new Response('Here are your options: ').matches({ + user: null, + text: 'choose from ', + attachments: [Attachment.Cards], + })).to.be.false; + }); + it('should not match attachment without text if text is expected', () => { + expect(new Response('Here are your options: ').matches({ + user: null, + text: null, + attachments: [Attachment.Cards], + })).to.be.false; + }); + it('should not match incorrect type of attachment', () => { + expect(new Response('').matches({ + user: null, + text: null, + attachments: [Attachment.Other], + })).to.be.false; + }); + it('should not match a missing attachment', () => { + expect(new Response('Here you go: ').matches({ + user: null, + text: 'Here you go:', + attachments: [], + })).to.be.false; + }); + it('should match multiple attachments', () => { + expect(new Response('Here you go: ').matches({ + user: null, + text: 'Here you go:', + attachments: [Attachment.Image, Attachment.Image], + })).to.be.true; + }); + it('should not match the wrong number of attachments', () => { + expect(new Response('Here you go: ').matches({ + user: null, + text: 'Here you go:', + attachments: [Attachment.Image, Attachment.Image, Attachment.Image], + })).to.be.false; + }); + it('should match different types of attachments', () => { + expect(new Response(' Here you go: ').matches({ + user: null, + text: 'Here you go:', + attachments: [Attachment.Image, Attachment.Cards], + })).to.be.true; + }); + it('should not match the wrong order of attachments', () => { + expect(new Response(' ').matches({ + user: null, + text: null, + attachments: [Attachment.Cards, Attachment.Image], + })).to.be.false; + }); + it('should match multiple attachments within correct text', () => { + expect(new Response(' Compare with ').matches({ + user: null, + text: 'Compare with', + attachments: [Attachment.Image, Attachment.Other], + })).to.be.true; + }); + it('should not match multiple attachments within incorrect text', () => { + expect(new Response(' vs ').matches({ + user: null, + text: 'Compare with', + attachments: [Attachment.Image, Attachment.Other], + })).to.be.false; + }); it('should match simple phrases correctly', () => { expect(new Response('Hello there!').matches(createTextMessage(' Hello there! '))) .to.be.true;