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

Expose beforeAll/beforeEach/afterAll/afterEach in require'd files #764

Closed
keithamus opened this issue Mar 6, 2013 · 37 comments
Closed

Expose beforeAll/beforeEach/afterAll/afterEach in require'd files #764

keithamus opened this issue Mar 6, 2013 · 37 comments
Labels
stale this has been inactive for a while... status: waiting for author waiting on response from OP - more information needed

Comments

@keithamus
Copy link
Contributor

Part of the utility of having mocha require in a common .js file (--require) is to be able to set up some common convenience features for ALL tests.

It would be useful to have some hooks similar to, or exactly, the /(after|before)(All|Each)/ hooks.

Use case: Being able to integrate tools like sinon more deeply with mocha, allowing automatic tear-downs of stubs/mocks for each/all steps.

@glenjamin
Copy link
Contributor

I've been running into this lately, I see that #286 tried to fix this but was a bit too magic.

I have a slightly simpler proposal.

  1. Expose the Mocha instance created by _mocha to the require()d files (require('mocha').current perhaps)
  2. Provide a hook similar to pre-require for run that triggers just before the run. Unsure if this should be on the suite or mocha instance.

It'd end up looking something like this

// ./spec-helper.js
require('mocha').current.on('run', function() {
    beforeEach(function() {
        whatever()
    })
})

Any thoughts?

@tj
Copy link
Contributor

tj commented Mar 7, 2014

regular require() works fine

@keithamus
Copy link
Contributor Author

Regular require() does not work fine for these. If you have mocha installed globally (so you can use the mocha command) then it requires in the test suites from the global version of mocha, not the local version of mocha. Of course using require('mocha') will always require the local version of mocha, meaning that for the biggest use-case (running the global version of mocha) you have 2 different instances of mocha - the global one which runs the tests, and the local one which you've attempted to add hooks to.

For proof of how difficult this is to get the actual running version of Mocha, look no further than @domenic's (now deprecated) mocha-as-promised which attempts to override mocha behaviour, but first has to fish inside require's cache to find the real running version of mocha domenic/mocha-as-promised@7714899#L4-L18

However, that is an aside from the main issue, which is that it would be useful for plugin authors to have hooks available for test suites, say for example when each Suite runs.

@searls
Copy link

searls commented Dec 12, 2014

👍 for this. It seems to me that files loaded with --require really ought to have access to the same global environment that tests run under. I'm struggling to get a single spec-helper working in my own test suite for this reason.

@boneskull
Copy link
Contributor

I still don't understand why this is necessary. I set up fixtures in a separate module and require them all the time.

I'm going to need to see a concrete example of how require() is insufficient for writing reusable hooks.

@boneskull boneskull added the status: waiting for author waiting on response from OP - more information needed label Dec 15, 2014
@glenjamin
Copy link
Contributor

My take is that it is a convenience feature that:
a) Ensures you can't forget to load the helper in any test file
b) Saves you a line of setup per test file

It's a common pattern in Ruby's RSpec (which I think mocha is inspried by?) to have a file called spec_helper.rb in the root of the testsuite which performs common setup by adding hooks.

Since I've started writing smaller modules, this hasn't affected me so much - but for larger projects with larger teams I think it provides some benefit.

@keithamus
Copy link
Contributor Author

@boneskull I've detailed my use case as part of the initial issue, but I'm happy to explain again:

I want to use Sinon's Sandbox feature to integrate with Mocha (so I can use this.stub() in a before() step and it automatically cleans it up in the after(). The reason for doing this is that its a pain to do it manually, and becomes a problem with thousands of tests when you find one missing. I've actually created a package for doing just this, sinomocha.

Mocha does not provide a particularly friendly interface for this. Using require('mocha') does not always return the running instance of mocha (e.g. if you have npm i -g mocha and run mocha from your global location, then the global version of mocha runs the tests, while in your code running require('mocha') will fetch the non-running, local instance). Because of this you need some crazy code to manually reach into require's cache and fish out the right copy. And then there's the problem that Mocha doesn't define hooks that we can plug into for each test (unless things have changed here?). So, once gain, more magic is needed to get this to work.

In summary, the sinomocha library I have, which is a pretty reasonable usecase and if Mocha had the right tooling, could be defined in about 20 LOC, is instead a convoluted hack 10 times the size because Mocha makes it difficult to write plugins.

Admittedly I haven't revisited the situation in the 9 months that have passed since I filed this issue - so I don't know how much of what I have said has changed.

@boneskull
Copy link
Contributor

I should have been more specific--I need to see code, because as TJ originally wrote, require() is sufficient:

// my-fixtures.js
module.exports = {
  before: function() {
    this.sandbox = sinon.sandbox.create();
  },
  after: function() {
    this.sandbox.restore();
  }
};
var fixtures = require('./my-fixtures');

describe('foo', function() {
  fixtures.before.call(this);
  fixtures.after.call(this);

  it('should blah', function () {
    // ...
  });
});

If that's too much:

// required.js
var sinon = require('sinon');
global.sandbox = null;
global.myBefore = function () {
  global.sandbox = sinon.sandbox.create();
};
global.myAfter = function() {
  global.sandbox.restore();
};
describe('foo', function () {
  myBefore();
  myAfter();

  it('should blah', function () {
    // ...
  });
});

The above will necessitate --require required.

I still don't get why require('mocha') is necessary unless you want to make this completely transparent to the user--at which point it doesn't belong in Mocha core, because it's specific to a 3p lib.

In summary, the sinomocha library I have, which is a pretty reasonable usecase and if Mocha had the right tooling, could be defined in about 20 LOC, is instead a convoluted hack 10 times the size because Mocha makes it difficult to write plugins.

Agreed. A plugin API is the top priority feature right now.

As far as "convenience" features are concerned, we can't really accept PRs for these at the moment due to lack of resources. The plugin API would make it easier to for users to develop these features themselves.

I'll have to investigate if it's feasible for --require to run in the root suite context, but there's likely a good reason why it doesn't.

@glenjamin
Copy link
Contributor

I think a proper plugin API would enable this to work - assuming a plugin is arbitrary code that can be hooked into the mocha lifecycle.

There's a PR above with a sketch of how this might work API-wise.

@keithamus
Copy link
Contributor Author

Yes, a plugin API would work nicely for this and other use-cases. We could use @boneskull's examples, but I am after a seamless/transparent experience where requiring the plugin and loading it sorts everything, no user intervention required. I like the ideas @glenjamin mentioned in the second comment

@boneskull if you're struggling with a lack of maintainer time, I am happy to help out where I can - either stewarding the issue tracker, or reviewing PRs, as I do for Docco, Chai, and Node-Sass. I use Mocha on a daily basis so can dedicate some time to it. Feel free to email me (work at keithcirkel dot co dot uk) to discuss further.

@rstacruz
Copy link
Contributor

what sort of feedback is still needed here? would love to see this. at the moment, i have to use require('./setup') on top of all my test files.

@pigulla
Copy link

pigulla commented Sep 11, 2015

Actually, there's another situation where this would be really useful.

I'm using Wallaby to run my tests in parallel. For this to work with the integration tests I've to use a separate database per test runner (I simply append the process id to the database name). This is done in --require setup.js file and it works perfectly.

The issue I'm having is: how can I drop that temporary database when the test is complete? I can't use after() in my setup.js because that hook is not exposed there. And I can't use the "root suite before/after" solution proposed here either because I have no control over which test files are executed in which Wallaby runner (I'd need the spec with the global hooks to be run in all Wallaby workers).

@eagleeye
Copy link
Contributor

I still don't understand why this is necessary

I have a lot of use-cases of this feature.

  • Globally start selenium in before and shutdown in after
  • clean database after tests will run
  • start web-services before tests
  • build/compile library before test

@boneskull
Copy link
Contributor

@eagleeye thanks for some good specifics

@jamesplease
Copy link

jamesplease commented Aug 31, 2016

It's a bummer that this isn't supported (easily) by the mocha CLI. I've been spoiled by gulp-mocha, which lets you do...

gulp.src(['path/to/setup/*', 'path/to/unit/**/*'])
  .pipe(...)

Aside from changing the behavior of the -r flag, I prefer the API of Gulp. Although the syntax isn't as nice, I've recreated that behavior using the CLI by changing my glob:

{test/unit/setup/*,test/unit/specs/**/*}

I figured I'd post the option in case anyone else likes it more than the other workarounds suggested above ✌️

@tbranyen
Copy link

tbranyen commented Feb 2, 2017

This is exactly how I solved it as well in my tests. Removed the --require and moved the setup script to be the first test executed. Won't work for everyone's use case, but works fine for mine:

mocha test/client/setup test/client/some-test

@gemyago
Copy link

gemyago commented Mar 16, 2017

The issue with mocha test/setup test/some-test approach is that you have to remember to include the test/setup if you need to run some individual test file (and I do that very often).

I have "test" script in package.json similar to this mocha test/setup.js test/*-spec.js and this is working well. But to run individual tests I have to always keep in mind to run them like this:

mocha test/setup.js test/some-spec.js

I found that adding "test/setup.js" to mocha.opts (as a last thing) is working well, not sure if it's by design, but it works with mocha 3.2.0

@tbranyen
Copy link

@evgeny-myasishchev if you only want to run one test, isn't that what only and grep are for?

@gemyago
Copy link

gemyago commented Mar 16, 2017

@tbranyen Yep, I'm using those options as well. For me it's quite often just faster and more flexible to use mocha test/some-spec.js [--watch --some-opts...]

Also with only there is another issue if you use it with --watch. If you start some specs in a watch mode and "only" and then remove only, then mocha will "see" zero tests so I have to restart it.

@tbranyen
Copy link

@evgeny-myasishchev I hate that issue so much. If I could fix one thing about Mocha, it'd be that watch bug.

@glenjamin
Copy link
Contributor

Looks like someone has nearly done this #2544

@stale stale bot added the stale this has been inactive for a while... label Jul 15, 2017
@stale
Copy link

stale bot commented Jul 15, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot closed this as completed Jul 29, 2017
@emahuni
Copy link

emahuni commented Jul 31, 2017

Ahhhhghhhhh!!! I am new to Mocha and according to the documentation this is supposed to be a cinch. I thought that it should just work in required files. why isn't this working? I have read the comments but still don't get why you guys think that exposing these hooks is bad.

@ScottFreeCode
Copy link
Contributor

For the record, Mocha should run files like this:

  1. --require <file> from mocha.opts without (before|after)(Each)?|describe|it (guaranteed to run)
  2. --require <file> from an NPM script without (before|after)(Each)?|describe|it (guaranteed to run as long as run through that NPM script)
  3. --require <file> from commandline without (before|after)(Each)?|describe|it (whatever you specify on the CLI)
  4. <file> (no --require) from mocha.opts with (before|after)(Each)?|describe|it (guaranteed to run)
  5. <file> (no --require) from an NPM script with (before|after)(Each)?|describe|it (guaranteed to run as long as run through that NPM script)
  6. <file> (no --require) from commandline with (before|after)(Each)?|describe|it (whatever you specify on the CLI)

For example, if you have six files named 1.js through 6.js that need to run from 1 to 6 with the first three not having (before|after)(Each)?|describe|it and the last three having (before|after)(Each)?|describe|it, you can do it with this in mocha.opts:

3.js
--require 1.js

...and this in package.json's scripts:

"test": "mocha 5.js --require 2.js"

...and calling this on the CLI:

npm test -- 6.js --require 3.js

(Note that I've reversed the order of --required files vs. plain test files relative to each other simply to emphasize that both their being --required and the order of mocha.opts then npm script then CLI overrides it; individual file ordering only affects files of the same --required/not---required type specified in the same one of these three places.)

That should provide more than enough flexibility to satisfy all reasonable use cases; as far as I can think of the only possible cases it doesn't cover (of which I'm not sure there is any value) are:

  • running a file without (before|after)(Each)?|describe|it after one with (before|after)(Each)?|describe|it
  • guaranteeing files of a given type (with vs without (before|after)(Each)?|describe|it) are run after those of the same (or the later) type specified on the CLI
  • tooling that wraps Mocha or intercepts Mocha's behavior in a way that interferes with those points of control without providing a mechanism of its own to fully replace the behavior with which it interferes (this is a problem in any such tooling and should be addressed as an issue there)

It might be worth adding a small section to the documentation to highlight this relationship of --require <file> vs. <file> and mocha.opts vs commandline.

@machineghost
Copy link

I'm amazed this is still a problem. It seems to me a pretty common use case to:

A) have a test setup file, which does thing like instantiating a Sinon sandbox

B) want to invoke mocha tests multiple ways; you might run mocha *, or you might run individual test files (some IDEs make this very easy), or you might have a testSuite.js file

C) not want to have to repeat boilerplate such as import testSetup from 'testSetup'; beforeEach(testSetup); in every test file

Given those (IMHO pretty sane) requirements, --require seems like the natural solution. It ensures that no matter how you run your tests the proper setup happens first, without the need to repeat beforeEach(testSetup) in every file.

The only problem is, if you try to use beforeEach in a --require file, it won't exist. Why, I have no idea, but it prevents the above solution from working. You can't even bring in beforeEach manually with import { beforeEach } from 'mocha'; it will be undefined.

It would be really convenient if Mocha just made the globals available to --require files, but until they do I found a workaround elsewhere:

const doTestSetup = () => doWhatever();
setTimeout(() => beforeEach(doTestSetup));

You can also use defer if you're a Lodash fan like myself. By calling beforeEach inside the timeout function it actually will exist by the time that function executes, allowing you to do proper test setup via --require.

Hope this helps others.

@boneskull
Copy link
Contributor

No, this is not still a problem; use —file

@machineghost
Copy link

machineghost commented Apr 19, 2018

I still get: "ReferenceError: afterEach is not defined" when I use --file instead of --require.

Nevermind, I was confused by build tasks. When I run my tests with --file I actually get a window is not defined ... which implies that the rest of my code was loaded before my test setup file (which creates DOM mocks).

I suppose I could do ... --require mockDomSetup.js --file beforeEachSetup.js ... but A) it's sort of lame to need to two files like that, and B) it means I have to repeat code (or introduce yet a third file) because in addition to setting the mock DOM up initially I also like to refresh it between tests.

Is there really no way to run code with the mocha globals exposed before your tests?

@boneskull
Copy link
Contributor

Ensure you've upgraded to a version of Mocha with --file, but here's an example:

// test.spec.js
it('should be ok', function () {});
// setup.js
afterEach(function () {
  console.log('a test happened');
});
$ mocha --file setup.js test.spec.js


  ✓ should be ok
a test happened

  1 passing (6ms)

@machineghost
Copy link

machineghost commented Apr 19, 2018

I just updated Mocha to make sure I wasn't being stupid :)

  • mocha@3.5.3

Here's a real simple example:

A.js

window.foo();

ATest.js

 require('./A');

testSetup.js

global.window = { foo: () => {});
beforeEach(() => {
  global.window = { foo: () => {}); 
});

mocha --file testSetup ATest.js

You should get:

ReferenceError: window is not defined

@boneskull
Copy link
Contributor

Mocha doesn't support import in Node.js (and won't until no longer experimental)

@machineghost
Copy link

machineghost commented Apr 19, 2018

Oh, should have mentioned that I'm using Babel, so it's all getting transpiled down to require statements. I'll update my example to use require instead of import.

@boneskull
Copy link
Contributor

Sorry, didn't see your Mocha version. --file was added in v5.

@machineghost
Copy link

machineghost commented Apr 19, 2018

Ah! I thought doing npm i mocha would get me the latest, but I guess I had something in my package.json blocking it. In any case, I'm now at:

  • mocha@5.1.1

and when I run my example again I get ...

(function (exports, require, module, __filename, __dirname) { window.foo();
^

ReferenceError: window is not defined

...still.

It's just three files, two of which are one-liners; any chance you could try it on your end? If it works for you I'm fully willing to blame user error/my own stupidity.

@boneskull
Copy link
Contributor

what does mocha -v output?

@machineghost
Copy link

machineghost commented Apr 19, 2018

mocha -v
error: unknown option `-v'

mocha --version
3.5.3

WTF! 😳 I just retried npm -i mocha again and again got that "+ mocha@5.1.1" ... but mocha --version still refers to 3.5.3!

Sorry for the confusion; not sure what's going on in my environment but it clearly doesn't appear to be Mocha's problem. Thanks a bunch for all of the help.

P.S. I guess I was using an (outdated) global mocha without realizing it. When I switched to: ./node_modules/mocha/bin/mocha --file testSetup.js ATest.js to use the proper/latest version of Mocha everything runs successfully (so I can confirm that everything ... except my understanding of NPM ... is working as intended). Thanks again.

@boneskull
Copy link
Contributor

It's pretty simple; mocha is installed globally and is version 5.1.1. This is what you get when you use mocha. The mocha that's in your local node_modules is in node_modules/.bin/mocha.

@emahuni
Copy link

emahuni commented Jun 19, 2018

Can you pliz explain:

  1. what co-mocha is doing that's enabling --require to work with global hooks?
  2. if this is bad and why?
  3. why mocha isn't implementing it like this

mocha --require co-mocha test/app.mock.js test/**/*.test.js

there is a before() in app.mock.js and it works well with the above but not with the below.

mocha --require test/app.mock.js test/**/*.test.js

Somehow co-mocha is exposing those hooks to a global scope enabling all requires to work with hooks. What got me back here was actually trying to figure out, how it's done. Actually it was a Google search and it got me back here, was surprised to see I had this same issue about a year ago. You guys should just expose the hooks, give an option to do so or explain this different relationship between --file and --require in the docs.

Please try to explain how mocha runs things, how it works with our tests in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while... status: waiting for author waiting on response from OP - more information needed
Projects
None yet
Development

No branches or pull requests