search

dice-group / sask

Projectgroups Search and Extraction
GNU Affero General Public License v3.0
2 stars 10 forks source link

Chatbot junit test #47

Closed Juzerhk closed 6 years ago

Juzerhk commented 6 years ago

Chabot is now able to handle "bye" and other single word requests.

chatbot_bye
Juzerhk commented 6 years ago

All checks passed successfully. Code can be merged now

guruprasads7 commented 6 years ago

@Juzerhk , Thanks for making the changes and sending it for review. Please note from the next time. Try to create a branch for the fix, rather than making the changes on an older branch. In this way, all of us can infer, why the branch was created in the first place. Also saves you from maintaining older branches.

Can you please, walk us through why we were receiving an exception, even through the rivescripts had responses for them (bye in this case).

Also, how was this problem handled in the current fix. And how does your fix handles these scenarios.

P.S : Sorry for making multiple edits to the same message :)

@guru5590 , Thanks for bringing the logs issue into notice. Single word reply from rivescripts where converted in JSON because of which we had this issue. This issue is fixed now and No longer JSON is created for one word replies from rivescript. These are the logs for the reference. So I think one word request are handled in an efficient way but if you find any issues please let me know. Thanks.

chatbot_bye_logs
guruprasads7 commented 6 years ago

@Juzerhk I just put some debug messages to view the code logs, it looks like a string message is treated as a valid Json. Can you please check why is this issue occuring. Attaching the screenshot for your reference.

screen shot 2018-05-08 at 21 33 38
Juzerhk commented 6 years ago

I made a new branch for the fix as my previous branch was deleted
after the merge. So this is a new branch in which I am making the fix, although the names are bit similar. Quoting GuruPrasad notifications@github.com:

@Juzerhk , Thanks for making the changes and sending it for review.
Please note from the next time. Try to create a branch for the fix,
rather than making the changes on an older branch. In this way, all
of us can infer, why the branch was created in the first place. Also
saves you from maintaining older branches

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/dice-group/sask/pull/47#issuecomment-387512406

guruprasads7 commented 6 years ago

Merging the pull request as this fixes this issue, @Juzerhk, But the problem with your fix is maskes the real issue with the library used.