-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Consolidate mini and verbose reporters #2217
Comments
@yovasx2 you've privately expressed interest in working on this. If you comment on this issue I can assign it to you within the GitHub interface. |
You could assign this task on me. But I have to warn you that this is my first time on github, so it will take some time to get familiar with. |
@biofreak that's great, but @yovasx2 is working on this at the moment. |
Sure |
as of #2458 I expressed my interest to work with this and I'd take it! |
@pcdevil did you manage to take a stab at this? I think @Michael55555 is interested in working on this as well. |
Hello @novemberborn, Sorry, I only did small improvements locally, started to outsource the identical part of VerboseReporter.consumeStateChange() into a new Unfortunately I didn't have much time in the last two weeks as anticipated. I can hand it over that code to @Michael55555, no harm on my part if he wants to take it over! |
Thanks for the update @pcdevil. FWIW I don't think we need any base class. We can envision a single reporter, which in verbose mode… reports more! @Michael55555 what do you think? |
Moving the functionality from minimal to verbose would possibly be the best way to do it. |
I'm not sure if modifying the current code is going to cut it. |
Ok, I see. I think creating a base class would be great to avoid writing code twice. Some functions in the tap and the verbose reporter seem to be nearly the same. |
I don't want us to expend any energy on the TAP reporter. Longer term we'll move it out of this repository. The duplication is fine for the time being. |
Alright, I see. I just created a |
@novemberborn I opened a pull request referencing this issue already, I'd be happy if you review the changes 😄. #2488 |
Currently we have two different reporter implementations. Code is duplicated across them which leads to various bugs. I once tried to consolidate them but that work stalled.
The idea is that we would have only one reporter. By default it provides minimal output. We show a spinner and the status of the last test, so you get a sense of activity, and once an entire test file completes do we print failures. In verbose mode, we'd print the results of all tests & hooks, including logs. Various counts for passing and failing tests are always shown at the end of the output.If a TTY is not available, we wouldn't print a spinner and only write the appropriate output for a test file once it completes.We'd useink
to construct the output.The text was updated successfully, but these errors were encountered: