Mocha CLI Args

[Jack-Barry]

What's the problem this PR addresses?

Unfortunately, solving any one of these issues on its own wasn't necessarily an easy route to go, so they're all in a pretty big PR, however all issues needed to be addressed in some capacity.

• Separates concerns of parsing args, translating args into options, and passing options along to class constructors
• Brings Mochapack's CLI args parsing up to parity with Mocha 7.1.0 (may not yet implement all arguments if they target CLI, but should be ready for all that target the Mocha constructor)
• Renames MochaWebpack to Mochapack to more clearly define it as the main class within the context of this project
• No more chained function calls all over the place just to initialize an instance of a class with options. Ensures that Mocha and Mochapack classes are initialized with as many options up front as possible as opposed to configuring instances via long chains of function calls, to make CLI options added by Mocha later easier to support
• Directly solves or facilitates easier solutions for: Ability to use a mocharc config file. #9, [Feature] Implement the require interface #13, Add support for --file option #20, [Feature] Pass through arguments into Mocha #34 and [Feature] Allow usage of custom Mocha initializer #46

How did you fix it?

• Separated args parsing into parseArgv and optionsFromParsedArgs functions to make their scope more focused and intent more clear
• Renamed functions that were not clear in their scope/intent
• Converted signal strings for events to constants
• Ensured that all relevant tests still pass when the library uses the new parseArgv, initMocha under the hood

I'd like to ask for help from anyone interested to test this and/or contribute automated tests for particular arguments that you'd like to see covered. I didn't write any new tests for new arguments, however you can see which arguments are not passed directly to the Mocha constructor in this types.ts file. The arguments in the MochaCliOptions interface must be handled in some way by Mochapack itself, which happens in the cli and initMocha functions. All other arguments are passed directly to the Mocha constructor, so any options that can be passed to it in a MochaOptions object are directly handed off in initMocha.

This makes it very clear whether Mochapack needs to do some special handling of Mocha args (that impact the CLI usage) or it just needs a quick update to the mochaOptions.ts file (until hopefully this gets merged, in which case Mochapack no longer has to update references to Mocha's CLI args).

[Jack-Barry]

From the Travis run, looks like a few issues will need to be addressed:

1. Since the goal is to minimize the amount of duplicate work, some files are imported from Mocha now. The lib/cli/run-option-metadata.js, lib/cli/config.js, lib/mocharc.json, lib/errors.js, lib/cli/one-and-dones.js, and lib/cli/run-helpers.js files used in the new args parsing mechanism look like they were introduced at some point in Mocha version 5.2.0, so this breaks compatibility with Mocha 4 unless we want to reimplement the contents of those files. Are there any strong objections to dropping Mocha 4 support? Seems like the quickest fix would be to drop Mocha 4 support and ensure that Travis runs with 5.2.0 or above. (The last release of Mocha within version 4 was December 2017.)
2. I'm not sure what's happening with Mocha 6 🤷🏻‍♂️ Probably related to a difference in the files mentioned above.
3. The tests for initMocha need to have a less strict check of equality for the expected output

Issue 1 I just need to know if it's alright to drop Mocha 4 🤞🏼 and issue 2 if anybody can take a look at that and see if it's clear what the reason for failure is I'd appreciate it.

I should be able to address issue 3 pretty quickly tomorrow.

[larixer]

@Jack-Barry 1. Yep, lets bump mochapack to a new major version and drop Mocha 4

[codecov-io]

Codecov Report

Merging #63 into master will increase coverage by 13.99%.
The diff coverage is 87.87%.

@@             Coverage Diff             @@
##           master      #63       +/-   ##
===========================================
+ Coverage   51.13%   65.13%   +13.99%     
===========================================
  Files          26       42       +16     
  Lines         878     1219      +341     
  Branches      125      135       +10     
===========================================
+ Hits          449      794      +345     
+ Misses        414      403       -11     
- Partials       15       22        +7     

Impacted Files	Coverage Δ
src/runner/runnerUtils/initMocha/loadUI.ts	100.00% <ø> (ø)
src/runner/testRunnerReporter.ts	15.11% <10.00%> (+0.99%)	⬆️
src/runner/TestRunner.ts	16.92% <11.11%> (+0.48%)	⬆️
src/webpack/compiler/registerReadyCallback.ts	22.22% <33.33%> (+9.72%)	⬆️
src/webpack/compiler/registerInMemoryCompiler.ts	18.91% <66.66%> (+2.25%)	⬆️
...c/cli/argsParser/parseArgv/mocha/parseMochaArgs.ts	70.96% <70.96%> (ø)
src/Mochapack.ts	75.00% <75.00%> (ø)
src/cli/argsParser/parseArgv/mocha/mochaOptions.ts	81.25% <81.25%> (ø)
src/runner/runnerUtils/initMocha/initMocha.test.ts	84.31% <84.31%> (ø)
...romParsedArgs/mocha/mergeMochaConfigWithOptions.ts	85.00% <85.00%> (ø)
... and 43 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff12edd...9885c83. Read the comment docs.

[Jack-Barry]

Some egg on my face: The files imported from Mocha as part of this PR were added while their repo was under 5.2.0, but not made available in their published package until 6.0.0. Since 5.2.0 was the last release of 5 and was published in May of 2018, I don't think it's a huge difference. But the tests are now configured to run under Mocha 6 and 7, and the package.json for Mochapack reflects the peer dependency should be Mocha 6 or 7.

If all's clear here, all that's left is a version bump. (Figured I'd save that until everything was resolved).

[larixer]

I remember about this PR, but its quite big and I'm very busy this week. Hopefully I can review it the next week.

[Jack-Barry]

Sounds good, thanks for the update amigo

[larixer]

@Jack-Barry Bravo! Awesome work, thank you so much!

[larixer]

Published new major version: [email protected]

[larixer]

@Jack-Barry Could you check and close issues that should be fixed by your PR in a new major version, or maybe just provide some feedback in the issues that have some impact from your PR?

[Jack-Barry]

@larixer Thanks for merging this in, will make sure to make notes in applicable issue threads.

Looks like I broke something here as per #65 but I figured there would be an untested bug or two to pop up. Will look into that one once a repro project is available or somebody else can provide a similar problem repro project.

[larixer]

@Jack-Barry Is is intended that we invoke requires BEFORE webpack compilation? It seems strange. The intention of the user is to use require for global setup before all tests run, but not before webpack compilation, after it. For example if user requires jsdom-global it will setup browser-like environment and will break compilation, because webpack loaders will think they are invoked in the browser. And the user intent was to invoke jsdom-global before all tests to provide the browser-environment for the tests.

[larixer]

Disregard, there is --include for that