-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Plumb test failures through to github #15831
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zadjii-msft
changed the title
Attempt to plumb test failures through to github
Plumb test failures through to github
Aug 14, 2023
zadjii-msft
added
Area-Build
Issues pertaining to the build system, CI, infrastructure, meta
Product-Meta
The product is the management of the products.
Issue-Task
It's a feature request, but it doesn't really need a major design.
labels
Aug 14, 2023
DHowett
approved these changes
Aug 14, 2023
lhecker
reviewed
Aug 15, 2023
Comment on lines
22
to
+23
public bool Passed { get; set; } | ||
public bool Skipped { get; set; } |
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.
nit: This could be an enum to make the if()
/ if(!)
code a bit simpler.
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
lhecker
approved these changes
Aug 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area-Build
Issues pertaining to the build system, CI, infrastructure, meta
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Meta
The product is the management of the products.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does two bits:
Probably regressed around #6992 and #4490.
details
Part the first
We were relying on the MUX build scripts to convert our WTT test logs to xUnit format, which AzDo then ingests. That script we used relied on some WinUI-specific logic around retrying tests. They have some logic to auto-retry failed tests. They then mark a test as "skipped" if it passed less than some threshold of times. Since we were never setting that variable, we would mark a test as "skipped" if it had 0 passes. So, all failures showed up on AzDo as "skipped".
Why didn't we notice this? Well, the
Run-Tests.ps1
script will still return1
if any tests failed. So the test job would fail if there was a failure, AzDo just wouldn't know which test it was.part the second
Updates
ConvertWttLogToXUnitLog
inHelixTestHelpers.cs
to understand that a test can be skipped, in addition to pass/fail. Removes all the logic for dealing with retries, cause we didn't need that.part the third
TAEF doesn't emit error messages in a way that AzDo can immediately pick up on which tests failed. This means that Github gives us this useless error message:
That's the only "error" that AzDo knows about.
This PR changes that by adding a build step to manually parse the xUnit results, and log the names of any tests that failed. By logging them with a prefix of
##vso[task.logissue type=error]
, then AzDo will surface that text as an error message. GitHub can then grab that text and surface it too.Addenda: Why aren't we using the VsTest module
as noted in #4490 (comment), the vstest module is literally 6x slower than just running TAEF directly.
closes #7286