Make CLI more reusable

[rijnhard]

Hi

In our use case mocha is basically the underlying engine for a behavioural framework we wrote that tests a bunch of async systems we use in-house.

We did a lot, but for brevity we write the behaviours in mocha specs, and use it to assert the systems output during a test case run. This allows us to test that the system is behaving the way that it should since it's basically a giant async state machine constantly spewing out state change notifications.

In our test case we want our framework to work as much as is possible with other mocha tools.
The problem is that using the Runner class you basically lose all of the third party tool support unless you want to reimplement the entire mocha cli on your own.

I have used Commander extensively in many other projects, so I have figured out how to make the CLI more reusable.

More details are in the PR (#2553)

I am willing to make any changes requested myself. And the functionality of the cli has not changed, so if you choose not to expose a CLI component for reuse then this can be looked at as just a code cleanup.

I know I shouldn't have made these changes without speaking to you first, but as the PR says I didn't even know if it would be possible or work, so I dug into it face first and by the time I proved it could be done, I had it working.

FYI I am using my PR in said testing framework and its working well.

[sgen]

I would be a little weary of Commander.js at this point. There are nasty bugs in it right now. The original author seems to be quite busy with other projects and hasn't touched it in quite some time. The other contributors seem quite busy as well with other projects.

[rijnhard]

@sgen I'm aware of tj/commander.js#404 ironically i ran into it with the opts option overriding the Command.opts() function, had to do some magic to get it to work as expected in this PR, I did open an issue in commander (tj/commander.js#584) for that.

But you raise a valid point, the other option is yargs. Which is fairly similar and the maintainers are more active but not as fast in some cases due to their use of read-pkg-up.

[rijnhard]

It probably won't be toooo hard for me to swap one out for the other.

[sgen]

I've never used yargs so I cannot comment.

I fully support migrating the some of the mocha cli logic to the main library. Ideally all tool functionality should be included in the library, with the cli tool being a simple wrapper to decode cli input.

But Im not a Mocha dev so you should wait for them to weigh in.

[sgen]

I should also mention that I am a Mocha user and may need similar functionality in the near future. This gets a huge +1 from me.

[ScottFreeCode]

Thanks for the proposal! And sorry for the delay in reviewing.

I'm one of the juniors around here, so I'll defer to the judgement of the team, but my inclination is to say that we really want the CLI to be doing less so there's little to no need to make it reusable/programmatic like this (and hopefully so it's simpler to implement or reimplement if need be). For instance:

• --watch exists because the programmatic interface isn't entirely well-behaved when running multiple times and therefore can't be safely used with external, dedicated watching tools (while having such tools spawn up a whole new Mocha process each time is inefficient); if we can get the programmatic interface to handle state retention between uses more sanely we should be able to get rid of that and switch to plugging it into a better, separate watcher instead. See Deprecate --watch #1780 for more info.
• mocha.opts we've actually wanted to replace for a while with something less CLI-centric and more programmatically useful, e.g. a mocha.yaml instead, if I've understood correctly. (First instance of discussion I dug up in searching: 🚀 Feature: Plugin API #1457 (comment)) I think, ideally, this would even be two separate pieces, one helper function that loads configuration from a file and converts it into a JS object compatible with Mocha's programmatic interface, and then a programmatic-interface ability to take that object as configuration (actually, it already has some of the latter given the way an options hash can be passed to mocha...).
• The separation between _mocha and mocha is due to the need to pass certain options (e.g. --harmony* flags, --use_strict) back to Node (hence the spawn call in mocha and the way some arguments are pushed onto the re-sorted list and others are unshifted onto the other end, with _mocha in the middle of the new list); as far as I can see (assuming I haven't missed a spawn somewhere in the new code) the PR currently breaks this by removing that Node-option handling from before the spawn in mocha. (I wouldn't be surprised if we're missing tests for some or all of the Node options...) See also Consider using flagged-respawn #2517

My hunch is that if we made Mocha better useable through the programmatic interface, then not only would there be less to code in the CLI, but there'd be little to no need to reuse the CLI code -- all it would be doing is making the same functionality that's available in the programmatic interface callable from the commandline. But maybe I'm not understanding the use-case properly, so a few questions for clarification:

• How do you go about using Mocha with a custom Runner? I thought the Runner itself was the least pluggable part of the whole system, so I'm not sure what you mean your system is doing.
• How does the custom Runner help your use case? Would it be sufficient to be able to use the regular Mocha instance more programmatically without as many problems and edge cases as it currently has?
• How does the CLI code play into this system? Are you needing to use your custom Runner but to call it through the commandline, or are you just trying to use Mocha programmatically but need to leverage some of the behavior that's currently only implemented in the CLI?

[rijnhard]

hi @ScottFreeCode thank you for the in depth reply.

It would be correct that the main reason to have a reusable CLI would be because the Mocha interface itself is not sufficient in handling all the options provided by the CLI.
But not only that, in order to make any kind of framework built ontop of mocha compatible with third party tools we would still need to emulate the CLI interface, and with the amount of third party tools theres a large benefit to that.

To dig into specifics we load variations of mocha specs using a custom bdd interface. That basically scans a load of yaml files with parameters for test cases. (Essentially we built parameterized test cases ontop of mocha.) and then depending on additional CLI flags we added it will run the mocha specs with those specified test case parameters. So there's a fair bit of magic at the moment.

I used the term Runner incorrectly, I did mean the mocha interface, sorry for the confusion (I wrapped it in a class I called Runner that creates the mocha instance, hence the incorrect terminology)

So I take it the first step would be to make the mocha interface capable of handling more of the CLI behaviour, considering I have already dug into the code this is something I would be willing to do.
I will still keep my PR open, and fix the issue with _mocha where I moved the parameter expansion introducing the break, there are definitely missing tests in this regard.

[rijnhard]

@ScottFreeCode

So I've revisited this, an important use case if you were to have a reusable CLI would be being able to use it as a subcommand.
And in trying to do so I ran into a lot of limitations with commander that would require really nasty workarounds. (and thats over and above the already nasty things I have to do for Command.opts().)

In detail thats becasue it always calls the base Command instances Command.parseOptions() function which means my overloading of it won't work without really hairy magic, which is too sensitive to have in a project of this magnitude.

This is looking like its not a good idea with the current limitations in Commander, not to say the functionality wouldn't be wanted.

So to conclude what I would like to do:

• take some of my refactoring and still push it but not expose any of it, and remove all the dodgey things i do with Command.parseOptions() and instead just code that into _mocha. Thus not making it reusable but a code cleanup. This will still make it easier in the future to create the mocha.yml and tear out the mocha.opts handling as you hinted at.
• In a separate PR extend the Mocha interface constructor to have parity with the CLI (I'm sure I can do this quickly)

[rijnhard]

I updated the PR to not expose the cli, and tore out anything that looks like technical debt, also added the changes we discussed.
So now it's basically a refactor.

[deepakpathania-zz]

Hey, in my use case, while using mocha to test the cli app, mocha options are being given precedence over the arguments supplied by the command line leading to name collisions and lost data.
for eg, something like

program
.option('-d, --data <path>', 'Data')
.parse(args)

where args is either process.argv or arguments supplied through mocha for testing the cli app.
Then if I have something like -d, --data <dummyData >, it adds a debug options supplied through mocha and set that to true rather than having a data key.

Is there a way to give precedence to the actual options in case both the actual ones and the ones supplied through mocha are present. Thanks.