Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filter for outgoing messages #15

Closed
bilby91 opened this issue Sep 4, 2017 · 9 comments
Closed

Add filter for outgoing messages #15

bilby91 opened this issue Sep 4, 2017 · 9 comments
Labels

Comments

@bilby91
Copy link
Contributor

bilby91 commented Sep 4, 2017

It would be a good idea to be able to configure a filter function for outgoing message. For example, when ending a conversation, the botbuilder sends a silent menssage with type: 'endOfConversation' that I don't want to match.

@bilby91
Copy link
Contributor Author

bilby91 commented Sep 4, 2017

Maybe we can only filter the endOfConversation message, not sure how many different type of messages can cause problems like this one...

@bilby91
Copy link
Contributor Author

bilby91 commented Sep 5, 2017

@microsoftly What are you thoughts here ?

@microsoftly
Copy link
Owner

not a bad idea, but I think that filtering out messages can't be done to all messages. A user may want to use any type of message for checking. For now, I would check against the event or the events you're expected to see.

Actually implementing this may require a bit more thought. The current model is rather simple; a builder model with one class. To give more flexibility, that type of filtering on a per message sent basis may be a bit harder to implement and require a more complex model

@bilby91
Copy link
Contributor Author

bilby91 commented Sep 6, 2017

@microsoftly Just to check if I understood correctly. What you are saying is that for now, users are required to check for the endOfCoversation activity always ? Is that even possible ? An IMessage type should always be message right ?

Can you explain a little further on the new model that you are suggesting ?

Thanks for the support as usual!

@microsoftly
Copy link
Owner

Yes, I would suggest checking that directly if you need to. I would first try to see if you don't need to set any expectations the test to work. You may need to add in a few millisecond delay with a .then between steps.

The new model is in reference to this, the main exported class from the module. checkSession, sendMessageToBot, then, and sendMessageToBotAndExpectSaveWithNoResponse all return the same botbuilder instance that accumulates all the test steps. The way I'm envisioning a filter would be something like

new BotTester(bot)
    .sendMessageToBot('respondWithEventFirst')
        .filter((message) => message.type === 'message')
        .expect('hey! this is the second message I sent!')
    .sendMessageToBot('another message')
    // ...
    .runTest();

Doing something like this would require different builders to be returned between each sendMessageToBot call, complicating the simple one class is the only return type builder model that exists today.

@bilby91
Copy link
Contributor Author

bilby91 commented Sep 6, 2017

@microsoftly At the moment if I don't add the following expectation to my test it fails.

return new BotTester(bot)
.sendMessageToBot('hi', 'Hello')
.sendMessageToBot('bye',
   'Talk later',
   { type: 'endOfConversation' } as IMessage,
   'Im going off' // This is send in a different conversation for example
  )

I agree 100% on keeping simple the BotTester API. The simple chaining works really good.

@microsoftly
Copy link
Owner

closing for now. If a good implementation comes by I'll happily look at it

@microsoftly
Copy link
Owner

reopening. New PR makes it looks like this isn't that difficult to implement. Will close when merged

@microsoftly microsoftly reopened this Oct 4, 2017
@microsoftly
Copy link
Owner

#33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants