Fix an error when fn===undefined

[Lana-Light]

This change fixes an error that occurs when reporter is 'xunit' and run is called without callback: " /rbd/pnpm-volume/a49b9a7c-c3f0-42ba-ae66-612ead86e96c/node_modules/.registry.npmjs.org/mocha/5.2.0/node_modules/mocha/lib/reporters/xunit.js:126
fn(failures); ^
TypeError: fn is not a function "

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

Alternate Designs

Why should this be in core?

Benefits

Possible Drawbacks

Applicable issues

[jsf-clabot]

All committers have signed the CLA.

[coveralls]

Coverage increased (+0.04%) to 90.612% when pulling 8736b46 on Lana-Light:patch-1 into a9f3526 on mochajs:master.

[boneskull]

Hi, thanks for the PR!

If this is a bug, it's unclear to me how to reproduce it. Could you explain further? This code (which you've removed) will not attempt to call fn if it is undefined:

fn && fn(failures)

[boneskull]

Also, since this changes logic, adding a unit test would be appropriate.

[Lana-Light]

Hi! Here is link to the project, where this bug occurs. Also there is function run2, which fixes the bug. https://glitch.com/edit/#!/test-function-run?path=server.js:17:0 . When I use mocha.ui('tdd').reporter('xunit').run(function() {}); or mocha.run2 = run2; mocha.ui('tdd').reporter('xunit').run2(); it works, but if mocha.ui('tdd').reporter('xunit').run(); it throws an error. This code if (reporter.done) { reporter.done(failures, fn); } attemts to call fn even if it is undefined and cause this error in console

[boneskull]

The line you've changed isn't the problematic line in your screenshot...

[Lana-Light]

mocha.reporter('xunit').run() returns runner.run(done), which invokes function done(failures) { if (reporter.done) { reporter.done(failures, fn); } else { fn && fn(failures); } } . There is reporter 'xunit' and reporter.done==function, so the function reporter.done is invoked with fn===undefined. Here is code of reporter.done github.com/mochajs/mocha/blob/843cd93ba1e9bfe76db95af7ebeab3d2c22fa810/lib/reporters/xunit.js#L122 , where fn is invoked and an error occurs. With my fix if fn===undefined in the function done, fn will become function() {}. So code which uses done will not attempt to call undefined.

[boneskull]

OK, I understand. Can you please add a test for this?

[Lana-Light]

I have added a test #3620 .

[boneskull]

@Lana-Light The lint check is failing. Did you happen to make these changes directly on GitHub?

After cloning the repo and running npm install, when you commit, your code will automatically be formatted to avoid this problem.

See the step-by-step guide for more info.

[Lana-Light]

I made commits directly on GitHub. Do I have to clone my repo with patch-1?

[boneskull]

@Lana-Light Yes. It's best practice to do this, as most projects (including this one) ask that you run tests on your changes before sending a PR. Additionally, Mocha has tooling which will automatically format your code correctly upon commit, which is not invoked when committing on GitHub.

Making changes on GitHub directly should rarely be used for anything other than documentation updates.

[boneskull]

LGTM, thanks!