Parent test result support for data driven tests#417
Conversation
| var aggregateOutcome = results[0].Outcome; | ||
| foreach (var result in results) | ||
| { | ||
| UnitTestOutcomeExtensions.GetMoreImportantOutcome(aggregateOutcome, result.Outcome); |
There was a problem hiding this comment.
We don't need to assign result of GetMoreImportantOutcome() back to aggregateOutcome?
There was a problem hiding this comment.
Yes. UTs caught this and were failing. Corrected.
| private UTF.UnitTestOutcome GetAggregateOutcome(List<UTF.TestResult> results) | ||
| { | ||
| // In case results are not present, set outcome as unknown. | ||
| if (!results.Any()) |
There was a problem hiding this comment.
Suggestion: results.Count==0 seems more readable
There was a problem hiding this comment.
In case of list, using Count or Any doesn't matter. But in case of Enumerable any is preferred over count as count iterates through entire list. Thus as a general practice, for both list and enumerable I prefer any.
| this.ValidateFailedTests( | ||
|
|
||
| // Parent results should fail and thus failed count should be 7. | ||
| this.ValidateFailedTestsCount(7); |
There was a problem hiding this comment.
ValidateFailedTests method checks for number of tests. Number of params passed to the methods are considered as numbers of tests. In our case, we dont want to pass parent results as param but still want to consider it for results length check. Thus checking the count of tests separately.
| unitTestResult.DisplayName = testResults[i].DisplayName; | ||
| unitTestResult.DatarowIndex = testResults[i].DatarowIndex; | ||
| unitTestResult.ResultFiles = testResults[i].ResultFiles; | ||
| unitTestResult.ExecutionId = testResults[i].ExecutionId; |
There was a problem hiding this comment.
Add/modify some existing tests to cover this as well.
| EndTime = endTime | ||
| }; | ||
|
|
||
| testResult.SetPropertyValue<Guid>(Constants.ExecutionIdProperty, this.ExecutionId); |
There was a problem hiding this comment.
Similar for this. Add/modify existing tests to cover this
| @@ -429,9 +429,11 @@ public void RunTestMethodForMultipleResultsReturnMultipleResults() | |||
| var testMethodRunner = new TestMethodRunner(testMethodInfo, this.testMethod, this.testContextImplementation, false); | |||
There was a problem hiding this comment.
There are a bunch of data-driven tests in this class. Please make sure they are validating correct things even though they are not failing. Ideally they should have failed.
There was a problem hiding this comment.
Various data driven tests were failing. We have fixed them in the PR.
|
Add a RFC for this change and link RFC to this PR. |
| internal static UTF.UnitTestOutcome GetMoreImportantOutcome(this UTF.UnitTestOutcome outcome1, UTF.UnitTestOutcome outcome2) | ||
| { | ||
| return outcome1 < outcome2 ? outcome1 : outcome2; | ||
| var unitTestOutcome1 = outcome1.ToUnitTestOutcome(); |
|
@abhishkk : Please push this in soon. |
|
RFC is not required for this. So skipping. |
|
This change should have been an opt-in / configurable because it breaks existing flows. For example, Visual Studio Online doesn't support displaying child tests (yet). So atm, if a child test fails, you only see the parent failing in the UI and there is now way (as far as I'm aware of) to get hold of the test attachments of the failing child. |
Support is enabled in visual studio online (vsts). You can use vstest v2 task along with
The behavior of vstest v2 task was different for single agent and multi agent flows. For single agent, all child results were shown but for multi agent, only one result was shown. These changes not only enable hierarchical support but brings consistency in behavior of single and multi agent flows. Now in both cases, parent and child results are shown (where child results are in hierarchical form) |
|
We don't have VS version >= 15.8 preview 4 yet, so that might be our issue. My mistake then :). Thank you for the detailed update. |
3 participants
Mstest v2 adapter changes:
Copy from PR #342