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

Creation of Functional Test project #1632

Merged
merged 5 commits into from
Apr 12, 2019
Merged

Conversation

ceciliaavila
Copy link
Collaborator

@ceciliaavila ceciliaavila commented Apr 5, 2019

This PR fixes issue #1586

Proposed Changes

  • Created the new Microsoft.Bot.Builder.FunctionalTests project to gather all tests that hit services.

  • Moved the GetTokenRefreshTests and the LuisRecognizerTests into this new project.

  • Removed all references of FuseBox from the tests libraries.

image

@coveralls
Copy link
Collaborator

coveralls commented Apr 5, 2019

Pull Request Test Coverage Report for Build 54131

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 421 unchanged lines in 19 files lost coverage.
  • Overall coverage decreased (-7.2%) to 68.969%

Files with Coverage Reduction New Missed Lines %
/libraries/Microsoft.Bot.Builder/IntentScore.cs 2 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Age.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Temperature.cs 3 0.0%
/libraries/Microsoft.Bot.Builder/ITurnContextExtensions.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Dimension.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/LuisApplication.cs 3 88.64%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Money.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/LuisPredictionOptions.cs 4 0.0%
/libraries/Microsoft.Bot.Builder.Dialogs/Prompts/Prompt.cs 4 82.0%
/libraries/Microsoft.Bot.Builder/Adapters/TestAdapter.cs 6 79.14%
Totals Coverage Status
Change from base Build 53863: -7.2%
Covered Lines: 3985
Relevant Lines: 5778

💛 - Coveralls

@cleemullins
Copy link
Contributor

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

Copy link
Contributor

@cleemullins cleemullins left a comment

Choose a reason for hiding this comment

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

This looks great. The things we'll need to do:

  1. Make this NOT run as part of PR builds in Devops.
  2. Make these tests NOT run by default if someone locally runs "All Tests". That is, figure out how to exclude them.
  3. Make this project run only as part of the nightly build tests.
  4. Put a build badge on the repo, indicating the Pass/Fail state of the Functional Tests.

LMK when you're ready to merge this (Monday?). Once that's done, we can get started on the build & test updates.

@cleemullins
Copy link
Contributor

Note - many of the LUIS and Q&A tests are actually UNIT tests and shouldn't be moved.

Only the tests that actually hit services should be moved.

@ceciliaavila
Copy link
Collaborator Author

This looks great. The things we'll need to do:

  1. Make this NOT run as part of PR builds in Devops.
  2. Make these tests NOT run by default if someone locally runs "All Tests". That is, figure out how to exclude them.
  3. Make this project run only as part of the nightly build tests.
  4. Put a build badge on the repo, indicating the Pass/Fail state of the Functional Tests.

LMK when you're ready to merge this (Monday?). Once that's done, we can get started on the build & test updates.

Hi @cleemullins,
We've been analyzing several ways to exclude this new project from the local Build and "Run All tests" execution.
We could follow the approach from the PR #765 using a compilation symbol named FUNCTIONALTESTS to check if the test should execute:
image

This symbol will be explicitly set on the nightly build only. So, they won't execute on regular or local builds.

Another option could be using an environment variable to check at execution time and run the tests only if it's set to true.

A third option could be to create two PlayLists, one for the Functional-Tests and other for the Non-Functional-Tests and choose which one you want to execute in the Test Explorer.

Thanks!

@cleemullins
Copy link
Contributor

Of those 3 solutions, I lean towards the "#if" statement.

The playlist seems confusing. The Environment variable would result in the tests returning either "Inconclusive" or "Passed", both of which are misleading. I would rather just have the test ignored in the local scenario.

@ceciliaavila ceciliaavila force-pushed the fix/southworks/functional-test branch from af5fe5d to 6d80f76 Compare April 9, 2019 20:48
@JuanAr JuanAr force-pushed the fix/southworks/functional-test branch from 6d80f76 to 61c4ec6 Compare April 9, 2019 20:59
@ceciliaavila ceciliaavila marked this pull request as ready for review April 9, 2019 21:19
@cleemullins
Copy link
Contributor

@ceciliaavila Apparently changing from "Draft" to "Ready for review" doesn't kick off of build. Can you touch the PR so a build is kicked off?

@cleemullins
Copy link
Contributor

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

@ceciliaavila
Copy link
Collaborator Author

Pull Request Test Coverage Report for Build 53920

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 417 unchanged lines in 18 files lost coverage.
  • Overall coverage decreased (-7.2%) to 68.94%

Files with Coverage Reduction New Missed Lines %
/libraries/Microsoft.Bot.Builder/IntentScore.cs 2 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Age.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Temperature.cs 3 0.0%
/libraries/Microsoft.Bot.Builder/ITurnContextExtensions.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Dimension.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/LuisApplication.cs 3 88.64%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/Money.cs 3 0.0%
/libraries/Microsoft.Bot.Builder.AI.LUIS/LuisPredictionOptions.cs 4 0.0%
/libraries/Microsoft.Bot.Builder/Adapters/TestAdapter.cs 6 79.14%
/libraries/Microsoft.Bot.Builder.AI.LUIS/Generator/InstanceData.cs 6 0.0%
Totals Coverage Status
Change from base Build 53863: -7.2%
Covered Lines: 3982
Relevant Lines: 5776

💛 - Coveralls

The Coverage has decreased due to Functional Tests no longer been executed as part of the Build.

@cleemullins
Copy link
Contributor

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

@ceciliaavila
Copy link
Collaborator Author

Once this PR is merged, the "Nightly-Build" Pipeline must be configurated the following way:

  1. Define the FUNCTIONALTESTS as an MSBuild argument in the Build Task:
    image
  2. Define the environment variables for Luis Tests (1) and for Token Tests (2) as Pipeline Variables and mark them as Secret:
    image
    image
  3. As most of the variables are secrets, we need to add a PowerShell Script Task to take those secrets and pass them as environment variables for the tests to be able to read them:
    image
    The script is:
	echo '##vso[task.setvariable variable=TESTAPPID]$(TESTAPPIDSECRET)'
	echo '##vso[task.setvariable variable=TESTPASSWORD]$(TESTPASSWORDSECRET)'
	echo '##vso[task.setvariable variable=LUISAPPID]$(LUISAPPIDSECRET)'
	echo '##vso[task.setvariable variable=LUISSUBSCRIPTIONKEY]$(LUISSUBSCRIPTIONKEYSECRET)'

Then, the Pipeline will execute the Functional Tests using the environment variables:
image
NOTE: To add a Badge in the Readme file we've created the PR #1650

@cleemullins
Copy link
Contributor

One change before merge - Let's move the project from the /tests folder to a "/FunctionalTests" folder. These tests are different enough from our existing unit tests, that they should be in a different folder.

@ceciliaavila
Copy link
Collaborator Author

One change before merge - Let's move the project from the /tests folder to a "/FunctionalTests" folder. These tests are different enough from our existing unit tests, that they should be in a different folder.

Done! Thanks!

@cleemullins
Copy link
Contributor

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

@cleemullins
Copy link
Contributor

Let's target merging this, and the accompanying build changes, tomorrow. Should be straightforard.

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.

None yet

4 participants