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

feat: Added support for CQA with unit tests. #4162

Merged
merged 23 commits into from
Apr 7, 2022
Merged

feat: Added support for CQA with unit tests. #4162

merged 23 commits into from
Apr 7, 2022

Conversation

Arsh-Kashyap
Copy link
Contributor

@Arsh-Kashyap Arsh-Kashyap commented Mar 16, 2022

#minor

Description

Added Language Service utils for querying Language Service via bot. DefaultAnswer can be configured from secrets file. Updated unit tests and added tests for CQA(Custom Question Answering).

How to test

  1. Use this PR for samples.
  2. Run yarn and yarn build in root folder of SDK.
  3. Run yarn build in botbuilder-ai and botbuilder-dialogs in SDK.
  4. Run yarn link in botbuilder-dialogs in sdk.
  5. Run yarn link botbuilder-dialogs in botbuilder-ai. Then run yarn link.
  6. Go to samples and run npm install , yarn link botbuilder-ai and yarn link botbuilder-dialogs in 48.customQABot-all-features.
  7. Run npm start to test.
  8. Same steps can be performed to test 12.customQABot, and previous samples 11.qnamaker and 49.qnamaker-all-features.

Specific Changes

  • Added Language Service utils for querying Language Service via bot.
  • Default Answer can now be configured from .env file in nodejs sample.
  • If Default Answer is not mentioned it will render the one filled in language studio.

    Testing

    Added Language Service tests and updated existing tests.

  • @Arsh-Kashyap Arsh-Kashyap marked this pull request as ready for review March 17, 2022 12:06
    @Arsh-Kashyap Arsh-Kashyap requested a review from a team as a code owner March 17, 2022 12:06
    @Arsh-Kashyap Arsh-Kashyap changed the title Added support for CQA with unit tests. feat: Added support for CQA with unit tests. Mar 17, 2022
    @joshgummersall joshgummersall removed their request for review March 22, 2022 17:20
    libraries/botbuilder-dialogs/package.json Outdated Show resolved Hide resolved
    libraries/botbuilder-testing/package.json Outdated Show resolved Hide resolved
    @mrivera-ms
    Copy link
    Contributor

    @Arsh-Kashyap, please add code coverage to qnaCardBuilder.ts, generateAnswerUtil.ts and languageServiceUtil.ts. Particularly qnaCardBuilder.ts which is dropping to 38.71%:

    Before:

    image

    After:

    image

    @coveralls
    Copy link

    coveralls commented Mar 31, 2022

    Pull Request Test Coverage Report for Build 2106661739

    • 197 of 215 (91.63%) changed or added relevant lines in 11 files are covered.
    • No unchanged relevant lines lost coverage.
    • Overall coverage increased (+0.01%) to 84.502%

    Changes Missing Coverage Covered Lines Changed/Added Lines %
    libraries/botbuilder-ai/src/qnaCardBuilder.ts 13 14 92.86%
    libraries/botbuilder-ai/src/qnamaker-utils/generateAnswerUtils.ts 10 11 90.91%
    libraries/botbuilder-ai/src/customQuestionAnswering.ts 56 59 94.92%
    libraries/botbuilder-ai/src/qnamaker-utils/languageServiceUtils.ts 51 55 92.73%
    libraries/botbuilder-ai/src/qnaMakerDialog.ts 52 61 85.25%
    Totals Coverage Status
    Change from base Build 2031580169: 0.01%
    Covered Lines: 19907
    Relevant Lines: 22312

    💛 - Coveralls

    @mrivera-ms
    Copy link
    Contributor

    mrivera-ms commented Mar 31, 2022

    Thanks for the latest changes, @Arsh-Kashyap. I have resolved the comments that have been fixed since the previous review. A few issues:

    • The sample 48.customQABot-all-features does not run as is. Perhaps the sample (which is in Draft), is not updated?
    • After fixing the errors in the sample, testing as indicated in the description throws errors because there are modules in botbuilder-ai that are not found. Please make sure to add the corresponding exports for your new types.
    • The code coverage of qnaCardBuilder that I mentioned in the previous iteration has gone down.

    cc @sahithimurty @gabog @tracyboehrer

    @tracyboehrer tracyboehrer merged commit da9999e into microsoft:main Apr 7, 2022
    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.

    4 participants