mocha icon indicating copy to clipboard operation
mocha copied to clipboard

🚀 Feature: Plugin API

Open boneskull opened this issue 10 years ago • 32 comments

We need plugin API(s).

More to follow.

boneskull avatar Nov 30 '14 19:11 boneskull

Currently, we don't have the resources to add many features users want, because we cannot afford to take on more of a maintenance burden.

In order to allow users to add features that are not available in Mocha core, we need a set of well-documented and easy-to-use plugin APIs.

I'm thinking we'll need the following:

  • Automatic detection of installed Mocha plugin(s). By default, all will be loaded; users can load an explicit list with command-line options. Given installed plugins mocha-plugin-foo, mocha-plugin-bar and mocha-plugin-baz:

    $ mocha test/ # run tests in test/ with `mocha-plugin-foo`, `mocha-plugin-bar` and `mocha-plugin-baz`
    $ mocha --plugin foo --plugin bar test/ # run tests in test/ with `mocha-plugin-foo` and `mocha-plugin-bar` plugins
    $ mocha --plugin /path/to/your/plugin.js test/ # run tests in test/ with some custom plugin
    
  • To ensure that we don't receive a bunch of issues coming out of plugins (meaning "bugs in plugins that are reported to Mocha"), each plugin should probably run in its own domain. If a plugin throws an error, we can report the proper contact to the user.

  • Specific APIs:

    • Reporter
      • Reporters will be plugins.
      • Streamline Reporter API; extract all but default and base reporter(s) from core.
      • May necessitate refactoring current reporters.
      • Keep eye on js-reporters for anything we may need to change.
      • May be able to throw in "multiple reporter output" as long as we're in there.
    • Interface
      • Interfaces will be plugins.
      • Extract interfaces except BDD and TDD from core.
      • Refactor, as they are kind of difficult to work with atm. Just give the user some methods to implement. Make sure they don't need to require Mocha internals.
    • General
      • Ensure Mocha's exports expose any bits of the system people would be interested in fiddling with.

      • Decide when to load and initialize these. It should be pretty early. Ideally users will be able to add command-line flags/options this way; we can decouple the options parsing from bin/_mocha and allow users access to an object which they can extend.

      • Identify places where emitting an event would be handy.

      • Implement a system that allows any of core modules (mostly stuff in lib/ proper) to be swapped out easily. Perhaps something like this:

        var modules;
        
        var broker = function broker(name) {
          return (modules && modules[name]) || broker.defaults[name]();
        };
        
        broker.defaults = {
          runner: function() { 
            return require('./lib/runner'); 
          },
          test: function() { 
            return require('./lib/test') ;
          }
        };
        
        broker.init = function init(opts) {
          modules = opts;
          return broker;
        });
        
        module.exports = broker;
        

        A plugin could export an object somewhere, which, if present, would be read by Mocha:

        var modules = {
          runner: require('./foo-runner.js') // optionally support a path string
        };
        

        Mocha would initialize the "broker" with it (calling require('broker').init(modules)), replacing any core libs with those the object specifies. To get a current core module from anywhere, Mocha would require('broker')('runner') which would either give them the default Runner or a custom module. If it needs its default, the broker.defaults object is available.

      • Allow interplugin communication. Since only one Runner or Interface, can be used at once (for example), plugins can leverage the "broker" themselves. Reporters, if/when multiple reporter support is introduced, should be able to get a mapping of all active reporters and access any endpoints within them, if appropriate.

Ideas/comments/suggestions? I'll propose some more specifics later.


boneskull avatar Nov 30 '14 21:11 boneskull

Extracting any reporters or interfaces from the core will cause a major bump

boneskull avatar Nov 30 '14 21:11 boneskull

It would be great if this plugin architecture supported adding global before/after hooks!

oveddan avatar Dec 09 '14 15:12 oveddan

Some thoughts on the above proposal:

  • While possibly cool, auto-detection of plugins isn't very node-like. Using --plugin <X> and requiring the user to list them in mocha.opts will be far easier to implement and work basically the same way.
  • Perhaps combine with a --disable-plugin <X> to allow overriding defaults on the CLI
  • I think the module signature should just be a single export function, which receives the mocha plugin API as an argument. This is pretty straightforward and provides a simple way to expose additional functionality.
// my-cool-plugin.js
module.exports = function(mocha) {
  mocha.registerReporter();
  mocha.addCliOption();
  // etc
}

I would suggest that core-module-replacement and inter-plugin-communication not be included in an initial release, as they're quite tricky and it would be more important to get the basic mechanisms in first.

A possible initial featureset could be:

  • Custom reporters
  • Custom interfaces
  • Access to the current instance options
  • Access to the Runner instance (can attach events to do stuff)
  • Add extra functions to existing interfaces?

One way to allow plugins to specify extra options but also be parsed from options is to do one pass over argv to get the plugin list, then load them, then do another pass to get the full options list.

glenjamin avatar Jan 30 '15 14:01 glenjamin

@glenjamin Thanks for the input. I'm going to fiddle with this a bit.

boneskull avatar Mar 15 '15 03:03 boneskull

First, This is a great idea.

I've always like chai's plugin architecture:

chai.use(require('some-chai-plugin'));

That should not preclude functions like mocha.registerReporter(), etc. But rather those functions should be called by the plugin author, not the end user. This allows plugin authors to hide ugly implementation details from the user (i.e. my super-awesome-reporter may need to install a global beforeEach, afterEach, etc. You don't want to know that. You just want to to usesuper-awesome-reporter).

So spec should be something like this:

  1. user calls mocha.use(require('plugin-module'))

    or optionally mocha.use(require('plugin-module')(opts)).

  2. convention dictates that a plugin module returns a callback.

  3. mocha will validate that callback has never been called before. If it has, silently ignore (mocha should ensure plugins are not added twice - should not be an error).

  4. mocha will call that callback, passing a registration object as the first parameter.

  5. registration object will expose all the allowed "hooks"

jamestalmage avatar Mar 17 '15 06:03 jamestalmage

@jamestalmage use sounds ok, but Chai isn't a command-line executable, so that's pretty much what it needs to do.

Mocha, on the other hand, allows CLI options or a mocha.opts file, so simply putting --plugin mocha-foo-reporter in there might be fine.

What do you think?

Personally, I think mocha.opts is kind of lame and would prefer a proper configuration file in JSON and/or YAML format.

boneskull avatar Mar 17 '15 17:03 boneskull

I wasn't suggesting that you NOT provide a command line. Just suggesting what I thought was a good API. --plugin mocha-foo-reporter from the command line, could just become mocha.use(require('mocha-foo-reporter')); internally.

I'm not so sure about JSON/YAML. They end up being a bit too restrictive. I like how karma handles config:

// karma.conf.js
module.exports = function(karma) {
  karma.set(
    /* ... */
  );
} 

That tiny bit of extra verbosity provides all kinds of power:

karma.set({
    browsers: [process.env.TRAVIS ? 'Firefox' : 'Chrome']
});

This would be super handy for say, skipping certain tests that were part of a PR because they required secure variables on travis (which are not available to PR's for security reasons). Angulars karma configs make heavy use of this power and provide a good example of an ~~overly~~ complex build. The 90% of configs that do not need the extra power offered by this approach just end up looking a lot like JSON inside a module.exports = function wrapper.

jamestalmage avatar Mar 17 '15 17:03 jamestalmage

Fwiw I like that mocha.opts forces all config to also be available on the command line.

glenjamin avatar Mar 17 '15 19:03 glenjamin

@glenjamin I don't think there's anything wrong with making all of core config available on the command line, but I don't think you want to force that on every plugin. Certainly provide a mechanism for plugins to digest command line args.

jamestalmage avatar Mar 17 '15 19:03 jamestalmage

@jamestalmage I'm in disagreement--I understand that as a .js file, it gives you more flexibility. However, my preference is configuration instead of, well, more code. I don't want to ever feel compelled to write unit tests against my config files or build scripts. :stuck_out_tongue_closed_eyes:

I wasn't suggesting that you NOT provide a command line.

Not sure where I gave you this impression..?

Anyway, I'm of the opinion this sort of thing (and likely other collaborators as well):

karma.set({
  browsers: [process.env.TRAVIS ? 'Firefox' : 'Chrome']
});

...should not be handled by Mocha; it wades into "task runner"/Makefile/shell-script territory. Detecting an environment variable and changing its behavior is not something Mocha, nor its configuration, should be in the business of. Instead, a script should run Mocha with different options. This keeps Mocha simple. You'll find a similar argument in many other GH issues.

I'd probably support .json and .yaml config files out-of-the-box. This would add only a single 3p dep. That said, as far as supporting a .js config: if there is some sort of popular need, I'd be behind adding it. I certainly have ideas about what should and should not be in Mocha, but that doesn't mean I'm deaf to public opinion.

mocha.opts just seems pretty silly to me, as its functionality is duplicated--and improved upon--by a simple shell script.

@glenjamin

The config file should allow plugin-specific configuration which may or may not be applicable to the command line; think browser-only reporters.

UPDATE:

No, browser reporters won't have access to the config file, so yeah, I don't envision anything in the config file that wouldn't be available on the command line. :smile:

boneskull avatar Mar 17 '15 19:03 boneskull

@boneskull Yeah, I pretty much anticipated this would be your response, but I'm a masochist, so I'm going to keep arguing :stuck_out_tongue_winking_eye: .

I don't want to ever feel compelled to write unit tests against my config files or build scripts.

If you were ever silly enough to make your build script complicated enough where you did feel so compelled, would you rather write unit tests against a bash script, a Makefile, or a javascript module? IMO, Makefile/shell-script is just more code in a language I (and probably the majority of mocha users) have spent orders of magnitude less time learning.

...should not be handled by Mocha; it wades into "task runner"/Makefile/shell-script territory

To be clear, in my example all karma sees is browsers:['Firefox'] or browsers:['Chrome'], it never sees an complex logic. As I said:

The 90% of configs that do not need the extra power offered by this approach just end up looking a lot like JSON inside a module.exports = function wrapper.

As for keeping mocha simple, I couldn't agree more, but I would think a .js config is pretty simple.

var config = require('config.json');
// vs
var config = require('config.js')(mocha);

jamestalmage avatar Mar 17 '15 20:03 jamestalmage

No, browser reporters won't have access to the config file, so yeah, I don't envision anything in the config file that wouldn't be available on the command line.

Why would you prevent browser reporters from accessing the config file?

jamestalmage avatar Mar 17 '15 20:03 jamestalmage

Why would you prevent browser reporters from accessing the config file.

@jamestalmage Because browser reporters run in the browser and do not have access to the filesystem.

boneskull avatar Mar 17 '15 21:03 boneskull

Yeah, but couldn't you bundle it right alongside all the sources? That's certainly an argument for sticking with JSON.

jamestalmage avatar Mar 17 '15 21:03 jamestalmage

err, nevermind. I'm conflating mochas abilities with karma-mocha (which handles bundling the files up for you)

jamestalmage avatar Mar 17 '15 21:03 jamestalmage

It'd be neat if it were possible to use mocha programmatically without it requiring all the tty/shell related dependencies.

I integrated Mocha in ArangoDB to allow us to execute tests in ArangoDB's JavaScript environment (which is mostly compatible with node modules) but had to shim several dependencies as well as some internals like process.argv just in order to be able to require('mocha').

I think it'd be a good idea to extract the mocha runner and interfaces and have them separate from the CLI (which could then handle features like plugin auto-loading).

It'd also be nice if the interfaces used post-require to clean up after themselves. This would allow re-using the context in which a test suite is run. I had to modify the logic of mocha.loadFiles so it would create a new context object (really just a bunch of variables that get dumped in a closure the module code is executed inside of) for the tests to execute in.

See https://github.com/arangodb/arangodb/blob/2f8e5d/js/server/modules/org/arangodb/foxx/mocha.js

pluma avatar Apr 09 '15 15:04 pluma

@pluma see #1457

boneskull avatar Apr 09 '15 16:04 boneskull

oh wait, this is #1457.

boneskull avatar Apr 09 '15 16:04 boneskull

@pluma Anyway, it's going to be iterative. I'm not sure we can release it iteratively, but the first thing I'm focusing on is providing several common interfaces for reporters of different types.

Currently I have a Base reporter which all other interfaces extend. Then I have (or will have) three more:

  1. Console -- all reporters which work in a TTY should use this interface
  2. Browser -- for reporters which work in the browser
  3. Stream -- for reporters working with other non-TTY streams
  1. may be a specific case of 3), so there's that, but that's kind of what I'm working from. Then, I propose moving every reporter except for a default in each category into its own repository. That means "spec", "html", and either create a new generic "file" reporter or use "json".

Suggestions welcome!

boneskull avatar Apr 09 '15 16:04 boneskull

Feels like the most straight forward place to start out with, and we get to move out the reporters as a bonus. Hopefully I can contribute to this, but my free time is quite limited lately..

dasilvacontin avatar Apr 09 '15 17:04 dasilvacontin

Shiny. If there's anything I can help with, I'm available. I'd love to see mocha become more environment agnostic.

pluma avatar Apr 13 '15 10:04 pluma

+1 for an API for global hooks, this would be really helpful for setting up/tearing down any datastores/caches.

opsb avatar Apr 19 '15 19:04 opsb

@opsb You can't do this?

// top of file
beforeEach(function() {
  // blah
});

describe('stuff', function() {
  // more tests
});

or perhaps not asynchronously? I thought this was mentioned in another ticket, but I don't recall where.

boneskull avatar Apr 26 '15 17:04 boneskull

anyway

mocha.opts (or more precisely, "the configuration file") should be accessible via a programmatic API.

boneskull avatar Apr 26 '15 17:04 boneskull

as mentioned in #1668

boneskull avatar Apr 26 '15 17:04 boneskull

@boneskull I came across that just earlier today, I had no idea!

opsb avatar Apr 26 '15 22:04 opsb

Just stopping in to say I am a huge fan of the idea of exposing the config file (currently mocha.opts) via a programmatic API. Maybe switching it over to JSON (like all of those other rc files out there) would make this easier.

jamesplease avatar Aug 02 '15 19:08 jamesplease

Maybe domictarr/rc could be useful to implementing a configuration file easily

perrin4869 avatar May 24 '16 05:05 perrin4869

Maybe domictarr/rc could be useful to implementing a configuration file easily

Definitely. I forked it to rc-yaml which adds, well, YAML support.

boneskull avatar Jul 02 '16 23:07 boneskull