[Feature] Pass through arguments into Mocha

[JamesMcMahon]

• I'd be willing to implement this feature

Describe the user story

Why

As user of mochapack
I want be able to use all the command line features of mocha, both currently and as mocha is updated
So that I don't have to wait on mochapack to be updated

Acceptance Criteria

Scenario: 
Given a command line option that mochapack doesn't currently support (ie, `--forbid-pending` or `--file`)
When I add that option when I run mochapack, `npx mochapack --forbid-pending *.test.js`
Then I see mochapack behavior in the same way as the option would behavior in mocha

Describe the solution you'd like

While working on #30 it became clear to me that while it would be easy to add additional mocha options to the cli, it feels sub-optimal to have to essentially write the same code and tests for each option.

I'd like to refactor and add to the internals of mochapack to support a passthrough of args to mocha. That way we don't need to scramble to support additional args as the mocha people add them.

Describe the drawbacks of your solution

Not sure how this would effect the programmatic API of the mochapack (or if we care about that as a feature). There is a possibility of breaking changes to that API, but as far as I can tell the CLI options are 1 for 1.

I also think it's possible to implement the changes in a way that the API does not break, with possible deprecation markers being added to certain methods that we want to remove in the future.

Describe alternatives you've considered

Looking for feedback on this thread of alternative solutions.

Additional context

This make sense to me given the current requested features from both this project and mocha-webpack. I also think this could result in a simpler code base as right now mochapack is deeply concerned with the particulars of mocha's options. I think we should get out of the business of trying to re-implement mocha features.

I am willing to work on this. It feels like a decent chunk of work, though I am admittedly new the project, so my estimate may be off. I wanted to get buy in before embarking on a PR.

Command line option requests:

• Add support for --file option #20
• Implement --forbid-only #14
• Add support for --inspect and --inspect-brk options #5
• Support for Mocha option --globals zinserjan/mocha-webpack#192
• Support for --debug option zinserjan/mocha-webpack#201
• Forbid only and forbid pending zinserjan/mocha-webpack#197

Probably missed a few, but you get the gist 😆

[larixer]

I agree with this approach in general, we should stop trying to re-implement mocha features, but the question is how to do it :)

[JamesMcMahon]

Sorry for the lack of follow up on this. I want to work on this but I haven't had the time to put into it. Hoping to get to it in the new year.

[larixer]

@JamesMcMahon No problems, this is open source, we all are volunteers, take your time!

[JamesMcMahon]

Jack's PR for Mocha, mochajs/mocha#4122 seems like it would be helpful here.

[ghost]

A possible workaround could be to add a parameter to mochapack --mochaOpts and then pass it on to mocha's --opts: https://mochajs.org/#-opts-path

That way you should be able to allow your users to utilize the full power of mocha, without having to wrap each parameter. It still feels a bit like a workaround to me, but perhaps it's good enough.

[ghost]

Oh didn't notice :/ that seems to be deprecated :( Perhaps the config file will do the trick: https://mochajs.org/#-config-path

[Jack-Barry]

I think the snag here though is that if a user points to a config file then they still might want to override the configured settings for a one-off run of their test script.

Hopefully I can get something rigged up to get as close to parity with Mocha as possible by end of next week. Since a few of our projects at work are relying on this, I am pretty much going to be focused on it until it's up to snuff.

Barring acceptance of mochajs/mocha#4122, the best option here that I can see is to split the Mocha specific options into their own file to at least make updating them more painless until that PR does get merged in. Once it does get merged we just point to that and can delete the file out.

[Jack-Barry]

@larixer and @JamesMcMahon I am tackling this issue this week ✊🏼 Jumping into it, there are a couple of things I wanted to get some second/third opinions on:

Short Version

1. Should we split the options groups into Mocha Mocha's defined groups, Webpack, and Mochapack instead of Basic, Output and Advanced?
2. Should we err on the side of cleaner architecture and no support for Mocha aliases or a dirtier architecture with full support for Mocha aliases?
3. Is there any reason you are aware of that we should apply the Mocha options to the instance via function calls instead of passing options directly to the Mocha constructor?

Long Version

1. The options groups don't make much sense to me (Basic, Output, Advanced). I think it would make more sense to split out the options groups to the user as ~~ Mocha~~ Mocha's defined groups, Webpack and Mochapack. This would more clearly point the user to which library they can expect the CLI arguments to target. One drawback is that this would not preserve how Mocha splits up their options groups (at least from Yargs docs, it doesn't look like sub-grouping/multi-level grouping is a thing but maybe I'm wrong), but this seems like a minor loss since a user can always refer to Mocha docs if needed for options by group. Scratch that, Mocha specific options would still be broken down according to the group set in the options provided by Mocha.
2. Mocha does not include their aliases in the options object they provide to Yargs, but rather they provide aliases as a separate argument to the aliases function. This means that Mochapack would need to import their options as well as aliases, then separately merge those for when Mochapack calls Yargs. A simpler route would be to just enforce non-aliased CLI arguments for Mocha when using it within Mochapack. I'm personally OK with the latter as it would keep Mochapack's architecture cleaner, but not sure how much of an impact this would have on the typical user of Mochapack. Seems easy enough to change a CLI argument to the non-aliased version to me, but don't want to assume this for anyone else 🤷🏻‍♂️
3. I think the biggest hurdle to providing support for future Mocha CLI options is that the Mochapack codebase has the application of options to the Mocha instance sprinkled in multiple places: src/cli/parseArgv.ts, src/runner/configureMocha.ts, and src/MochaWebpack.ts as far as I've found at least. This means that we don't only have one place to make an update to support a new option, but at least 3 potentially. I see that configureMocha provides options directly to the Mocha constructor - is there any reason why we can't provide the rest of the options for the instance directly as well? The options it looks like can't be provided to it directly are those which all seem like they would have already been utilized elsewhere anyway. Of course those would probably still pose somewhat of a barrier to adding new options since they require handling elsewhere, but at least we could minimize the changes needed for options that directly affect the Mocha instance. Here are the options that are available as CLI args that are not handed directly to Mocha which would either need to utilize existing Mocha code to implement if possible, or be overridden by Mochapack with an explanation to the end user of why:

• config
• exit
• extension
• file
• ignore
• invert
• list-interfaces
• list-reporters
• package
• recursive
• reporter-option
• sort
• watch
• watch-files
• watch-ignore

[larixer]

@Jack-Barry

1. Probably yes
2. Unsure
3. No apparent reason, lets go constructor way if its simpler and better

Generally, anything that improve compatibility with future Mocha versions and reduce maintenance burden should be positive.

[Jack-Barry]

Followup for previous comment:

1. Went ahead and split it up into Mochapack, Webpack and left Mocha's groupings up to their parsing rules
2. Found a super easy way to use Mocha's aliases and types in Yargs so it's a non-issue
3. More on that below 😄

Jumped ship on my previous stab at this (forgot what the hell I was doing on it). However, making some huge progress on this and pretty excited about it: https://github.com/Jack-Barry/mochapack/tree/mocha-cli-args

Building out the replacement tooling on the side so that it doesn't interfere with too many existing files. Added a temporary script as test:temp to run tests that apply to the new code.

Modularized Args Parsing

A new implementation of parseArgv provides a more modular architecture around parsing incoming arguments. To make things easier to track, it splits the incoming arguments up into an object in the format of

parsedArgs = { 
  mocha: {}, 
  webpack: {}, 
  mochapack: {} 
}

One Place for Args-to-Options Translation

Functionality for translating the parsed arguments into an object that can be used to initialize Mochapack is split into its own set of functions under optionsFromParsedArgs. This provides a more clear distinction between parsing the arguments vs. translating them into a MochapackOptions object that includes an object that can be provided directly to a Mocha constructor. It splits translated options up into an object in the format of:

mochaPackOptions = {
  mocha: {
    cli: {},
    constructor: {},
  },
  webpack: {
    config: '',
    env: '',
    // ...
  },
  mochapack: {}
}

where mocha.cli houses any arguments from the user that do not translate into the options that are passed to the Mocha constructor, but rather must be handled in another way by Mochapack. Options under webpack and mochapack are mostly untouched, except keys are converted to camelCase and webpack-config and webpack-env keys get simplified to config and env

The Really Exciting Part

🎉 A new initMocha function initializes Mocha by providing it options up front instead of making subsequent function calls. The bad news is that in some cases there are arguments that can be provided via CLI that are not used in the Mocha constructor, so for new arguments added that fall under that umbrella we still might need to perform ad-hoc updates later. However, given the new parsing scheme, args-to-options translator, and an initializer that uses options up front, we'll save some of the overhead of having to implement each and every CLI option, and it should be a little more straightforward to add functionality.

Next Steps

There's still plenty of work to do to get this wrapped up, but overall I think some of these changes will make extending the functionality in other areas of Mochapack a lot more straightforward in the future as well. A lot of gigantic functions can be broken down into smaller chunks that are easier to reason about and build on top of. parseArgv was one case where it was taking on a lot of responsibility that it probably shouldn't have and got confusing to reason about (at least for me).

I'll be trying to get the following done over the next few days:

• Create a new Mochapack class to eventually replace MochaWebpack, this new class will also need to minimize subsequent function calls used as configuration by providing configuration up front instead
• Ensure that the new Mochapack class can effectively be used as a replacement within the CLI

[larixer]

@Jack-Barry Excellent progress! I like how your approach gives structure to the options parsing and utilizing by different tools. Thanks!

[Jack-Barry]

@JamesMcMahon Would you consider this resolved as per the finalization of #63?