Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🚀 Feature: Plugin API #1457

Open
boneskull opened this issue Nov 30, 2014 · 32 comments
Open

🚀 Feature: Plugin API #1457

boneskull opened this issue Nov 30, 2014 · 32 comments
Labels
future needs to wait status: in discussion Let's talk about it! type: feature enhancement proposal

Comments

@boneskull
Copy link
Contributor

We need plugin API(s).

More to follow.

@boneskull boneskull self-assigned this Nov 30, 2014
@boneskull boneskull added type: feature enhancement proposal High priority labels Nov 30, 2014
@boneskull boneskull added this to the v2.1.0 milestone Nov 30, 2014
@boneskull
Copy link
Contributor Author

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 boneskull modified the milestones: v3.0.0, v2.1.0 Nov 30, 2014
@boneskull
Copy link
Contributor Author

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

@oveddan
Copy link
Contributor

oveddan commented Dec 9, 2014

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

@glenjamin
Copy link
Contributor

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.

@boneskull
Copy link
Contributor Author

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

@jamestalmage
Copy link

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"

@boneskull
Copy link
Contributor Author

@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.

@jamestalmage
Copy link

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.

@glenjamin
Copy link
Contributor

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

@jamestalmage
Copy link

@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.

@boneskull
Copy link
Contributor Author

@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. 😝

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. 😄

@jamestalmage
Copy link

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

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
Copy link

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?

@boneskull
Copy link
Contributor Author

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.

@jamestalmage
Copy link

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

@jamestalmage
Copy link

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

@pluma
Copy link

pluma commented Apr 9, 2015

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

@boneskull
Copy link
Contributor Author

@pluma see #1457

@boneskull
Copy link
Contributor Author

oh wait, this is #1457.

@boneskull
Copy link
Contributor Author

@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!

@perrin4869
Copy link
Contributor

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

@boneskull boneskull removed this from the v3.0.0 milestone Jul 2, 2016
@boneskull
Copy link
Contributor Author

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 boneskull added the future needs to wait label Jul 2, 2016
hoverduck referenced this issue Feb 7, 2017
@OmarDarwish
Copy link

Is this issue still being considered?

@JoshuaKGoldberg
Copy link
Member

I've been linking issues to this one that seem relevant. There are ... quite a few 🫠. But given how many use cases folks have asked for Mocha that would be solveable by a plugin API, they're all required reading to submit a proposal for this.

#4116 has some very good notes too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future needs to wait status: in discussion Let's talk about it! type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests