This prevents interference between the mini logger and child processes that use `console.log`. The mini reporter previously used logUpdate which deletes lines previously written with the logUpdate API before writing a new one. This caused problems if lines from console.log were written inbetween logUpdate calls. It would delete the users log messages instead of the test status line. To fix this, we store the last written last log line, clear it, write the users log output, then restore the last log line. This keeps the miniReporter output always at the bottom of the log output.
It also fixes an incorrect use of the `child_process` API. We were using the `stdio` option for `child_process.fork`, but that option is ignored (it is honored for just about every other method in that API). See: 7b355c5bb3/lib/child_process.js (L49-L50)
It also adds a set of visual tests which can be run via `npm run visual`. They must be run manually, and should be run as part of our pre-release process.
This fixes that regression, and extends the promise returning behavior to all assertions.
This would be useful in `async` test functions to bail on the remaining tests if some precondition fails. Simply `await` the assertion.
This uses `nyc` directly instead of via `tap`. There is currently some semver brokeness happening due to a major version bump in `nyc` making it's way into a minor version bump for `tap`. I think the easiest solution is just to avoid the problem altogether by turning off `tap`s coverage option and using `nyc` directly.
Reference:
https://github.com/isaacs/node-tap/pull/197
---
This also switches us to the `classic` reporter. Historically were using `dots` reporter with `tape`, but that provided poor diagnostic information on failures. The `classic` reporter does a much better job of providing all the information we need when a test fails.
There was an incorrect plan count in one of the tests. It's troubling that this did not cause a failure.
Issue filed in `tap` project:
https://github.com/isaacs/node-tap/issues/198
It's become a bit annoying at times and it doesn't always make sense to require 2 LGTM's. It's better to use our best judgement. But the main reason I'm removing it is that enabling it means we can no longer push directly to master.