mochajs / mocha

☕️ simple, flexible, fun javascript test framework for node.js & the browser
https://mochajs.org
MIT License
22.53k stars 3k forks source link

🚀 Feature: opt-out of globals #956

Open domenic opened 11 years ago

domenic commented 11 years ago

@isaacs is complaining that Mocha tests aren't node-able. I think this is a silly complaint, but I think he represents a vocal minority, so shutting them up would be possibly valuable.

Here is what I envision: instead of being able to use e.g. describe, it, etc. by default, you'd do

var mochaBDD = require("mocha/bdd");
var describe = mochaBDD.describe;
var it = mochaBDD.it;

// ok, stupid boilerplate is over but at least I don't have to use a test runner, woohoo!

Alternately it could be just

require("mocha/bdd");

and that would set up some globals for you.

What do you think?

isaacs commented 11 years ago

Even if it just dumped its crap all over the global space when you require('mocha'), would be better than the current state of things.

tj commented 11 years ago

I don't think it's silly but there are tradeoffs with everything. I'd be happy with something like this:

var Mocha = require('mocha');
var mocha = new Mocha(options);
mocha.describe('blah blah', function....

no one would use it but at least it would be a cleaner way to implement what we currently have. There would be a ton of boilerplate that everyone would have to set up each time, but if we could narrow those down to CLI-ish APIs that would be ok. Even if there was lib/cli.js that just passed in the ARGV, but I still doubt anyone would use it, you can use it without the CLI reasonably easy, but that illustrates that no one really wants to beyond some edge cases.

domenic commented 11 years ago

@visionmedia that seems pretty nice. The reason I suggested require("mocha/bdd") or similar is that it would be pretty easy to implement in terms of existing Mocha, but yeah, yours is probably better. (You could envision using it to e.g. run multiple test suites at once or something. Well, that would probably break because of the process.on('uncaughtException') usage, but you see what I mean.)

I may try a pull request for this one day.

park9140 commented 11 years ago

This is a great example where a little future JavaScript via destructuring assignment goes a long way. https://developer.mozilla.org/en-US/docs/Web/JavaScript/New_in_JavaScript/1.7#Destructuring_assignment_%28Merge_into_own_page.2Fsection%29

var [describe,it,beforeEach,afterEach] = new require("mocha")(options);

Just wish there was node harmony support for this.

glenjamin commented 10 years ago

Is the idea of this to be able to do

node test/a-mocha-test.js

And have it run that test?

tj commented 10 years ago

@glenjamin yup

glenjamin commented 10 years ago

Having tried to do this myself on nodespec, the problem wasn't really making the various functions available, the problem was figuring out how/when to begin executing - we'd still need a two-pass define/run approach.

The options I came up with were: 1) have every user add something like mocha.exec() to the bottom of every file they might want to run in isolation 2) Wait for core to add something like process.on('exit'), but when the event loop is still available 3) assume each file has only one top-level describe block, and begin execution when that finishes

(1) is probably the nicest, which could look like this:

var run = require('mocha/required');

describe('blah blah' , function() {
// ..
});

run();

This doesn't seem to add much over doing node ./node_modules/.bin/_mocha test/a-mocha-test.js when you really need to run without the mocha wrapper.

jbnicolai commented 9 years ago

I think this no longer fits with how mocha currently is used. As this thread has been inactive for over a year, I'm closing it for now.

If anyone is still interested in this, feel free to comment or propose a pull-request and we'll consider :)

jfirebaugh commented 8 years ago

Could this be reopened? I'm still very much interested in it. Not being able to just node test.js mocha tests was one of the main reasons me and my colleagues at Mapbox have been moving away from mocha to harnesses like tape and tap. Personally I'd prefer to stick with mocha but do find that "node-ability" argument somewhat compelling.

Edit: the "node-ability" argument was elaborated by @tmcw here.

jfirebaugh commented 8 years ago

For clarity, the lack of a requireable interface is not the primary blocker here from a user perspective. The following is already documented to work:

var describe = require('mocha').describe;
var before = require('mocha').before;
var it = require('mocha').it;

And it gets nicer with ES6:

import { describe, before, it } from 'mocha';

The issue is the fact that this only works when running via the mocha binary.

danielstjules commented 8 years ago

Oh, thanks for the clarification. :) If we already offer this ability when ran using the mocha binary, I agree it'd be nice to do the same with node. Will look into it. Thanks!

glenjamin commented 8 years ago

Now that node has a beforeExit hook, this seems fairly doable.

Would need to be something like

var { describe, it } = require('mocha/auto')
boneskull commented 8 years ago

Yeah. As TJ said, the problem is even if we were to make everything "node-able", it may require too much boilerplate to be useful.

