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

mocha: the next big thing #1969

Closed
boneskull opened this issue Nov 18, 2015 · 76 comments
Closed

mocha: the next big thing #1969

boneskull opened this issue Nov 18, 2015 · 76 comments
Labels
future needs to wait status: waiting for author waiting on response from OP - more information needed type: feature enhancement proposal

Comments

@boneskull
Copy link
Contributor

(..or maybe Mocha 4. I don't know.)

@mochajs/mocha + everyone,

I mentioned this in the mochajs/maintainers room on Gitter, but since it appears people aren't using Gitter much, I'll repeat it here:

Pitch

Mocha's old. What's cool about that is that we know what's wrong with it. And indeed, it has problems which make certain issues difficult to address. The major issue is "plugins". Others include (but are not limited to):

  • The whole thing is written in ES5
  • The core is bloated (modularize it)
  • Test files are not node-able
  • No parallelism
  • Lack of automated browser tests
  • Inability to leverage domains
  • Patchwork "diff" support
  • Too many hand-rolled shims/polyfills
  • Too much hand-rolled stuff in general which should be replaced by 3p libs that do it better
  • "Configuration" is tightly coupled with the CLI
  • Programmatic usage is undefined
  • Multiple executables = pain
  • Various instances of callback hell
  • Poor support for writing 3rd-party reporters and interfaces

It's my opinion that any attempt to address these problems in an iterative fashion is a fool's errand. Components are too tightly coupled; each item above, if taken in the context of the current codebase, is a major undertaking. I propose we rewrite Mocha from scratch.

Plan

Mocha should made of plugins--all the way down. It should come with a default interface and a default reporter, but little else--Mocha's business is running tests and reporting the output. This is what it does well, and this is what the core should be.

From the current version of Mocha, we'd retain:

  • The "simple, flexible, fun" motto. Mocha should remain unopinionated; its core should be simple. We would split some portion of the existing functionality into plugins (separate modules) which can be easily included if necessary.
  • The interface APIs. Users should not have to modify their tests.
  • Support for "deprecated" APIs would move to plugins or separate modules. For example:
    • I'd like to move to using JSON/YAML/JS .mocharc file(s) instead of mocha.opts. We would retain the mocha.opts functionality in a plugin; package mocha-opts-plugin, for example.
    • --watch, which should be handled by a plugin or another executable entirely.
    • Command-line flags to node should be handled by node itself--either by executing mocha with node, or using node-able tests.
    • The CLI itself should move to a package mocha-cli, as not everyone uses it.
  • Some portion of the E2E tests should be retained to guide adherence to the API.

For libraries and tools which consume Mocha (I'm thinking stuff like JetBrains Mocha reporter, Wallaby.js, mochify, karma-mocha, any Grunt/Gulp plugin which executes _mocha), we must keep lines of communication open to ensure a smooth transition. Ideally, these tools should use the resulting programmatic interface instead of forking processes!

We should supply a Yeoman generator for Mocha plugins, which would provide starting points and boilerplate for Browserify (since certain plugins will need to run in a browser).

Conclusion

If you guys are buying what I'm selling, I think the best thing to do is just start coding. The direction is clear, and the requirements are known. Let's create a prototype and take it from there. When the dust settles, we can address specific areas of concern, and begin to deprecate whatever needs deprecating. And start documenting. An upgrade to v3 shouldn't require the user to do much more than install an extra package or two.

cc @segrey @mantoni @ArtemGovorov @vojtajina @maksimr @dignifiedquire

@danielstjules
Copy link
Contributor

The interface APIs. Users should not have to modify their tests.

Is this a hard requirement for a new major release? For example, would you consider dropping support of this and playing with context. It would be awesome if instead of:

it('foo', function(done) {
  this.timeout(10000);
});

we could do:

it('foo', (test) => {
  test.timeout(10000);
});

@dignifiedquire
Copy link

That's a good question, I would be very much in favor to transition to something without a globals in the default setup, so the usage would be something like this

const {describe, it, timeout} = require('mocha')

describe('my test', () => {
  it('my slow test', () => {
    timeout(10000)
  })
})

and for backwards compat it would be easy to add something like this

const mocha = require('mocha')

global.describe = mocha.describe
global.it = mocha.it

@ArtemGovorov
Copy link
Contributor

@dignifiedquire What's wrong with globals by default? And for tests in general? Why bloating every node test file with const {describe, it, timeout} = require('mocha') and making it even harder for non-CommonJs based browser tests?

@dasilvacontin
Copy link
Contributor

@ArtemGovorov : making it even harder for non-CommonJs based browser tests?

I was imagining backwards compatibility via config or executing something like mocha.deployGlobals() / mocha.deployInterface(window).

@danielstjules: dropping support of this and playing with context

I like this change a lot. Doing test.timeout(5), test.skip(), test.done(), is more readable and straightforward to me, than fiddling with the context. And you can easily extend functionality via adding more methods/props to that test object.

@danielstjules: Is this a hard requirement for a new major release?

No, but I imagine we are likely to change quite a lot along the way, and old hacks might no longer work (or maybe we don't want them to work anymore), like coverage via _mocha. I wouldn't like to limit the changes just because I'd prefer not to bump the major version.

@dignifiedquire
Copy link

@ArtemGovorov yes something like @dasilvacontin suggest for the non-commonjs based environments. Simply namespacing everything under one global mocha in that case, so you would have

const {describe, it} = window.mocha
// and as a convenience method
window.mocha.deployGlobals = () => {
  window.describe = window.mocha.describe
  window.it = window.mocha.it
}

@ArtemGovorov
Copy link
Contributor

@dasilvacontin @dignifiedquire I understand there could be a function to set globals. I don't understand why making no globals the default option. Those who'd like to use globals can do it now without doing anything. Those who want to require: const {describe, it} = require('mocha') can do it and will have to do it anyway. So why changing the status quo?

@dignifiedquire
Copy link

Because you are forced to have globals in the namespace with the current version, instead of being able to opt-in the global name space is already polluted as soon as mocha starts, even if I use require('mocha') to assign them. The change I suggest would make it a real choice between using globals or not.

@ArtemGovorov
Copy link
Contributor

@dignifiedquire I don't see anything bad in having a couple of globals for test environment. Pretty sure many people don't mind it as well. Perhaps we could consider an option to remove globals for those who're strongly against them:

window.mocha.undeployGlobals = () => {
  delete global.describe;
  delete global.it
}

From my observations there're way more people using globals for mocha than those who are not using them.

My point is to try making it easier for the majority of existing users to migrate to the new version by trying not to change configuration defaults without a good reason.

@segrey
Copy link
Contributor

segrey commented Dec 1, 2015

It could be an option in .mocharc:

{
  "interface": {
    "bdd": true
  }
}
// if "bdd": true, globals will be deployed

@dasilvacontin
Copy link
Contributor

From my observations there're way more people using globals for mocha than those who are not using them.

Well, the 'mocha' way has always(?) been using globals, so I would put my bets on that statement too. It's only recently that we finally have browserify support, and not with all the features, I believe.

It's wiser IMO to focus on having node-able tests – you can add 'globals-only' support easily. Whereas if you don't even consider having node-able tests, you are going to have trouble adding them later... which is the situation we found ourselves into. I picture this situation like someone trying to dig out sand from a hole while a sandstorm is going on.

That's why a rewrite is being suggested – hacking on the current codebase won't lead to good code and it will take ages. So it's not very motivating, and motivation for maintainers is very important.

Related: http://www.macwright.org/2014/03/11/tape-is-cool.html

@ArtemGovorov
Copy link
Contributor

It's wiser IMO to focus on having node-able tests

Leaving globals by default doesn't prevent one from writing node-able tests. Given mocha adds the support for node-able tests, one can use const {describe, it} = require('mocha'); and don't even know/care that there are any globals. And those who do worry about global space could use a config option that controls assigning those globals (config will still be applied in node-able tests, right?).

This way node-able tests are supported, the majority using globals will not have to add config options, and only those few who are somehow affected by globals will have to use the option (they must already have something in place anyway if they're affected in the current version - will just have to use the official option instead of whatever workaround they use now). Makes sense?

That's why a rewrite is being suggested

I understand, but the rewrite doesn't necessarily have to change certain previously established contract/defaults suitable for the majority of users.

@segrey
Copy link
Contributor

segrey commented Dec 2, 2015

IMHO, for browser tests it seems reasonable to deploy globals by default, but for server-side node tests, deploying globals breaks CommonJS style making people confused a bit about environment they are developing their tests in.

@segrey
Copy link
Contributor

segrey commented Dec 2, 2015

JFYI: jasmine spec runner for Node.js deploys globals always. Do people expect compatibility between mocha bdd style and jasmine?

@boneskull
Copy link
Contributor Author

@ArtemGovorov I don't see any problem with globals for the test environment, personally, however some people do. I'd like to support those people by allowing Mocha tests to be run via node or by a separate mocha binary. I think this is much like how tape works.

@boneskull
Copy link
Contributor Author

I understand, but the rewrite doesn't necessarily have to change certain previously established contract/defaults suitable for the majority of users.

To be clear, I'm not suggesting eliminating a global describe() or whathaveyou--simply that users should have the option. Tests run with the mocha executable would still have this behavior.

Perhaps that means the cli should still be within Mocha's core instead of a separate package (like mocha-cli). Indeed, this would probably be less jarring than needing users to install both packages.

@boneskull
Copy link
Contributor Author

@ArtemGovorov Please let me know if I haven't addressed your concerns.

@boneskull
Copy link
Contributor Author

It could be an option in .mocharc:

Not so hot on this idea. To recap, the idea is this:

  1. If using mocha test.spec.js to run tests, you get globals (but can disable these with a flag).
  2. If running node test.spec.js, you obviously need to require('mocha') and get whatever you need from it.
  3. A browser context probably should use globals by default, but could be disabled via configuration. Even if Browserify is used and require() is present, the tests may still be written using the globals.

@boneskull
Copy link
Contributor Author

I see the role of the mocha CLI as having three main responsibilities:

  1. Overriding .mocharc settings, which would otherwise have to be overridden in the tests themselves.
  2. Glob support. You can easily node test.spec.js, but you can't node test/**/*.spec.js. Sure, you can use find -exec or something, but it should be easy.
  3. .mocharc generation. Perhaps just dump a .mocharc file in the cwd with the defaults populated, but later maybe leverage inquirer to guide people through it. Similar to npm init, karma init, etc.

I'm thinking node test.spec.js should still search for .mocharc and use it. Does that sound fair?

@boneskull
Copy link
Contributor Author

I do like the idea of a function call to populate the globals.

@boneskull
Copy link
Contributor Author

I would like to eliminate bin/_mocha, so that means node flags would no longer be supported. If you need them, run node --flags path/to/mocha test.spec.js or node --flags test.spec.js. This can trivially be configured in the scripts property of package.json.

This is a breaking change for sure, but also not unreasonable. It's a maintenance headache to have to manage the flags and two binaries, even if we have gotten better about it...

@boneskull
Copy link
Contributor Author

It could be an option in .mocharc:

Not so hot on this idea. To recap, the idea is this:

Actually this isn't a bad idea, but it does mean running tests using node would still have to require('mocha') to make that happen. There's an opportunity for confusion there; I'm curious how we should avoid it.

@ORESoftware
Copy link

there should also be better support for dynamic tests in the "new Mocha"

@ArtemGovorov
Copy link
Contributor

  • If using mocha test.spec.js to run tests, you get globals (but can disable these with a flag).
  • If running node test.spec.js, you obviously need to require('mocha') and get whatever you need from it.
  • A browser context probably should use globals by default, but could be disabled via configuration.

This makes sense to me and addresses my concerns. Every group of users gets the most predictable and reasonable defaults.

Even if Browserify is used and require() is present, the tests may still be written using the globals.

Agreed. From what I'm observing, most of the people prefer globals in tests even with Browserify/Webpack. It's just faster than making Browserify/Webpack to compile your testing framework apart from everything else and easier than having to configure Browserify/Webpack to avoid the compilation.

@boneskull
Copy link
Contributor Author

cc'ing @gotwarlost

@boneskull
Copy link
Contributor Author

@ORESoftware what do you mean by this? If you have a specific use case or expected behavior, that'd be very helpful.

Keep in mind we don't want to break existing tests.

@ArtemGovorov
Copy link
Contributor

So with .mocharc, what's the planned format? YAML sounds a bit alien, from JSON and JS I'd personally prefer JS like:

exports = mocha => {
  mocha.option1();
  mocha.option2();
  ...
};

It provides more flexibility, for example one could define --require code right in the .mocharc to avoid creating a separate file just for a couple lines of code (for example, I often see people having to create some fixture.js file just to initialize chai style).

@boneskull
Copy link
Contributor Author

@danielstjules that's true. It sucks. But I think the worst thing we could do is break thousands of tests which rely on this--major or no.

@boneskull
Copy link
Contributor Author

Hmm. If the interfaces were all plugins in their own packages, we could simply version those.

For mocha 3 we would retain the old BDD API. We could then develop the newer BDD API in unencumbered by Mocha. Users could install it if they wish. At the next major of Mocha it could become the default. Users could always fix their version of the BDD interface and still use the new major of Mocha.

Is this sane?

Cc @danielstjules

@boneskull
Copy link
Contributor Author

Also since I'm probably talking about a fair amount of individual packages, a monorepo might make sense for us.

@sheerun does Bower play nice with monorepos? I'm worried about tags.

@boneskull
Copy link
Contributor Author

@ArtemGovorov I prefer YAML over JSON. A .js file is likely overkill for trivial usages of Mocha.

I'd envision a .js file working pretty much like what you propose. You could call setup functions in Mocha, set global variables--maybe even define global hooks--and return a configuration object if you wish. Kinda like wallaby. :)

@boneskull
Copy link
Contributor Author

@ORESoftware I'm unclear why you're talking about bugs in this issue. Please stop commenting in it unless you have anything helpful to offer.

@boneskull
Copy link
Contributor Author

What about allowing extensions on .mocharc files? That's what ESLint does

Indeed, that's the idea

@dasilvacontin
Copy link
Contributor

Randomization as a plugin using the API, I guess.

@avimar
Copy link

avimar commented Dec 6, 2015

As a comment on the idea of mocha being unopinionated: I think docker's approach has much merit: 'batteries included but removable'
Make it so that mocha does everything it needs to do, but it can be swapped out. That way beginner users have a full setup, but those with more advanced needs can swap in more advanced plugins.

@boneskull
Copy link
Contributor Author

I'm unsure whether or not to push this code to branch v3.0.0 or call it v4.0.0. I'd love it if someone could look at the stuff in the v3.0.0 branch and see if it's important enough to have its own release...

@ORESoftware
Copy link

@boneskull - what did you mean in the OP that Mocha can't leverage domains? are you referring to the domain core module and if so how do you mean that Mocha can't leverage them?

@danielstjules
Copy link
Contributor

domain core module and if so how do you mean that Mocha can't leverage them?

They're not available in the browser, and the domain API is being deprecated. See nodejs/node#66

And something like nodejs/node-v0.x-archive#5243 wouldn't be quite as beneficial as we can't bind contexts to arbitrary functions within a test's function.

@boneskull
Copy link
Contributor Author

What I meant was if your code under test is running in a domain and throws
or emits an error, mocha cannot catch it.

I see domain support as a necessary evil but it doesn't need to be part of
core.

Look at lab for a solution; you basically just listen on process.domain
for errors.
On Tue, Feb 16, 2016 at 22:11 Daniel St. Jules notifications@github.com
wrote:

domain core module and if so how do you mean that Mocha can't leverage
them?

They're not available in the browser, and the domain API is being
deprecated. See nodejs/node#66 nodejs/node#66

And something like nodejs/node-v0.x-archive#5243
nodejs/node-v0.x-archive#5243 wouldn't be
quite as beneficial as we can't bind contexts to arbitrary functions within
a test's function.


Reply to this email directly or view it on GitHub
#1969 (comment).

Christopher Hiller
https://boneskull.com | https://github.com/boneskull

@rugk

This comment has been minimized.

@boneskull
Copy link
Contributor Author

nothing has happened here for quite a long time, so closing this, and we can readdress in another issue or discussion medium

@mochajs mochajs locked as resolved and limited conversation to collaborators Aug 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
future needs to wait status: waiting for author waiting on response from OP - more information needed type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests