WIP: Consolidate mini and verbose reporters #1776
Conversation
| if (str) { | ||
| this.write(indentString(str, 2) + os.EOL); | ||
| } else { | ||
| this.write(os.EOL); |
There was a problem hiding this comment.
No need to use os.EOL. Windows also supports \n for console output. Even console.log in Node.js uses \n: https://github.com/nodejs/node/blob/34bd9f318a164d6b2ee949793146b85b3e152d5f/lib/console.js#L141-L148
| return; | ||
| } | ||
|
|
||
| const str = chunk.toString('utf8'); |
There was a problem hiding this comment.
Using .toString('utf8') on chunks is an anti-pattern, as the chunk might be split on a Unicode code point, resulting in an invalid character. The correct way is to do `setEncoding('utf8') on the stream. That is also more efficient.
| } | ||
| } | ||
|
|
||
| consumeStateChange(evt) { // eslint-disable-line complexity |
There was a problem hiding this comment.
Can you use a more descriptive name than evt?
| // we multiplex stdout and stderr into a single stream. However as | ||
| // long as stdStream is different from reportStream users can read | ||
| // their original output by redirecting the streams. | ||
| if (evt.chunk[evt.chunk.length - 1] !== 0x0A) { |
There was a problem hiding this comment.
Can you make 0x0A a local constant, so it's clear what it is?
| } | ||
|
|
||
| _writeWithSpinner(chunk) { | ||
| if (!this.spinner || !this.spinner.id) { |
There was a problem hiding this comment.
With latest Ora, you can check this.spinner.isSpinning instead of this.spinner.id: sindresorhus/ora@3df8d0d
|
It may be worth considering not printing non-AVA output unless |
👍 Seems fine. We have |
|
Closing for now. |
Calling this a work in progress is an overstatement, but I wanted to at least communicate what I'm trying to do.
#1722 did some major refactoring of the reporters. Here, I'd like to consolidate the mini and verbose reporters into a single, default reporter.
--verbose) only failures are shown.t.log()isn't shown for passing tests--verbose. It only did that to disable the spinner, but the reporter now detects whether a spinner is supported.Fixes #653. Fixes #1337.