[Feature] Allow usage of custom Mocha initializer

[Jack-Barry]

• I'd be willing to implement this feature

User Story

It's a pretty big barrier to entry for testing to get a good environment set up for Electron projects. It would be great to be able to run electron-mocha instead of mocha for Electron apps that are built using Webpack (specifically electron-webpack).

Proposed Solution

Add a flag such as --electron to run electron-mocha wherever vanilla mocha would otherwise be used. This has been discussed over here: jprichardson/electron-mocha#102

Drawbacks

• It's another dependency to keep up with (though it could probably be labeled as a peer dependency since not all users would need it)
• electron-mocha includes a few flags that are not standard. These would need to be handed off to it somehow. Flags not used by regular mocha should quickly throw an error if a user tries to include them without --electron (this would probably provide some extra motivation for [Feature] Pass through arguments into Mocha #34 to be tackled)
• If devs are not building with electron-webpack their project structure might deviate from what's expected. This means there would have to be a way to inform electron-mocha of what files to consider renderer vs. main

Alternatives

Currently have a POC project electron-playground which involves maintaining separate scripts for testing:

• Code that has to be run through Webpack before testing (such as .vue files)
• Code that relies on electron and runs in the main process
• Code that relies on electron and runs in the renderer process.
• e2e tests

The tests that require electron are named .e.spec.ts and those that require webpacking are named w.spec.ts to ensure the test runner does not try and run something it can't handle per specific script.

This is cumbersome. electron-mocha does involve spinning up a process for main and renderer tests separately, but this could probably be handled gracefully by mochapack. That would condense all 3 unit test scripts here into one. Basically instead of running one instance of mocha, it would spin up two concurrently but relying on the same bundled code.

e2e tests can and should be handled separately, so handling those in a separate script is not an issue.

[larixer]

@Jack-Barry Thanks, its okay with me to merge support for electron-mocha if you are willing to implement it.

[Jack-Barry]

@larixer Cool deal, I'll start looking into the specifics on this one

[JamesMcMahon]

Kind of parchuting into this one so I hope I am not widly off base. Does calling electron mocha require a different signature the mocha or is it a drop in replacement?

If it's a drop in replacement I wonder if the more generalized solution to allow the user to specify a custom command to invoke?

[Jack-Barry]

@JamesMcMahon I'm not sure if you got a chance to read the discussion referenced above but according to their maintainers it's mostly a drop in replacement.

I haven't had a chance to really dig in, but from what I gathered mochapack is building an instance of the Mocha class, so if there is an analog class in electron-mocha then it would just need to instantiate that instead.

What I have been looking into first though is tackling #34 as the CLI argument parsing will probably have some implications here, and seems to be the root of a few other open issues at the moment. I've initiated a PR with mocha that would be the first step toward this as far as I can tell. It just exports the options that mocha sets up so that the object of options can be extended by mochapack instead of recreated entirely.

[JamesMcMahon]

RE: #34

Making mochapack less knowledgeable about mocha would probably be a boon. Just spit balling here, but do we need to invoke Mocha via API? Could it be a command line pass through instead? If we did that suddenly #34 and this feature are super easy.

[Jack-Barry]

That's the part I haven't had a chance to dig into yet - initially that's what I thought it was doing under the hood already but once I saw a little bit of what's going on in src/runner/TestRunner.js it looks like the API is used for good reason, I just haven't been able to figure out exactly what that is. Would have to understand the limitations of Mocha's CLI vs. API usage.

One other place I do foresee needing to be re-approached if the API continues to be used is src/runner/configureMocha.js, to basically do something like a forEach loop for all the options and run their methods using mocha[method]() notation otherwise even with the flags available from mocha each option ends up hard-coded there.

[larixer]

@Jack-Barry Don't also forget that mochapack compiles files into in-memory fs, which is accessible only to the current Node process. If you launch another Node process, it won't see the compiled files:

mochapack/src/webpack/compiler/registerInMemoryCompiler.js

Line 12 in adcd4cc

compiler.outputFileSystem = memoryFs; // eslint-disable-line no-param-reassign

[Jack-Barry]

@larixer I'm not 100% certain but if mochapack uses electron-mocha's run.js instead of mocha's, it doesn't look like it's spinning up another Node process there, unless that's what Electron does inherently in which case a whole other can of worms just opened 😳 and I just showed my ignorance 🤐

Investigation Notes

Adding some other notes of what I've been investigating here for reference and discussion.

Description of Snippet	Notes
Where the method to build an instance of Mocha and run it is called by electron-mocha	This is what mochapack needs to use since it is how electron-mocha runs mocha within an Electron environment. This file contains the code that would need to be run in lieu of normal mocha
Method where electron-mocha builds an instance of the Mocha class and runs it	This would essentially nullify any use of configureMocha since it is initiating its own instance of Mocha. Might want to take some notes from electron-mocha's strategy for initialization and argument parsing which seems more suited for handling problems such as #34: run.js called from main.js.
Where mochapack runs mocha during normal operation	Would likely need to use electron-mocha's run.js instead of mocha's
Where mochapack runs mocha in watch mode	Should be pretty similar to how a fix to normal mode is implemented

Test Notes

Not sure what would need to be tested, or what tests would be OK to drop if argument parsing is to be handled differently, but my thoughts on what functionality would need to be tested base level:

1. Allows usage of electron internals such as ipcMain and ipcRenderer where applicable
2. Runs tests in the appropriate environment (main or renderer or both). This is probably one of the trickier parts, as it needs to know which tests are meant to run in which environment and use the appropriate bundle
3. mochapack throws an error/warning if any Electron/electron-mocha specific flags are included but the --electron flag is not included
4. electron-mocha specific flags are handed off to it reliably
5. Default is that any spec file in {src,test,spec}/main runs in the main process and anything in {src,test,spec}/renderer runs in the renderer process
6. Should respect flags such as --electron-main, --electron-renderer, and --electron-both which are globs specifying what spec files to run in each environment

New Flag Proposals

I might have the glob patterns wrong and they can be adjusted, just trying to get the ideas down here. Some might not be needed once the implementation details are hammered out, but these are what it seems like would be necessary:

Flag	Description	Type	Default
electron	Tell mochapack to use electron-mocha instead of regular mocha	boolean	false
electron-main	Tell electron-mocha which spec files to run in the main process	string (glob)	**/{src,test,spec}/main/**/*.{test,spec}.{j,t}s
electron-renderer	Tell electron-mocha which spec files to run in the renderer process	string (glob)	**/{src,test,spec}/renderer/**/*.{test,spec}.{j,t}s
electron-both	Tell electron-mocha which spec files to run in the renderer process	string (glob)	undefined (Default behavior is to not run any specs in both processses)
electron-entry-main	Tell mochapack where the entry point is for the main bundle	string	src/main/index.{j,t}s
electron-entry-renderer	Tell mochapack where the entry point is for the renderer bundle	string	src/renderer/index.{j,t}s

[larixer]

@Jack-Barry Yep, makes sense to me. Though maybe boolean electron option might be excluded from this list, because user will have to specify one of other electron-... options anyway and this might be used as an indicator that user actually wants to run electron tests.

[Jack-Barry]

After sitting down with a pen and paper, and really thinking into this before writing any specific code for it - I think @JamesMcMahon was on to something before with the concept of something like a custom Mocha initializer to run. I'm calling it byom for Bring Your Own Mocha.

Basically, have a byom option that allows a user to specify a file where a custom Mocha initializer function can be imported from. Mochapack would provide a type to use to ensure the user understands what their initializer function needs to return. I don't know if it's a viable solution to invoke a CLI command instead, but I do think that if a customized instance of Mocha that has a run method is provided that behaves predictably, it would do the trick.

The big road block at the moment is that Mochapack is re-implementing quite a bit of what Mocha already does. Still waiting on this PR to at least help with that.

I think if we can implement a byom solution, I could just implement a separate library such as electron-mochapack that would gracefully handle the usage of electron-mocha, that way mochapack doesn't have to have a bunch of custom code to maintain that's aimed at a niche use case. Mochapack gets a new feature, that new feature makes a new library possible that handles the special case without Mochapack having to maintain it.

Here's a high level overview of how it might work:

This would, of course involve a big change to some of Mochapack's internals, but I believe it could be accomplished without changing anything about how users already interface with it as intended. It almost feels like a 2.0 but it shouldn't be a breaking change, just kinda chucking out what Mochapack shouldn't be re-implementing and adding the bonus of byom.

[Jack-Barry]

@larixer and @JamesMcMahon I'm getting into the argument parsing aspect of this, and there's a couple of scenarios I want to get some second/third opinions on.

With the new feature, a user can pass arguments for

1. Mocha
2. Mochapack
3. Their custom Mocha initializer

The problem I'm running up against is how to handle the files. For example:

mochapack --clear-terminal --bail ./my-test-file.js --byom my-mocha.js --unknown

Is pretty straightforward to assign the options as:

{
  byomOptions: {
    path: 'my-mocha.js',
    args: ['--unknown']
  },
  clearTerminal: true,
  files: ['./my-test-file.js'],
  mochaOptions: {
    bail: true
  }
}

But what happens in cases where the unknown flags are placed before the files? Like this:

mochapack --clear-terminal --bail --byom my-mocha.js --unknown ./my-test-file.js

The resulting object might end up being:

{
  byomOptions: {
    path: 'my-mocha.js',
    args: ['--unknown', './my-test-file.js']
  },
  clearTerminal: true,
  files: ['./test'], // default value
  mochaOptions: {
    bail: true
  }
}

There are a few ways to account for this, I'm just not sure which should be used:

1. Enforce order of arguments so that unknown arguments are always after everything else
2. Add a --files flag to allow specifying which parts of the script refer to files
3. Enforce usage of a --byom-option flag so that no arguments are actually unknown

Obviously each approach has its own pain points. Option 1 requires remembering to order arguments a specific way, option 2 rubs up against typical Mocha CLI usage, and option 3 adds more length to the test script.

I'm inclined to think option 3 is the better of them since it is more explicit. There could also be a secondary flag --byom-config to provide byom options from a JSON file which would minimize the script length if needed.

Please let me know if you see a glaring issue with option 3. I'm going to write up the tests around it for now, but can shift course if needed. Thanks!

[Jack-Barry]

Implemented a new parseArgv function that is passing all tests ✅ Just need to throw the Mocha options into the mix once that PR is ready to go.

Basically I'm building it out in a byom subfolder until it's all green and production ready.

I think it's a little easier to follow what it's doing, tried to break it up into smaller, more digestible chunks that can be more easily maintained.

One headache that arose is that for some reason nyc is acting up and not providing coverage reports for the byom stuff, so I'll need to address that but after I get some sleep 💤

[larixer]

@Jack-Barry Thanks! The code looks very clean!

[Jack-Barry]

Mostly sharing to ensure there's no double work done toward #34 or #9. The changes here directly relate to those.

A little more progress forward: split Mocha options into their own file which at the moment has parity with 6.2.2 until it can be replaced with a direct import.

As far as I know, the user's peerDependency is what will be used at runtime to pull in things imported from mocha/path/to/whatever, is that correct? If that is, then this solution will always have the user's Mocha's options available in their entirety once that PR on their side goes through.

Also - parsing the Mocha config can probably also just be handed off to loadConfig from Mocha's lib/cli/config.js which is already exported, so that's one more thing off our plate 🍽

[larixer]

@Jack-Barry Yep, user's peerDependency will be used as at runtime to get mocha instance.

[jpspringall]

Mostly sharing to ensure there's no double work done toward #34 or #9. The changes here directly relate to those.
A little more progress forward: split Mocha options into their own file which at the moment has parity with 6.2.2 until it can be replaced with a direct import.
As far as I know, the user's peerDependency is what will be used at runtime to pull in things imported from mocha/path/to/whatever, is that correct? If that is, then this solution will always have the user's Mocha's options available in their entirety once that PR on their side goes through.
Also - parsing the Mocha config can probably also just be handed off to loadConfig from Mocha's lib/cli/config.js which is already exported, so that's one more thing off our plate 🍽

Hi @Jack-Barry, are you saying you will cover #9 as part of this PR?

I was going to start looking at it, but looks like you are already doing so?

[Jack-Barry]

@jpspringall Yeah, it should be covered here. I've had to focus on some year-end wrap ups at work, but will be jumping back on this in the next week or two

[Jack-Barry]

Finally caught up on things, jumping back on this throughout the rest of the month 🔨

[Jack-Barry]

Relevant to #9

Made some solid progress on this here: buildMochapackOptions/index.ts

This method uses Mocha's configuration file and opts file parsers, so it can handle any file extension that Mocha supports. Anything in the config file that is not specific to Mochapack gets passed along as part of the Mocha configuration to be used when initializing Mocha.

Still waiting on this for CLI argument parsing, but at least for now it's a lot easier to establish parity since changes will only need to be made in mochaOptions.ts

Next steps:

• Will be taking a look at the Webpack code to see if there's anything we can use from Webpack to further reduce the burden on Mochapack where it's re-implementing existing functionality
• Rework the configuration of/running of Mocha within the context of Mochapack

[Jack-Barry]

Getting a chance to start looking at this in off-hours again and hoping optimistically to have it tackled in April or May, just making some investigatory notes for now (so I don't forget where I'm at on this when I come back to it 😝).

Looks like a lot of what is going on in src/cli/requireWebpackConfig.ts to pull in the config file is handled in Webpack CLI's lib/groups/ConfigGroup.js. Luckily, the ConfigGroup class is exported from that file, so I'm going to play around with it a little. It looks like in the definition of GroupHelper that it extends, the args referred to here are provided as an array, so the config would be provided as the options when initializing the instance of ConfigGroup, then calling the run method should do the trick to provide the config.

Once the config for Mocha and the config for Webpack is being handled as much as possible as it should be by their code, we arrive at the meat n' taters of what Mochapack is responsible for. I think it's mostly a matter of

• Utilizing Webpack's Compiler class to do Webpack's evil bidding
• Rework src/runner/configureMocha.ts to allow for a custom class that extends Mocha. Using the Mocha constructor, we can pass the configuration options to it directly instead of calling a bunch of methods on the instance later which should make life a lot easier

I've still got some digging around in src/webpack to do but hopefully this reconnaissance will prove fruitful.

[Jack-Barry]

Probably won't be able to get around to this for a while, but some notes after completing #63 :

This will be pretty straightforward. To simplify it, I'm only going to add a flag for --byom to point to a Mocha constructor function/factory. That function can do the work of determining any settings specific to itself, rather than having to keep track of more CLI options in mochapack. If mochapack picks up a value for byom then it just uses that module to initiate an instance of Mocha in the initMocha method or somewhere around that.