However...

@glenjamin thanks for the heads-up on beforeExit. I think we could leverage this. People would probably still complain it's too much "magic"...

boneskull commented 8 years ago

I've added a proof-of-concept to 3cdd4a04c48193cceac6af7db72af06d380014a9

boneskull commented 8 years ago

anyway, it would need a bit of work, but I think we can easily generalize it for all interfaces. there is no support here for Growl, mocha.opts or various other process-related stuff like exit codes. We should extract some code from bin/_mocha and reuse it here. Then we could have argument support as well. For example:

$ node test/my-test.js --reporter=dot

If anyone has any suggestions, lemme know

glenjamin commented 8 years ago

I think extracting some of the stuff in bin/_mocha into some sort of CLI would be pretty sensible regardless.

I'm still not sure this is really a useful addition though?

Is there much of a practical difference between any of the following?

node test/test.js
./node_modules/.bin/mocha test/test.js
PATH=./node_modules/.bin:$PATH mocha test/test.js
npm test -- test/test.js

I get the feeling that people who aren't using mocha because they can't run test files via node are simply people who don't like mocha, and are looking for an excuse! Mocha doesn't have to be for everyone :smile:

screendriver commented 8 years ago

Same here. I am using jspm and SystemJS and can't import Mocha in my tests that are running at the browser.

import { describe, before, it } from 'mocha';

Can't be used when used in browsers.

boneskull commented 8 years ago

@glenjamin it kind of just needs to happen. Mocha should have a core programmatic API. A nice side-effect of that is node-able tests. The environment in which it runs will consume the programmatic API, whether that's a browser, CLI, or something else entirely. Mocha.app, anyone? :)

kuraga commented 8 years ago

Interested of this feature!

jadbox commented 8 years ago

+1 for: import { describe, before, it } from 'mocha';

stevenvachon commented 6 years ago

We currently have const {describe, it} = require('mocha'), but it isn't necessary because the globals already exist.

If we're concerned with browser support, we could always do const {describe, it} = window.mocha. We already have to do this for chai.

boneskull commented 6 years ago

well, const {describe, it} = window too, I suppose.

this is so fundamental to mocha's architecture that mocha exports the global object when bundled.

stevenvachon commented 6 years ago

I don't think that mocha should define any global other than a single namespace, and maybe only for browsers.

boneskull commented 6 years ago

the mocha executable will continue to use globals; that will likely never change.

what we can (and should) shoot for is the ability to easily run mocha tests via node without polluting the global namespace. that’s what this issue is about.

unfortunately, that’s a Texas-sized ball of twine nobody has had the time or energy to unravel, which should be apparent by the age of this issue...

stevenvachon commented 6 years ago

the mocha executable will continue to use globals; that will likely never change.

While I know nothing about how mocha is structured, I can assert that this single design choice is the source of the problem.

boneskull commented 6 years ago

likely so!

ScottFreeCode commented 6 years ago

I actually took a second look at the code, figured out (more or less) what I'd done wrong last time I'd tried to hack around having globals, thought about what it would take to get test files runnable with node test/someFile.js, and believe I know how to do both of them -- with the caveat that doing so without tripling the number of edge cases is likely to require breaking some existing edge cases. In more detail, my thoughts on the design are:

Thoughts?

ScottFreeCode commented 6 years ago

Actually, on further thought, I motion to close this ticket, on the following rationale against using node <test file> in place of mocha <test file>:

Should we discover any significant advantage that is easier to achieve in core than outside of it, we can always reopen.

However, I also motion to reopen the avoiding-globals ticket, because:

boneskull commented 6 years ago

so you’re suggesting rather that Mocha provides a way to run tests with mocha but behind a flag that won’t pollute global?

boneskull commented 6 years ago

part of what makes mocha mocha and part of its success is that it doesn’t require boilerplate to import stuff. despite the dogma around polluting the global namespace there are completely valid reasons to do so for developer ergonomics.

ScottFreeCode commented 6 years ago

so you’re suggesting rather that Mocha provides a way to run tests with mocha but behind a flag that won’t pollute global ?

Yup.

part of what makes mocha mocha and part of its success is that it doesn’t require boilerplate to import stuff. despite the dogma around polluting the global namespace there are completely valid reasons to do so for developer ergonomics.

I'm not saying I disagree in general, but as an opt-in feature, I can see where some developers may want imports instead of globals (maybe they want their module to name a local variable context without accidentally referring to the global if they forget var, maybe they'd like to use a third-party interface where that sort of collision is even more likely), and now that I've figured out how to do it right it's also easier to do in Mocha than as an add-on of some sort (in fact, if I don't get sucked into something else, I'll probably throw together a rough draft in a branch this week to demonstrate).

