Throw a descriptive error when a non-function is given to a runnable

[Zirak]

Requirements

• Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
• All new code requires tests to ensure against regressions.

Description of the Change

Given a test such as:

it('foobars', 4);

mocha used to throw the following error: fn.call is not a function.

After applying this patch, it will throw a more descriptive error message:
A runnable must be passed a function as its second argument.

(Thanks to ssube for helping with the error wording.)

Alternate Designs

Why should this be in core?

This deals with an error builtin to mocha.

Benefits

While the previous error is technically true, I have personally spent some minutes of my life chasing what looked like a bug in the test itself, and not in the call to it. A more descriptive error message helps bring attention to where the problem originates.

The error message itself is welcome to change. One other possible wording is replacing runnable, a phrase internal to mocha, with test function, which may be more intuitive to users.

Possible Drawbacks

• Any code relying on runner.run returning the original error will now fail.
• This error is reported as if the test itself failed, which may leave the user confused still, wondering what's wrong with their test and why is it telling them such things, and still not end up looking at the function signature.
• The test for this function is, in my opinion, lacking. It merely tests how mocha behaves when it is passed a number, and not one of many possible values. Is there any prior art in mocha for such "generative" tests, for instance where the it string is constructed from variables? A common approach, which may be controversial to introduce if not already present, is something akin to:

var notFunctions = [4, 'four', false, ...];
notFunctions.forEach(function(val) {
    it('handles the value ' + inspect(val), testfn);
});

Thank you for your time.

[coveralls]

Coverage increased (+0.08%) to 92.889% when pulling 7f649bd on Zirak:handle-non-function-runnable into 7d78f20 on mochajs:master.

[craigtaub]

I think i'd rather it tested for the new message rather than lack of old one.

[Zirak]

Would you rather the match be the error message verbatim, or only a portion of?

[Zirak]

Changed to match the error message verbatim.

[craigtaub]

Thanks for the input.

Thinking about your drawbacks points, overall I can't think of anything which would block this fix.

Any code relying on runner.run returning the original error will now fail.

That sounds ok, i'd hope if it wasn't we'd have a test for it.

This error is reported as if the test itself failed,....

I mean isn't the point of this PR to give more context to the user to check the function signature? Not perfect but better than whats there.

[Zirak]

Thanks for the merge!
There is another PR of mine, #4032, that if you have some time could use a maintainer's quick look-over.