Skip to content
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

[RFC]: Rethinking reporters in Jest #7900

Open
SimenB opened this issue Feb 14, 2019 · 14 comments
Open

[RFC]: Rethinking reporters in Jest #7900

SimenB opened this issue Feb 14, 2019 · 14 comments

Comments

@SimenB
Copy link
Member

SimenB commented Feb 14, 2019

Reporters (that want to print to stdout) in Jest are pretty advanced as they have to replace existing text to show status (both per test and for the total run), they have to handle test results coming in out of order due to parallelized test execution and they have to print log output, assertion errors etc.. All of this has to look smooth with nice colors both on local machines and on CI.

Jest currently has 3 (4) built-in reporters: standard, verbose and coverage (the fourth being a Notify reporter, but that doesn't write to stdout). Those 3 reporters are implemented by a cool 2941 lines of code according to cloc. Source code lives here.

Implementation wise, a reporter currently have to implement an interface (documented here) where functions are called whenever an event happens (such as onRunStart, onTestResult etc.).

The problem with the current way reporters are implemented, is that they all have to print to process.stdout manually, dealing with whether or not it's interactive, clearing (or not) stale output. This is a hard thing to get right (e.g. #1781 is 2.5 years old). And you cannot really have multiple reporters working at the same time - they will trip each other up as there's no way to synchronize writes to the stream.


I'd love to be able to offload a lot of the work these reporters currently do to something better suited for building reactive UIs - React. We would then just re-render with new props whenever we have new state, React would give us how it's supposed to look, and we could offload to ink to actually render the output in the terminal.

Jest itself could also expose all the different parts of its default reporter (a few quick screenshots below) as React components, making it easier to build a custom reporters.

image

image

image

image

What do people think?

I currently have a working prototype implementation of Jest's current reporters using Ink here.

/cc @vadimdemedes

PS: Reporters that just want to write to a file and not stdout (e.g. jest-junit or the built-in Notify reporter) would not be affected by this change.

PPS: It might make sense to make it easier writing watch plugins in ink as well (if it's hard, I haven't tested), but that's out of scope.

@jeysal
Copy link
Contributor

jeysal commented Feb 14, 2019

I think this is one of the most exciting and necessary changes to improve UX, especially combined with your PPS. Would love to help work on this. Is there a good way to manage the transition phase? Using an ink and a legacy non-ink reporter at the same time seems like it would be hard to deal with.

@milesj
Copy link

milesj commented Feb 14, 2019

If I'm not mistaken, Ink uses log-update under the hood, which has problems re-rendering content larger than the terminal height? This may be an issue with Jest's RUN/PASS/FAIL stack?

Outside of that, I've ran into similar problems with Boost's console, where reporters all need to write to a stream in parallel. I believe I solved the problem by supporting 2 modes for writing:

  • Enqueuing an "output block", which triggers a render loop that continuously renders the block until the block is marked complete. Once all blocks are complete, the loop stops.

  • Writing to the stream directly. If the render loop is running, the stream is buffered until the next frame, otherwise the content is written immediately.

This requires all the writing logic to be handled in a single layer (the Console), instead of being handled in each reporter. From the looks of Jest's reporters, they all talk to process.stdout or process.stderr directly instead of through a unified layer.

@SimenB
Copy link
Member Author

SimenB commented Feb 14, 2019

Is there a good way to manage the transition phase? Using an ink and a legacy non-ink reporter at the same time seems like it would be hard to deal with.

Yeah, I think it'd have to be opt-in (through config or just reporter[0] having some static property). Supporting both at once seems out of the question, tbh

If I'm not mistaken, Ink uses log-update under the hood

Current version does, V2 (which is the one we'd use) does not: https://github.com/vadimdemedes/ink/blob/440b83140a8e16ddfc5ad1b01bdc156f09350aba/package.json#L40-L52. Not to say it won't have issue of course. I did not encounter that when testing (IIRC, this was back in early November, might've just forgotten)

@milesj
Copy link

milesj commented Feb 14, 2019

@SimenB Looks like they forked it: https://github.com/vadimdemedes/ink/blob/440b83140a8e16ddfc5ad1b01bdc156f09350aba/src/vendor/log-update.js But if you haven't run into any issues, then hopefully it's fine.

@SimenB
Copy link
Member Author

SimenB commented Feb 14, 2019

Ah, fair enough! I'm not really married to the choice of ink (there's also e.g. react-slate and react-blessed), it's just the one I've already played with, and (albeit with limited testing) it worked great. Hopefully the exact renderer won't really matter in the end, but we should make sure to test for edge cases such as that.

The goal (at least my goal) here is to make it easier to create beautiful interfaces with Jest, and I think migrating to a system that allows people to use React components unlocks some super exciting possibilities. The exact implementation of that is really fuzzy 🙂

EDIT: webpack-dashboard migrated to neo-blessed: https://github.com/FormidableLabs/webpack-dashboard/releases/tag/3.0.0

@milesj
Copy link

milesj commented Feb 14, 2019

Migrating to a system that allows people to use React components unlocks some super exciting possibilities.

Agreed! Hopefully it works :P

@vadimdemedes
Copy link

vadimdemedes commented Feb 15, 2019

@SimenB glad you were able to finish the PR, really happy to see it! As I've said before, let me know if you run into any issues!

If I'm not mistaken, Ink uses log-update under the hood, which has problems re-rendering content larger than the terminal height? This may be an issue with Jest's RUN/PASS/FAIL stack?

@milesj That is correct, Ink uses log-update to render output, but truth is it's not log-update's fault. It just seems that terminals are unable to properly erase last N lines of output if N is larger than terminal height (rows). What Jest actually does is it erases all session, including any previous output from other commands. I've tried doing that on every render in Ink, but rendering was lagging a lot if you have high frequency of renderings.

To work around this limitation, Ink has a <Static> component, which "flushes" all its children permanently, bypassing log-update. It means that <Static> is the perfect candidate for things like list of completed tests in Jest. But, once those components are written to stdout, that output can't be modified. I actually created a demo implementation of Jest's output with Ink as an example here - https://github.com/vadimdemedes/ink/blob/next/examples/jest/jest.js#L40-L61. You can run it by cloning Ink's next branch, installing deps and running node examples/jest.

@SimenB
Copy link
Member Author

SimenB commented Feb 15, 2019

Sorta related: #3160

@milesj
Copy link

milesj commented Feb 15, 2019

@vadimdemedes Yup ran into similar issues when implementing my console. That Jest POC is pretty slick, love it 👍

@SimenB
Copy link
Member Author

SimenB commented Mar 4, 2019

The underlying wrap-ansi issues has been fixed 🎉

In celebration, I merged in master into my branch (almost worked 😅). Will be taking a look at this in a couple of weeks 🙂

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@SimenB
Copy link
Member Author

SimenB commented Feb 27, 2022

I still want to do this, work is ongoing (albeit stalled 😅) in https://github.com/jest-community/jest-react-reporter. Any and all help welcome!

@pauldraper
Copy link

pauldraper commented Apr 2, 2022

This is all very ambitions, but IMO want a less sophisticated reporter. Like one that just lets me console.log and have it appear.

Jest makes debugging slow tests/infinite loops is really really hard. I haven't had that issue with other test frameworks, likely because they didn't get fancy about stdout.

@CMCDragonkai
Copy link

I find that jest reporter is overwriting/hiding STDERR output that comes from a native add on. This makes it hard to debug this I have to repeat the logs several times in the native addon to avoid jest reporter clobbering the stderr output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants