No double reloading defaults, fixes #3808

[dedene]

Description of the Change

As the mocha defaults were merged early on(https://github.com/mochajs/mocha/blob/master/lib/cli/options.js#L331), it prevented the yargs parser to properly fallback to its defaults configuration for each cli argument (which are coming from the same defaults (https://github.com/mochajs/mocha/blob/master/lib/cli/run.js#L26). With this fix 'js' is no longer always included in the list of extensions.

Why should this be in core?

This fixes issue #3808

Benefits

Allow mocha to properly run i.e. in Typescript based projects.

[jsf-clabot]

All committers have signed the CLA.

[dedene]

The test for loadOptions fail on the part when no options are given or missing and they should be filled in by the defaults. But isn't it so when no options are given, the defaults as configured in the yargs config will be taken..

mocha/lib/cli/run.js

Line 26 in e654253

const defaults = require('../mocharc');

which has the same effect?

[juergba]

I think the bug can be found here: lib/utils.js

extensions contains the correct CL-input, but does not use it within the function. jsis hard coded.

This is probably the reason:

--extension , --watch-extensions
Updated in v6.0.0. Previously --watch-extensions, but now expanded to affect general test file loading behavior. --watch-extensions is now an alias

[boneskull]

Thanks for the PR. Please revert this change, and see juergba's comment.

That code needs to be changed to use the contents of the extension option instead of .js.

[juergba]

I think we have more than one bug here. Something is wrong with the parsing of the extension option as well.

extension is of yargs option type = "array[]". => "run-option-metadata.js"
The way we parse and coerce those "array[]" => "YARGS_PARSER_CONFIG" = {'combine-arrays': true}
This must be the reason why the default extension which is the last member in the access sequence, is added to the extension array.

@dedene You have to get rid of this default js somehow. You can't just delete mocharc as you did before. Probably you should delete the default js before parsing. After parsing you don't know anymore which soure file provided js.

[boneskull]

extension is of yargs option type = "array[]". => "run-option-metadata.js"
The way we parse and coerce those "array[]" => "YARGS_PARSER_CONFIG" = {'combine-arrays': true}
This must be the reason why the default extension which is the last member in the > access sequence, is added to the extension array.

This is likely the reason. It's a regression, then. Essentially, I think we're going to need a special case that says "when merging the default configuration, if extension is defined and not the default value, do not merge the default value into the array."

[boneskull]

We want "combine arrays" to work in all other cases (AFAIK!) except this one.

[boneskull]

@dedene this is kind of hairy, and there's no easy way to make yargs-parser do what we want here--nor is it easy to explain exactly what it is we want to happen. If you don't want to continue with this PR, please let me know, and I'll pick it up.

[dedene]

@juergba @boneskull thanks for the feedback! I don't have much time this week, so feel free to take it over. Shall I close the PR or can you edit and continue on this PR?

[juergba]

@boneskull maybe you can use yargs "coerce" option.

When you do it before reducing the array to unique values, the logic is simple:
when "extension.length > 1" then delete one "js". If there is a second "js", it's one the user set intentionally and must not be deleted.

[boneskull]

closed in lieu of #3834