-
-
Notifications
You must be signed in to change notification settings - Fork 617
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
Added GoogleTest style output for unit tests #1078
Conversation
I like it... but I'm not sure if that's the practical part of me, or the "ooh, shiny thing" part of me ;) Anyway, I'm not saying don't merge this... it's cute. Just that there's a part of me that wonders if it's sensible :P |
Well, I certainly like it because it's shiny, but there is more in this commit then just shiny output, it also does 'duplicate name detection', which already caught a few issues, and it's sorting the tests for a more deterministic run order. So for that reason I think we can take the shiny too ;) |
I hear what you're saying, it's very much shiny. However, there is a benefit to this; you can see the timings. This allows us to look into unit tests that are "taking too long", or for users like me, you can see that the HTTP and Nuget tests take ~20 seconds to run and that you haven't broken anything. Other than that, all other fixes and changes could be done separately. I do agree that emitting two lines for each of the 1783 unit tests, as well as three lines (one empty) for the 177 test suites, and then another 3 lines for the test action itself is a bit intense. An extra 4100 lines isn't something we should ignore, and I'm absolutely happy to discuss changes. Though, it seems like you two like the shiny enough to accept the extra 4100 lines of logging and it does only happen when testing, end users will never see it. |
4100 lines is nothing... I'm looking through logs at least 100x that size at the moment.. So it would be preferable to have a summary at the end of all failed tests (with callstack etc). |
It does list the failed tests at the end, it doesn't emit the error message but that should be an easy fix. Would you like me to do that? |
if you could, and then also rebase the thing so we can merge it. |
5dffb75
to
854a84d
Compare
854a84d
to
5b73685
Compare
Done. This is what the output looks like with errors: (the HTTP tests decided now was the time to break and take ages)
|
No description provided.