Mocha: after() not called on error

[Almad]

describe 'Test', ->
  after -> console.error 'cleanup is not called'

  describe 'When I do something', ->
    before -> throw new Error 'BAM' # and there is a problem
    it -> assert.ok true

[tj]

js examples next time please :p

I can verify that it does work fine for me for test-cases, but yes if a hook fails mocha will pretty much bail immediately, though I suppose we could change that behaviour for after / before hooks only

[Almad]

OK ;)

It would be nice as I am using after() for cleanup. In integration tests, database is left in inconsistent state, which may then interfere with next test run.

[tj]

well i had this discussion in another issue about trapping signals etc, cleanup in after hooks technically makes little to no sense, but in this case yeah we can definitely get around that

[Almad]

516 I guess, yeah ;)

[wclr]

+1 Yes, It is not very good thing that after hook is not called if suite fails. For example I upload some files to test store to perform there some ops and after tests I want file to be deleted from store. Of course I can design my tests so that it would clean up file store in before hook (and usually i do so), but some times it is more convenient to clean up in after hook.

[mattsgarlata]

+1. Every time my tests fail, I have to change my test data because my cleanup code isn't executed

[tj]

I cleanup in before hooks personally, that way even if you have a fatal error that mocha cant handle you just end up doing it next run

[mattsgarlata]

What if your failure is in the very last test? Then there's not another before hook to execute. I guess you could add an empty test at the end, but that's pretty hacky.

[tj]

then it's cleaned up the next time you run the tests. I'm not saying we shouldn't fix this but i think it's bad practice to use after hooks in any test framework for cleanup really because there's no guarantee the framework itself or something else wont cause a fatal error

[johnaschroeder]

In my REST integration tests, the amount of teardown varies significantly by test, and errors will often leave the database in an unknown state. I have a lot of tests, so to add comprehensive teardown coverage using "before" to every test seems like it would add a lot of overhead. Putting the teardown in "after" would allow me to only add the teardown required for that individual test. I would imagine my code will be causing a lot more errors than the framework(s). in that less frequent scenario where the framework causes a fatal error, I can run a cleanup script. Right now I run that script a lot...

[MCluck90]

Is this issue being addressed? I have a case where I need to backup some files prior to manipulating them then restore the originals afterwards. It would be really beneficial if after would run regardless of any errors.

I'd even be willing to fix it up myself if someone pointed me to the point in the code which causes it to cease running.

[antony]

In some test frameworks there is a 'cleanup' method which can be guaranteed to run before a test, and a 'setup' method which runs before. If the default 'after' behaviour can't be altered to /always/ run regardless of errors, it might be nice to have methods with a similar behaviour to setup/cleanup instead. Trying to ensure a clean state for tests upon each run is quite vital to maintaining a reliable test suite.

Another option might be something like after.always(function() { // always run });

[joe-spanning]

I can't believe this issue is still not addressed. What is the point of after and afterEach if they don't get called when a test fails?

[samshull]

I have a test with a forked process, when there is an error, the forked process is not killed because after() never killed it. How would you suggest I kill the forked process?

[hallas]

we have a lot on our plate folks, we will look into this shortly (within days)

[rweng]

+1, in my case it's closing a webdriver I'd like to do after a test, independent whether it succeeds or fails.

[bradjasper]

I'd also like to see this added.

I have the same problem as @samshull—I have a forked process and it won't get cleaned up on an error. I am currently cleaning it up on before(), but this lets the process leak until the tests are re-run.

[samshull]

@bradjasper I started using

process.on('exit', function() {
  if (childProcess) {
    childProcess.kill()
    childProcess = null;
  }
});

So that I could ensure that it was cleaned up no matter what.

[travisjeffery]

fixed by #1043

[saitheexplorer]

I'm having a little trouble following the discussion here - happy to read up if someone can point me in the right direction.

Using the after() hook, rather than the before(), helps me prevent tests from clashing with one another when they're being run concurrently (e.g., on Travis).

• Before the tests run, I create a test id, and anything I insert to the database gets this identifier attached to it.
• When the tests are complete, I can then clean up only the values I just inserted.

If the test fails though, I have no way of deleting just those values, which are then stuck in the database until I manually clean them up.

Again, sorry if there is already some documentation on this - wasn't able to find it by following the above discussions.

[ndastur]

This is indeed very strange behaviour. Tests do fail and hence the expected behaviour is an order of

• before(...)
• the tests
• after(...) and this happens regardless of the tests passing or failing. Otherwise I see little value in the after hook. It just confuses everything

[vlucas]

I don't think this is fixed. My after hooks are definitely not getting run after a test failure. I have to cleanup manually every time, and it's highly annoying.

[travisjeffery]

post an example of a test suite where it's not working

[geoguide]

I am having an issues where the after does not run if the before hook fails. This is annoying because I do a register and authenticate before and a delete after and the register runs and then authenticate fails and i ahve tons of test data.

[dasilvacontin]

@geoguide, version of mocha and sample test suite where it fails?

[geoguide]

@dasilvacontin 2.1.0

before(function(done){
    request(app)
    .post('/register')
    .send({ 
        "email": testEmail,
        "password": testPassword
    })
    .expect(function(res){
        if(res.status != 201){
            return res.text;
        } else {
            userId = (res.body.id != null) ? res.body.id : 0;
            if(userId < 1){
                return 'id was '+userId;
            }
        }

    }).end(function(err,res){
        if(err){
            throw err;
        } else {
            done();
        }
    });
});

before(function(done){
    request(app)
    .post('/authenticate')
    .send({"email": testEmail, "password": testPassword })
    .expect(200)
    .end(function(err,res){
        if(err){
            throw err;
        } else {
            testJWT = JSON.parse(res.text).token;
            done();
        }
    });
});

after(function(done){
    request(app)
    .delete('/users/'+userId)
    .set('Authorization', 'Bearer ' +testJWT)
    .expect(200).end(function(err,res){
        if(err){
            throw err;
        } else {
            done();
        }
    })
});

Update! I am using supertest for the assertions. I neglected to mention that originally.

[dasilvacontin]

With mocha 2.1.0 I can't get to reproduce the bug with the following code:

describe('after a failed before', function() {

    before(function() {
        console.log('before hook');
        throw new Error
    })

    after(function() {
        console.log('the after hook should be called anyways')
    })

    it('shouldn\'t execute this test', function() {
        console.log('wat')
    })

})

[geoguide]

Maybe i need to throw the error instead of returning error messages on expects. I thought that was equivalent to throwing an error though.

To clarify I'm saying the "it shouldn't execute.." does not execute but the "after" should and does not in my code.

[geoguide]

That's not fixing it. It's very likely I'm formatting the test wrong, because this is my first shot at using mocha.

It is basically as you see it, i just tried changing the returns to throw errors and it got more messed up so went back to returning messages. I'll try looking at the docs some more to figure out where I'm going wrong, but for now i just have to make sure those methods in before are working.

[dasilvacontin]

@geoguide, the after hook is executing in my code.

Basically you have to throw errors if necessary, either manually, or via a library, like should, chai, etc. If you need help you can ping me at Gitter.

[dasilvacontin]

Okay, I confirm this is still failing when you are throwing async inside the before.

[searls]

Given how long this issue has been around, I suspect this is a relatively recent regression?

[dasilvacontin]

Maybe, our tests for these scenarios suck.

[dasilvacontin]

I'm working on this on PR #1531.

[dasilvacontin]

It seems like I'll have to change the uncaught error listener behaviour, so that I can test a mocha instance using mocha; the main mocha is getting the uncaught errors that are intended to get caught by the mocha instance I'm testing.

This might also fix a compatibility error we had with Hapi, or something similar that I read over the repo's issues.

[thardy]

I recently started using mocha, and I was surprised to see this bug as well. I have some tests that hit a MongoDb database and I wanted to use after to clean up any of the records my tests inserted. I was surprised to find out that "after" did not execute when my tests fail.

I look forward to this being fixed.

[rom1504]

I'm getting that problem too, also looking forward for a fix!

[jifeon]

guys! any news here?

[ajaks328]

@jifeon should be fixed in the latest release

[dasilvacontin]

Closed via https://github.com/mochajs/mocha/pull/1802. Thanks @ajaykodali!

[buddyp450]

Found a case in which afterEach doesn't run for mocha @ 5.2.0 if the async code is deeply nested and throws an error.

Repository here.

import { expect } from 'chai';

global.VALUE = 100;

describe('Suite 1', () => {
  it('passes a simple test', () => {
    expect(true).to.equal(true);
  });
  it('global value is 0', () => {
    expect(VALUE).to.equal(0);
  });
})

describe('Suite 2', () => {
  const anotherPromiseGuaranteedToFail = async () => {
    return new Promise((resolve, reject) => {
      // Modifying the global value here..
      VALUE = -1;
      try {
        throw new Error('failure');
      } catch (error) {
        reject(error);
      }
      resolve();
    });
  };

  // this before all hook is modifying the global value in an asynchronous way
  before(() => (
    new Promise((resolve, reject) => {
      resolve(0);
    }).then(() => {
      return anotherPromiseGuaranteedToFail();
    })
  ));

  it('passes a simple test', () => {
    expect(1).to.equal(1);
  });
})

describe('Suite 3', () => {
  it('global value is 0', () => {
    // fails since afterEach did not set the global value back to 0 after Suite 2
    expect(VALUE).to.equal(0);
  });
});

afterEach(() => {
  global.VALUE = 0;
});