Adds support for loading reporter from an absolute or relative path.

[sul4bh]

This closes #2434

Created this PR based on @cletusw comment here: #2453 (comment)

@boneskull Please review.

[jsf-clabot]

All committers have signed the CLA.

[Munter]

Looks good to me

[sul4bh]

Not sure why the build job #3029.7 failed. Probably a one-off failure.

[Munter]

We've been having some false negatives on the integrations tests lately. I've opened #2660 to try and resolve it. Also restarted the failing travis test

[sul4bh]

Any update on this?

[dasilvacontin]

We should avoid merging new features that do not include tests for it. I'll add some.

[andywer]

I know this PR has been closed some months ago, but it seems that bin/_mocha hasn't been updated since. Is that a bug?

[ScottFreeCode]

Thanks for the heads-up!

Reviewing the issues/PRs/behavior, I've found:

• on Mocha 3.0.0
  • mocha --reporter relative/path[.js] works
  • mocha --reporter /absolute/path[.js] works
  • mocha --reporter ./relative/path[.js] fails
• on Mocha 3.4.2
  • mocha --reporter relative/path[.js] works
  • mocha --reporter /absolute/path[.js] works
  • mocha --reporter ./relative/path[.js] fails
• After this PR was merged, tests were added for the two behaviors that worked all along: https://github.com/mochajs/mocha/pull/2773/files
• At a glance, it looks like reporter lookup logic is duplicated between lib/mocha.js and bin/_mocha (if so, this is definitely a "bug" in and of itself simply because it creates maintenance issues like this actual bug).
  • (This includes module.paths manipulation; see below.)
• I think (it's hard to be sure without documentation) that

  mocha/bin/_mocha

  Line 179 in 958fbb4

  module.paths.push(cwd, join(cwd, 'node_modules'));

  is what's enabling relative-without-./ path support.
  • I think it's also enabling support for npm i mocha <3rd-party reporter> && node_modules/.bin/mocha --reporter <3rd-party reporter>
  • If I'm not mistaken about that, then it'd be nice to not only get support for ./ reporter paths but to also get support for 3rd-party-in-node_modules reporters without module.paths manipulation, as that would allow us to drop the module.paths manipulation to not only gain compatibility with environments that lack it but bring them up to parity with Mocha on Node.
    • However, we'd need to ensure nothing else is also relying on it (interfaces, --require, --compilers -- anything else?) and/or that everything else relying on it is also corrected to handle the logic without module.paths. (See below.)

My recommendations based on what I've found so far:

1. Expand the test added in https://github.com/mochajs/mocha/pull/2773/files from including only these cases:

• local/reporter
• /absolute/path/to/local/reporter
  ...to including these cases:
• local/reporter
• ./local/reporter
• /absolute/path/to/local/reporter
• reporter-in-node_modules (this will probably involve temporarily linking or copying a folder into node_modules, preferably using some npm mechanism for doing so safely and cross-platform)
• <built-in reporter>
  Some of these cases are probably covered in other tests already; if so, we should either consolidate reporter lookup tests, or (in the case of tests that happen to cover this behavior but are primarily testing something else) use the existing tests as a reference when writing tests that are specifically intended to cover this behavior.

2. Try deleting everything reporter-related in bin/_mocha except the call to the lib/mocha.js reporter load function. If this makes the new tests pass, we're good!
3. Double-check that we have similar tests for looking up custom/3rd-party interfaces, --require, --compilers, and anything else that Mocha currently supports plugging in. (Is there anything else, or have I covered it all?)
4. Try deleting module.paths manipulation. If this doesn't break use of custom/3rd-party reporters, interfaces or whatever else, it should resolve We shouldn't rely on the existence of "module.paths" (Meteor) #2505.

[ScottFreeCode]

Updated to add --require and --compilers alongside 3rd-party interfaces as things that may be affected by the module.paths manipulation.

[rodolfo9]

Hi,
I couldn't load a local path reporter due to an error in bin/_mocha.
Same as @ScottFreeCodee, I found that the reporter loading code is duplicated in bin/_mocha and lib/mocha.js being the latter much finer.

So I've replaced the following lines in bin/_mocha (215:230)

// reporter

mocha.reporter(program.reporter, reporterOptions)._reporter;

// load reporter

var Reporter = null;
try {
  Reporter = require('../lib/reporters/' + program.reporter);
} catch (err) {
  try {
    Reporter = require(program.reporter);
  } catch (err2) {
    throw new Error('reporter "' + program.reporter + '" does not exist');
  }
}

with:

// reporter

var Reporter = mocha.reporter(program.reporter, reporterOptions)._reporter;

And it worked nicely. I would like to see it fixed on next versions.

Cheers!