Improve summary stats when using '--collect-only' #7875
Conversation
|
Thanks @caramelomartins for the contribution! Sorry for the delay, have been pretty busy. I will take a closer look on this over the next few days, thanks for the patience! |
|
It is okay @nicoddemus I just wasn't sure if was supposed to ask someone for a review or simply wait. 😂 |
Either way is fine, and a ping is always welcome. 😁 |
This now better separates the very different concerns now between the summary status during normal runs and the new summary status for "--collect-only".
|
Hi @caramelomartins! This looks good, but I noticed we didn't have a specific test for all the cases we wanted to handle, so I added it. While at it decided to refactor the function because it had grown too much functionality and it was hard to follow. For my part this is now good to go, I will wait a while for other maintainers to take a look if they want to. 👍 |
There was a problem hiding this comment.
Nice improvement @caramelomartins. LGTM with a couple of comments inline, and also, I would change "X tests found" to "X tests collected" since that's the terminology we use.
Done. I did change to "matched" to convey when there was deselection, but it didn't work so well in the end. "collected" is perfect, don't know why it didn't occur to me, thanks! All changes applied @bluetech. 👍 (I didn't bother changing the docs because they will be updated next time we run regendoc during the release). |
|
Thanks again @caramelomartins! |
|
@nicoddemus and @bluetech thanks for accepting this PR. Your suggestions and refactor made it much cleaner, thanks for the effort of making it better. 🙂 |
Hey,
This PR intends to improve on the output of the summary stats line when using the
--collection-only. I wrote it a bit "hackishly" because I was discovering the codebase so any feedback on improving the code, or the behavior, would be greatly appreciated. I also took the liberty of updating the existing tests to match this new behavior.I tried to make the behavior similar to what was requested in #7701 and it results in something similar to:
Additionally, I noticed that in the examples found in the documentation, it only displays contexts where only "no tests ran in (...)s" happens...would it make sense to add more examples of different outputs?
Closes #7701.
Let me know how you feel about this.
Thanks!