-
Notifications
You must be signed in to change notification settings - Fork 371
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
Bug fixes for test results #2732
Conversation
narrieta
commented
Jan 19, 2023
- A test run wuth failing tests were previously reported as Success
- The distro name was missing in the test report
@@ -45,5 +45,5 @@ lisa \ | |||
--log_path "$lisa_logs" \ | |||
--working_path "$lisa_logs" \ | |||
-v subscription_id:"$SUBSCRIPTION_ID" \ | |||
-v identity_file:"$HOME/.ssh/id_rsa" \ | |||
|| true # force a success exit code to allow execution to continue when a test fails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now this is handled in execute_tests.sh
@@ -26,20 +26,23 @@ jobs: | |||
|
|||
- bash: $(Build.SourcesDirectory)/tests_e2e/pipeline/scripts/execute_tests.sh | |||
displayName: "Execute tests" | |||
continueOnError: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failures should not stop the pipeline, so we allow this task to continue on error
searchFolder: $(Build.ArtifactStagingDirectory) | ||
testRunTitle: 'Publish test results' | ||
failTaskOnFailedTests: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the Execute Tests tasks fails, the build is declared as a Partial Success. This forces the build to Fail on test failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how this flag(failTaskOnFailedTests) is related and only checks for execute-tests status?
will this not run if I have test failures? I think we need all the time to know summary of failures and successes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The task will mark itself as Failed, but it won't prevent it from publishing the results
from https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/publish-test-results-v2
failTaskOnFailedTests - Fail if there are test failures
boolean. Default value: false.
Optional. When this boolean's value is true, the task will fail if any of the tests in the results file are marked as failed. The default is false, which will simply publish the results from the results file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see run 5534 for an example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explaining, makes sense. I see few runs with warning sign. what is that mean? we have some warnings in test but no failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are partial success (reported as "[Build partially succeeded] ...- WALinuxAgent - 0eeb2c4")
The test execution failed (either because we could not execute the tests, or because some test failed), but I forced the Pipeline to continueOnError when the test execution task fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW - In those test runs there are no test results due to a bug in LISA. I reported it this morning and they will fix it.
distro = message.information.get('distro_version') | ||
if distro is not None: | ||
message.name = distro | ||
super()._received_message(message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We replace the test name (which is always "main" in our test runs) with the distro name
@@ -72,7 +72,7 @@ concurrency: 10 | |||
|
|||
notifier: | |||
- type: env_stats | |||
- type: junit | |||
- type: agent.junit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a custom JUnit notifier for the Agent
env: | ||
# Add all KeyVault secrets explicitly as they're not added by default to the environment vars | ||
AZURE_CLIENT_ID: $(AZURE-CLIENT-ID) | ||
AZURE_CLIENT_SECRET: $(AZURE-CLIENT-SECRET) | ||
AZURE_TENANT_ID: $(AZURE-TENANT-ID) | ||
SUBSCRIPTION_ID: $(SUBSCRIPTION-ID) | ||
|
||
- publish: $(Build.ArtifactStagingDirectory) | ||
artifact: 'artifacts' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report
@@ Coverage Diff @@
## develop #2732 +/- ##
========================================
Coverage 72.04% 72.04%
========================================
Files 104 104
Lines 15832 15832
Branches 2265 2265
========================================
Hits 11406 11406
Misses 3912 3912
Partials 514 514 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |