Enable Adding Disabled Tests Skip Reason to their XML Results Log From issues.targets#93492
Enable Adding Disabled Tests Skip Reason to their XML Results Log From issues.targets#93492ivdiazsa merged 7 commits intodotnet:mainfrom ivdiazsa:the-tests-letters
Conversation
removed a development-only disabled test in issues.targets.
|
Tagging subscribers to this area: @hoyosjs Issue DetailsFeature that fixes issue #91562. This PR adds the necessary functionality to write the reason any given test was skipped in their results xml file. Currently, it is hard-coded to the message "No Known Skip Reason". With this PR, the reason will be taken from the test's disable explanation in
|
ivdiazsa
changed the title
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
trylek
left a comment
trylek
left a comment
There was a problem hiding this comment.
LGTM in general, I believe it's good to go it if works in the lab.
|
Ok there's a bug with certain cases where we have no specified reason for skipping a test. Will get to work on a fix for it. |
| hash = hash * 23 + (Method?.GetHashCode() ?? 0); | ||
| hash = hash * 23 + (ContainingType?.GetHashCode() ?? 0); | ||
| hash = hash * 23 + (ExecutionStatement?.GetHashCode() ?? 0); | ||
| hash = hash * 23 + (_executionStatement?.GetHashCode() ?? 0); |
There was a problem hiding this comment.
Can you use HashCode.Combine ?
There was a problem hiding this comment.
This code is in a source generator which targets .NET Standard 2.0.
There was a problem hiding this comment.
right, you'd have to have a package reference to the OOB. probably not worth it I guess.
There was a problem hiding this comment.
Referencing OOB dependencies for Roslyn components is very annoying to get right. I’d recommend against it unless necessary tbh.
| public static Dictionary<string, string> LoadTestExclusionTable() | ||
| { | ||
| HashSet<string> output = new (); | ||
| Dictionary<string, string> output = new Dictionary<string, string>(); |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Failures are unrelated so merging this feature now. |
ghost
locked as resolved and limited conversation to collaborators
4 participants
Feature that fixes issue #91562. This PR adds the necessary functionality to write the reason any given test was skipped in their results xml file.
Currently, it is hard-coded to the message "No Known Skip Reason". With this PR, the reason will be taken from the test's disable explanation in
src/tests/issues.targets, in their respective<Issue>tag. If there is no such message, then we will default to today's message of "No Known Skip Reason".A good amount of code cleanup was done as part of this PR as well. Due to how Github handles file differences and spacing, especially with long lines, I highly recommend using the Unified view instead of the Split one for reviewing this one PR.