Fix glob pattern support: no comma splitting for 'spec' and 'ignore' options

[juergba]

Description

Currently we can pass test files via CLI:

• node mocha test1 test2
• node mocha -- test1 test2
• node mocha --spec test1 --spec test2
• node mocha --spec test1,test2

via configuration files:

• spec: ['test1', 'test2']
• spec: ['test1,test2']

The last option (comma-delimited list) is neither documented nor provided in our Yargs configuration. But it does break Mocha's glob pattern support in certain cases, e.g.
{controllers,test}/**/*.test.js ==> ['{controllers', 'test}/**/*.test.js']

Description of the Change

For options spec and ignore:

• yars-parser: we remove splitting by comma
• yargs: no changes necessary

Applicable issues

closes #4307

[craigtaub]

Just checking, is exclude option alias'd into ignore?

[juergba]

Yes, but vice versa. ignore is the canonical and exclude the alias option name. Yargs-parser applies the coerce function automatically to both names.

[craigtaub]

So as I understand comma-delimited spec: ["test/one/index.spec.js,test/two/index.spec.js"] would work in master but not on the fix (tried locally to confirm).
In which case should this be a breaking change? Regardless of not documenting the feature if we make a fix which breaks a certain existing setup, that sounds like a major.

[juergba]

So as I understand comma-delimited spec: ["test/one/index.spec.js,test/two/index.spec.js"] would work in master but not on the fix [...]

Yes, that's correct. I'm fine with major.

[craigtaub]

Fix looks good, but we only have 1 line of docs on this option and its usage (from what I can see in the Positional Arguments bit). Should we expand on it to add clarity now? Should it have a --spec section?
With introducing a breaking change imagine it could be useful to be able to point people to an explanation. Thoughts?

[juergba]

There is another docs section.
It's somehow confusing to have a positional argument spec and an option --spec. There is no need to use --spec in the CLI, actually it's only needed for the configuration files.
I'm not sure how to document that, I will think about.

[craigtaub]

Nice one

it's only needed for the configuration files

Yes I could not think where else to put it either. Happy to leave.

[boneskull]

To be clear: this removes the ability to specify files such as foo.js,bar.js, but fixes {foo,bar}.js?

[juergba]

Yes, this is correct for spec and ignore:

• CLI: node mocha --spec foo.js,bar.js
• configuration file: spec: ['foo.js,bar.js']

see also comment.

The following has never worked and remains unchanged:

• CLI: node mocha foo.js,bar.js