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

Expand TestBot + Add tests #1022

Merged
merged 44 commits into from
Jul 8, 2019
Merged

Expand TestBot + Add tests #1022

merged 44 commits into from
Jul 8, 2019

Conversation

benbrown
Copy link
Contributor

Fixes #971
Fixes #972

Description

This PR expands TestBot to match the current CoreBot example from the Samples repo.

In addition:

Specific Changes

  • The MainDialog has been slightly refactored to accept a LuisHelper as a parameter (allowing it to be mocked for tests)
  • New Dialog unit tests have been added in the tests/ folder using the botbuilder-testing library

Testing

Check out the repo

Run lerna run test - all tests should run and test

Also go to libraries/testbot and run npm run test to see them run individually.

@benbrown benbrown requested a review from gabog June 25, 2019 19:53
@benbrown
Copy link
Contributor Author

Builds on #987

@benbrown
Copy link
Contributor Author

@ParadoxARG Can you help me update the deployment scripts, which are now out of sync with what is in samples?

@ParadoxARG
Copy link
Contributor

Hi Ben,

The tests are probably failing because this is reverting the changes of #954 where we created a new endpoint (/api/mybot) to match the DotNet implementation.

In DotNet the TestBot has an endpoint for an EchoBot and another endpoint for the Flight Assistant. The tests right now are configured to run against an EchoBot. We should replicate the same for JS and have one endpoint for the EchoBot(mybot) and another one for the CoreBot (dialogbot). Here’s the DotNet implementation.

We will be running some tests over this branch, and we will let you know when we have more information about it.

@benbrown
Copy link
Contributor Author

@ParadoxARG Oops, I didn't mean to regress that - I'll restore that feature and the simple echo that existed before.

But do you think this is what is causing the test fail? It seems to be something about botbuilder-testing missing on npm, but it should be getting it from the monorepo through lerna.

@ParadoxARG
Copy link
Contributor

The test against the deployed TestBot will fail for sure without this endpoint, but as you mentioned we may be having another issue along with this. We will look into this tomorrow and run some tests against our pipeline.

@benbrown
Copy link
Contributor Author

I just put that endpoint back in place :)

Copy link
Contributor

@gabog gabog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Ben, ping me tomorrow if you have any questions on the changes requested.

Also, you may want to add a FlightBooking.ts class generated using LUISGen, it may help with the mock for LUIS too.

The one I generated for C# is here: https://github.com/microsoft/BotBuilder-Samples/blob/8dcaafc51a73f6663c7f53d07f3c9eaec0b68d5d/samples/csharp_dotnetcore/13.core-bot/CognitiveModels/FlightBooking.cs

libraries/testbot/dialogs/bookingDialog.js Show resolved Hide resolved
libraries/testbot/dialogs/bookingDialog.js Outdated Show resolved Hide resolved
libraries/testbot/dialogs/bookingDialog.js Outdated Show resolved Hide resolved
libraries/testbot/bots/dialogAndWelcomeBot.js Show resolved Hide resolved
libraries/testbot/tests/testData/bookingDialogTestCases.js Outdated Show resolved Hide resolved
libraries/testbot/tests/testData/dateResolverTestCases.js Outdated Show resolved Hide resolved
libraries/testbot/tests/testData/mainDialogTestCases.js Outdated Show resolved Hide resolved
libraries/testbot/tests/mainDialog.tests.js Outdated Show resolved Hide resolved
libraries/testbot/tests/mainDialog.tests.js Outdated Show resolved Hide resolved
Jonathan Spruance (Insight Global) and others added 18 commits July 2, 2019 17:47
…n botbuilder.

Renamed some test cases to use the same capitalization as bot builder.
Added empty .env file.
Added general .gitignore.
Added TSConfig
… from master.

Removed unnecesary tests for creating DialogTestClient instance.
Renamed dialog variables as sut (systemt under test) to match the documentation.
Fixed some tests (need to fix others)
…ted cities and work the same as the dotnet one.

Added helper methods to extract LUIS values for composite entities and datetime to FlightBookingRecognizer.
Added luis result json captures to testdata so we can write tests for entity extraction based on serialized results.
Updated BookingDialog to use proper text, speak and inputhints.
Updated DateResolverDialog to use proper text, speak and inputhints.
Removed onBeginDialog from cancelAndHelpDialog to avoid handling interruptions on turn 0.
Removed myBot.
Updated deployment template to point to messages instead of mybot.
updated dateResolveDialog tests to use strictEquals.
…gnizer rather than creating one on each call.

Implemented 3 of for test scenarios for mainDialog (with mock dialogs and mock recognizer).
deleted unused mainDialogTestCases.
Updated other tests to use => instead of function().
Changed the oreder of constructor params for mainDialog.
Added placeholder for cancelAndHelpDialog.tests.
@gabog
Copy link
Contributor

gabog commented Jul 8, 2019

Hi @ParadoxARG, can you use the real TestBot instead of mybot for your tests? that would simplify things in the code and make it less confusing. TestBot does real work as opposed to mybot that is just an echo.

@gabog gabog merged commit 8b4d340 into master Jul 8, 2019
@stevengum stevengum deleted the benbrown/testbot branch October 24, 2019 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement JS unit tests for TestBot Refactor JS TestBot to make it easier to test
4 participants