-
-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Percentage going over 100. #58
Comments
This is an issue caused by the @ depends attribute, but in the functional mode |
(closed by mistake) After seeing your example, it seems that you have 159 tests, 52 (211-159) regular tests from those are called twice and 22 incomplete or skipped (211-189). If the fix works you will then see `OK (128 tests, X assertions)... until the skipped/incomplete tests issue is addressed :) |
On the subject of skipped/incomplete tests - for completeness sake, it might be a good reason to implement. However, those tests are not logged in JUnit XML. It seems like the only other way to report on those would be to do code analysis (possibly via the SuiteLoader?). |
@dbaltas So do you need me to test the latest master branch ? Or this one ? https://github.com/brianium/paratest/tree/issues/17-dependent-tests-on-functional-mode |
the master branch. thanks. |
Here's the result : With paratest
With paratest
With PHPUnit
I have found 34 occurences of |
Unfortunately I can't reproduce the problem. Thanks for the feedback and for the patience! Note: I hope that the fact that running with dev-master was almost 1 minute slower than with |
Note: it probably was, as I was running phpunit at the same time. ;) I'll try to do that as soon as I get some time (quite busy at work those next days). |
@dbaltas Still had no time to get a small reproducible test suite, sorry. The issue remains though. With paratest v0.6.0 :
With phpunit :
|
I'm getting 423 of 313 tests executed (120%) here https://travis-ci.org/aik099/qa-tools/jobs/15085005 when executed with
You can easily clone library from here https://github.com/aik099/qa-tools and run paratest on it. With
Running tests without This seems to be an issue with counting how much tests there are in total. |
Either paratest doesn't display stuff correctly or it really runs more tests/assertions, then it should. |
Just as a guess, this looks to me like it's caused by data providers. PHPUnit treats each dataset from a data provider as one test and paratest only looks at the number of test methods in a class. |
I'm using data providers a lot. But if problem is, that paratest thinks, that there are less tests, then how the total test*data_provider count is larger, than during regular phpunit run? |
Yeah, paratest merely counts the number of test methods: https://github.com/brianium/paratest/blob/87aea81277bf54f523f951677e7d11d833ed9f73/src/ParaTest/Parser/Parser.php#L83 PHPUnit has a pretty complex setup for the whole thing, including adding each dataset for a test as a separate test: https://github.com/sebastianbergmann/phpunit/blob/c517984099c588d7cbe04bea9fb756a764769f5f/PHPUnit/Framework/TestSuite.php#L500 |
That doesn't explain why assertion count is too high either. Maybe best approach would be to implement own test runner, based command line test runner, that would:
Or just process |
…ike with dataproviders) fix proposal for #58
I would like to propose a fix for this issue at https://github.com/brianium/paratest/tree/percentage-fix With https://github.com/aik099/qa-tools that now contains a total of 671 tests, it looks like this:
PHPUnit:
Summary:
Pros:
Why not execute dataProviders in advance? Sorry for the wall of text but I would really like to get feedback on this ;) |
In any case having <100% is better (except for shop discount 😄 ) then we have currently. |
This is definitely an improvement. I don't think I am a fan of the idea of executing dataProviders in advance, for pretty much the same reasons @julianseeger mentioned above. I wonder if we need perfect parity with PHPUnit here on text output? It seems reasonable to not have a total count available for concurrent tests. Would the usefulness of ParaTest be hurt if we removed the right side of the counts and the percentage?
|
The total count probably doesn't matter but the percentage seems to be of interest as a measure of progress. In #129 , @grongor even mentioned that he would prefer his own fork (that counts all dataProvider executions of a testmethod as one single test) over the paratest origin. The result is exactly this: correct/usable percentage but completely incorrect total count. So we could make a tradeoff:
|
I think that is totally acceptable. I certainly don't want to step on any toes if the majority of people using this prefer the |
Small issue I've noticed in the output for paratests : the percentage is often over 100.
Exemple:
As you can see here, the percentage shown is 118%. Any idea how this can happen?
Could it be because of skipped / incomplete / @depend tests ?
The text was updated successfully, but these errors were encountered: