-
Notifications
You must be signed in to change notification settings - Fork 8
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
Printed error message does not always match test status #19
Comments
Probably the most useful thing is to aggregate all the warnings and notices and print them all? |
You can never know how large such output can become. Printing everything might cause more problems that it solves. Since PHPUnit already reports first problem out of the box, it can be coherent to print the message that corresponds to that. You've been warned there're issues, now it's up to you to dig further or keep ignoring them. |
I'm talking in my imagination to the potential user of the library, not you :) |
Yes, I understood the context. :) Still not sure just printing the last one is the best way to go; how is one supposed to dig further without a complete set of information? It would be quite annoying to have to resolve each issue one by one when the software could just tell you all the issues at once. Nevertheless, it would be slightly better than what happens currently. |
Printing first message (you said last, I assume it was a typo) goes in line with test status, it complements that feature. For anything else, you can always log errors or execute your application code directly rather than through tests. I'm not superfamiliar with the internals of PHPUnit or this library, but I guess we'd need to store all traces in memory and then dump then when test finishes. I can think of a silly typo in a loop that throws 10,000 identical notices in common code that's involved in hundred of tests, flooding my terminal or killing my IDE. But my opinion might be biased because I mainly write application tests rather than unit tests. Making it an option is not necessarily better, plus it isn't something I find compelled to implement myself. 😅 |
I would accept a PR that keeps the printed trace in sync with the status. However, we can see from the PHPUnit summary output that it counts all (non-silenced) statuses, not just the last one. Therefore, printing all of them is probably the better solution. |
I think the summary counts individual instances of errors (and assertions), it isn't a per-test value as status. I'll see what I can come up with. |
Correct, which is why we should print all of them so the counts match the number of traces printed. |
I've learnt that PHPUnit does not count duplicated warnings (or notices/deprecations, will use warning for simplicity), meaning that if a given piece of code triggers a warning more than once in the test suite or set of filtered tests, it's only counted once in the summary. I had decided to go the extra mile and show all warnings, and this finding encouraged me further. 👉 Pull request #20 |
While working on #18 I noticed that whenever a test triggers several non-fatal messages (warning, deprecation or notice), the first one (whatever its severity) is reported in test status, but it's the last the one that's printed in detail. If last one happens to be silenced, no message gets printed.
vendor/bin/phpunit -c test test/CapabilitiesTest.php --filter ::testMixedSeverities$
In this example, summary and test status is correct but no message is printed. Compare with e.g.:
vendor/bin/phpunit -c test test/CapabilitiesTest.php --filter ::testWarning$
Root problem is that in
\ScriptFUSION\Pip\Printer
test status is only updated once (this is standard PHPUnit behaviour) but trace is overwritten every time, and that's what\ScriptFUSION\Pip\Theme\Theme::onTestFinished()
uses to print the message (this is a custom feature so PHPUnit cannot be used as guide).I don't know what's the intended behaviour. For me, the most useful would be to report the message that causes test status.
The text was updated successfully, but these errors were encountered: