Use more visually-distinctive characters on the 'dot' reporter

[ELLIOTTCABLE]

I see that on non-Windows, the characters are usually some Unicode; is there any particular pretty characters people would rather see than ! and ,? (Personally, I think this looks really, really great even with those basic characters.)

[ELLIOTTCABLE]

The tests are failing in a completely different way on my local machine than on Travis; and I'm really not familiar with the test-suite; does anybody with a more intimate knowledge of the codebase know why such an extremely simple change (and exclusively in code that affects output, to boot!) might be trashing up so many unrelated tests?

I'm happy to fix it, I just don't even know where to start. It seemed like such a simple change. 😑

[danielstjules]

There's integration tests that are failing, which read output from mocha to verify correct functionality. If you look at some of the errors, the exclamation mark is being prepended to some extracted strings used in assertions: https://travis-ci.org/mochajs/mocha/jobs/95875913#L1640

[danielstjules]

In any case, this could be a breaking change for tools that wrap or work with mocha

[ELLIOTTCABLE]

Really? Shouldn't those tools be wrapping the json reporter, not the prettified, ANSI-code-including, human reporters? :x

And, hm, I see the ! prepended to some output; but I have no idea why symbols.bang (which didn't exist before I defined it. Ugh. Well, I'd love to see this change, because the colour of a tiny dot is really not a visually-accessible output for those with poor sight (especially with default ANSI colours in many terminals). I think it's beyond me to fix this PR, though; because this looks like a mess with strange aspects of the test-suite. /=

[danielstjules]

It looks like there's 3 files that just need to have runMocha changed to runMochaJSON

https://github.com/mochajs/mocha/search?utf8=%E2%9C%93&q=runMocha&type=Code
test/integration/hooks.js
test/integration/hook.err.js
test/integration/regression.js

[danielstjules]

In the case of hooks, looks like the spec reporter is used because they use console.log in some places to verify hook invocation order, so what's printed to stdout isn't valid JSON

[boneskull]

what's the need for this? I'm leaning towards closing this one.

[danielstjules]

Nvm, it's only a single test file that has failures. Try running make test-integration. All failures are in test/integration/hook.err.js

Quick fix to make things pass:

        lines = res.output
          .split(/[\n․]+/)
// =>
        lines = res.output
          .split(/[\n․!,]+/)

Or even better, update the split to use lib/reporters/base.symbols

[danielstjules]

@boneskull The PR makes it easier to distinguish between passing/failing/pending tests in the output. For example, rspec uses:

....F.....*.....

Rather than simply vary the dots by color.

[ELLIOTTCABLE]

I'll take a look at fixing it myself later tonight! Thanks for helping with
that!

So the question now is: is ‘scraping’ the ANSI/pretty reporters a supported
interaction method? I personally definitely don't think it should be, but
that's not my call. If it is supported, though, this is a breaking
change, and would need to wait for a better reason to break the version.

As for the why: like I said above, it's very visually indistinct; besides
the aesthetic argument, it's a issue of usability: dot is the shortest /
most fs-watch-useful / cleanest reporter (and arguably the most useful
besides spec!), and there's definitely accessibility issues with
undifferentiated output.

(At the very least, can we switch to commas/exclamations when COLOUR is
disabled? In that case you're literally losing the information about
where the failures are occurring, because they're only currently exposed
via that colour. :P)
On Wed, Dec 9, 2015 at 6:09 PM Daniel St. Jules notifications@github.com
wrote:

Nvm, it's only a single test file that has failures. Try running make
test-integration. All failures are in test/integration/hook.err.js

Quick fix to make things pass:

    lines = res.output
      .split(/[\n․]+/)// =>
    lines = res.output
      .split(/[\n․!,]+/)

Or even better, update the split to use lib/reporters/base.symbols

—
Reply to this email directly or view it on GitHub
#2000 (comment).

[boneskull]

OK.

This is a change in Mocha's output. Due to past experiences with this, I have to recommend this wait for a major release.

We can release v3 and make #1969 v4 if someone wants to work out the merges.

[danielstjules]

Looks like this is similar to #1702

[ELLIOTTCABLE]

@danielstjules good catch! Yeah, these are trying to fix similar problems, in different ways.

His is an alternative option, whereas my point is that the current behaviour is actively user-hostile and that the defaults should be changed. But yeah, only one of these needs to be taken.

Note: Fixed the test that was failing according to your instructions. Also rebased on top of current mainline. (=

[ELLIOTTCABLE]

Just checking if this is still welcome? @boneskull, @danielstjules?

(Edit: Keeping it rebased on-top of current master.)

[boneskull]

@ELLIOTTCABLE I think we want to do this, but we can't merge it, unless we do another major release. I'm busy working on #1969. If someone wishes to coordinate a major before that lands, I'm happy to release it.

cc @mochajs/mocha

[boneskull]

to that end, I'm going to tentatively schedule this for the v3 milestone

[boneskull]

if we do end up doing a major before #1969, then this may need rebasing against the v3.0.0 branch

[boneskull]

fixed in v3.0.0 branch via a8e63d1 and dedf03d

@ELLIOTTCABLE thanks!