(Plus we're already maintaining a kinda-sorta attempt, where after the interface is set up on global it's copied onto the Mocha object so you can require("mocha").it, but the version we're already maintaining frankly is weird, inconsistent, bug-prone and doesn't even actually avoid polluting the global namespace. If we're going to keep that sort of feature at all, we may as well correct it rather than maintaining a dubious version.)

This is IMO in contrast to being able to node <test file> for Mocha tests, which as far as I'm aware doesn't have any advantage and would be equally hard to implement inside or outside Mocha.

zapphyre commented 6 years ago

@stevenvachon how come is that const {describe, it} = require('mocha') is not working for me? I just got undefineds...

$ node -v
v8.4.0
stevenvachon commented 6 years ago

@zapphyre you still need to run the test suite via the mocha program, not via node.

boneskull commented 6 years ago

You can't run multiple source files with node, so you're stuck with mocha. I don't know of a test runner/framework that doesn't have its own executable.

Updated the issue title to be more specific

lll000111 commented 5 years ago

Another reason for this: I would like to run tests using a different JS runtime, for example low.js (because it's one of our platforms, others are node.js, browser - and React Native). I can do that when I have a JS file that imports mocha and runs the tests, I can't do that when I have to call mocha to run tests.

I mean, you already do it for the browser. On that platform I have JS code that imports "mocha" and then runs the tests from a global mocha object using mocha.run().

In the browser I

And that's it. When I try the same in node.js, using a require statement instead of SystemJS to load a test file, I get (using mocha 5.2.0)

> mocha.run();
TypeError: Cannot read property 'search' of undefined
    at Mocha.mocha.run (/home/mha/one/node_modules/mocha/mocha.js:149:54)

EDIT: I tried https://github.com/mochajs/mocha/wiki/Using-Mocha-programmatically and mocha.addFile instead of just loading it, same result.

protometa commented 5 years ago

Is there currently any way to import describe/it?

lll000111 commented 5 years ago

There is a new (as of now 20 hour old) Wiki page https://github.com/mochajs/mocha/wiki/Using-mocha-programmatically but that is for running the tests.

glenjamin commented 5 years ago

Another reason for this: I would like to run tests using a different JS runtime, for example low.js (because it's one of our platforms, others are node.js, browser - and React Native). I can do that when I have a JS file that imports mocha and runs the tests, I can't do that when I have to call mocha to run tests.

If the alternative runtime has a node-like executable (which I think low.js does?) then you can do alternative-executable ./node_modules/.bin/_mocha, and that should work

christianbundy commented 4 years ago

I don't know of a test runner/framework that doesn't have its own executable.

https://github.com/tapjs/node-tap/#tutti-i-gusti-sono-gusti

ggrossetie commented 4 years ago

Now that https://github.com/mochajs/mocha/issues/3006 is resolved it would be nice to have the same named exports in the browser.

Currently, the following is working:

<script type="module">
  import './node_modules/mocha/mocha.js'
  console.log(mocha)
</script>

But the following is not:

<script type="module">
  import { mocha } from './node_modules/mocha/mocha.js'
  console.log(mocha)
</script>

(as requested by https://github.com/mochajs/mocha/issues/956#issuecomment-167453800)

Munter commented 4 years ago

This might become easier when we merge our branch with roll-up and get transpilers set up. It seems feasible to have an esm bundle and a legacy bundle that consumes the esm code paths and adds the global leaks. We'd have to take a lot of care to only make changes in newly added code paths and bundles so we wouldn't break or code both in browsers and node

wclr commented 4 years ago

With global vars and typings this just makes mocha legacy thing.

wclr commented 4 years ago

Btw just created @types/mocha without globals: https://github.com/whitecolor/mocha-types

Hope they (globals) just will be removed officially soon.

danon commented 9 months ago

You can't run multiple source files with node, so you're stuck with mocha. I don't know of a test runner/framework that doesn't have its own executable.

Updated the issue title to be more specific

If you use ts-register, it actually doesn't provide globals, so for that case, the types are broken.

lo1tuma commented 1 week ago

As requested in #2210 I would like to suggest the proposed solution as a way to opt-out of globals.

[...] I'd like to suggest that you consider making it possible to do mocha --ui require to signal that you want the require interface -- meaning that you don't want globals defined.

Right now only one of the following interfaces can be defined via the --ui option: bdd (default), exports, qunit or tdd. So even if the require interface is used and e.g. describe is imported via import { describe } from 'mocha'; a different interface than require has to be used in addition which most probably exposes globals (not sure about the exports interface though). That means importing a mocha function just shadow the global variable.