-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
WIP: initialize data-provided tests late #3210
Conversation
removing trailing space from testcount output at line end
correctly handling numeric indices
fixing spaces in xml log tests
@@ -18,7 +18,7 @@ PHPUnit %s by Sebastian Bergmann and contributors. | |||
Runtime: %s | |||
Random seed: %d | |||
|
|||
..... 5 / 5 (100%) | |||
..... |
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.
👎 for dropping info how far we are with execution
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.
a percentage of an unknown number of testcases it impossible to calculate, so he options were having over 100% tests executed or just showing the number of tests executed.
The only reason there is no number in this specific line is, that it's the last one and directly beneath it the result - including the number of tests - is shown (multiple lines, see: https://github.com/sebastianbergmann/phpunit/pull/3210/files/47fdd0b8a6309e1534190ef8b76a97fff0d919ac#diff-cc5dd71ca2e247cdaa4cde8688137e59R18 ).
I picked the later one for now since I didn't see a way of doing the counting properly, see #10 (comment)
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.
When additional tests are discovered the total number of tests can be increased making the percentage work again.
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.
will also lead to very weird cases, let's take the following:
Suite1: 100 Tests, 20 DataProviderTestsuites -> initial estimation 120 tests
Assuming that doesn't fit into a line we could end up with
... 80/120 (66%)
... 160/160 (100%)
... 180/180 (100%)
I think this is even more confusing.
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.
Just wondering, would keeping the count itself work for you? That would bypass the issue of having the target number change but at least provide some information on where we are at? @keradus ?
This branch has conflicts that must be resolved. If the problem you wanted to address with this pull request still exists then please send a new pull request. |
Currently very unclean and broken, meant to be a part of #10, so maybe interesting for @epdenouden so we don't duplicate work