Fixes #5126 TestResult.*Counts#5127
Conversation
|
Note that the order of the Fail/Warn/Pass properties differs among |
|
@maettu-this Thanks for this! Looks good. Just do the cleaning in this PR. Also, from my comments, would be nice if you also elaborated on this in the documentation, ref my links there. @nunit/framework-team Let me merge this PR, I am in the middle of a release. :-) |
Which way around does the team prefer the order?
|
I am not so concerned about the order, so any way is fine by me. This is just source code, so. Also be aware that some tools order fields and properties alphabetically. Use of such tools on this code would change "the order". If order matters somehow somewhere it would be in the generated XML, there it might help for those few that read it, to see them in a meaningful way. Your suggested order is fine by me there. |
1a6d2d5 to
d2042ac
Compare
d2042ac to
743fd99
Compare
|
Changes to XSD/XML now pass CI. |
… Skip/Ignore, Inconclusive
|
Looking at the different orders in detail, I came to the conclusion to make the least intrusive cleanup by following the order in the Cleanup passes CI as well => @OsirisTerje the PR is ready to be reviewed. |
OsirisTerje
left a comment
There was a problem hiding this comment.
Ensure that there are no breaking changes.
It is a bit hard to review now since the "order" have changed.
| /// Gets the number of test cases that got initiated | ||
| /// when running the test and all its children. | ||
| /// </summary> | ||
| int AssertCount |
There was a problem hiding this comment.
Seems you have renamed this ?
Existing methods in the interface have to be there, so that we avoid breaking changes.
It can just redirect to the new one, with a comment it is legacy.
There was a problem hiding this comment.
The last reorderig commit 4fc7e8a shows how PassCount and AssertCount got "moved" to the matching location.
| /// Gets the number of test cases that have completed | ||
| /// when running the test and all its children. | ||
| /// </summary> | ||
| int PassCount |
There was a problem hiding this comment.
Same as the comment above, it has to stay.
There was a problem hiding this comment.
The last reorderig commit 4fc7e8a shows how PassCount and AssertCount got "moved" to the matching location.
| /// Gets the number of test cases that got initiated | ||
| /// when running the test and all its children. | ||
| /// </summary> | ||
| public abstract int TotalCount { get; } |
There was a problem hiding this comment.
has this been removed? Note: No breaking changes, right?
There was a problem hiding this comment.
The last reorderig commit 4fc7e8a shows how the counts got "moved" to the matching location.
|
@OsirisTerje please see the individual https://github.com/nunit/nunit/pull/5127/commits:
|
*CountsrefinedInitiatedCountandCompletedCountaddedFeel free to merge/cherry-pick/reject as feasible.
Fixes #5126