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

beforeEach and afterEach are running in nested suites/contexts #911

Closed
twolfson opened this issue Jun 28, 2013 · 17 comments
Closed

beforeEach and afterEach are running in nested suites/contexts #911

twolfson opened this issue Jun 28, 2013 · 17 comments

Comments

@twolfson
Copy link

There is no documentation on when a beforeEach or afterEach will run. My intuition states that it should be run before/after every describe/it block in the current context completes.

However, the behavior I am noticing, is that beforeEach and afterEach are run before/after every it block in the current context and all nested contexts.

I have created a proof of concept to demonstrate that the noticed behavior is occurring: https://gist.github.com/twolfson/5883057#file-test-js

For convenience, I will copy/paste the script and output here:

Script

describe('An afterEach hook', function () {
  afterEach(function () {
    console.log('afterEach run!');
  });

  before(function () {
    console.log('before run!');
  });

  describe('in some nested contexts', function () {
    before(function () {
      console.log('nested before run!');
    });

    it('runs after this block', function () {
      console.log('nested it run!');

    });

    it('runs again after this block', function () {
      console.log('second nested it run!');
    });
  });
});

Output

before run!
nested before run!
nested it run!
afterEach run!
second nested it run!
afterEach run!

The behavior I intuitively anticipate is afterEach to run once, after the second nested it.

before run!
nested before run!
nested it run!
second nested it run!
afterEach run!

My use case for the intuitive functionality is cleaning out the this context after a context completes. The current implementation will clean out the context after every assertion.

afterEach(function () {
  var key;
  for (key in this) {
    delete this[key];
  }
});

describe('A banana', function () {
  before(function () {
    this.banana = new Banana();
  });

  describe('when peeled', function () {
    before(function () {
      this.peeledBanana = this.banana.peel();
    });

    it('is white', function () {
      assert.strictEqual(this.peeledBanana.color, 'white');
      // `afterEach` is invoked here, cleaning out `this`
    });

    it('is soft', function () {
      // `this.peeledBanana` is no longer defined since `afterEach` cleaned it out
      assert.strictEqual(this.peeledBanana.hardness, 'soft');
    });
  });
});

A comparable example to this would be opening a database transaction in each new context of a test suite.

I can work around it by invoking after in each nested context but I feel like there could be a hook in the framework.

@twolfson
Copy link
Author

For those curious, this is the workaround for my use case:

// Before anything else is run
before(function () {
  // Iterate over all of the test suites/contexts
  this.test.parent.suites.forEach(function bindCleanup (suite) {
    // Attach an afterAll listener that performs the cleanup
    suite.afterAll(function cleanupContext () {
      var key;
      for (key in this) {
        delete this[key];
      }
    });
  });
});

@Plypy
Copy link

Plypy commented Jul 7, 2014

+1

@boneskull
Copy link
Member

afterEach runs after every Runnable instance; in your case, an it() block. If you do not want the nested behavior, don't nest your tests. Altering this behavior is not on the table.

If your second test depends on something defined in your first test, your tests are dirty and you're doing it wrong. The order in which your tests execute should not matter. Write your tests accordingly, and they will be more valuable.

Also, it's not necessary to put anything on this 90% of the time.

describe('A banana', function () {
  var banana;

  before(function () {
    banana = new Banana();
  });

  describe('when peeled', function () {
    var peeledBanana;

    beforeEach(function () {
      // I'm assuming peel() has no side-effects since it returns a new object.
      peeledBanana = banana.peel();
    });

    it('is white', function () {
      assert.strictEqual(peeledBanana.color, 'white');
    });

    it('is soft', function () {
      assert.strictEqual(peeledBanana.hardness, 'soft');
    });
  });
});

@RathaKM
Copy link

RathaKM commented Dec 31, 2015

Is there a way to run hook methods in the describe() level instead of it() level? It might be useful in some scenarios.

@boneskull
Copy link
Member

@RathaKM What's the use case?

@cpoonolly
Copy link

cpoonolly commented Sep 14, 2016

Not sure if this is a good example but, would the following be a good use case for this? @RathaKM feel free to correct.

describe('A banana', function () {
  var banana;

  beforeEach(function () {
    banana = new Banana();
  });

  describe('when peeled', function () {
    var peeledBanana;

    before(function () {
      // Lets assume peel() HAS side-effects and doesn't return a new object.
      banana.peel();
      peeledBanana = banana;
    });

    it('is white', function () {
      assert.strictEqual(peeledBanana.color, 'white');
    });

    it('is soft', function () {
      assert.strictEqual(peeledBanana.hardness, 'soft');
    });
  });

  describe('when set on FIRE', function () {
    var flamingBanana;

    before(function () {
      // Same assumption as above
      banana.setOnFire();
      flamingBanana = banana
    });

    it('is hot', function () {
      assert.isAbove(flamingBanana.temperature, 9000);
    });
  });
});

@RathaKM
Copy link

RathaKM commented Sep 18, 2016

@cpoonolly: thanks for your comments and bring it back. :) Your example is what logically expected to work in the describe level(the BeforeEach). But, this could be achieved with 'before()' within each describe.

The issue I was facing is given in the below example.

var testData = [{country: 'NIL'}],
dataDriven = require('data-driven'),
assert = require('assert');

describe('@Testing_For_Describe_Level_Hook_Method_To_Update_TestData@', function () {

    describe('@Updated testData for US@', function () {
        before(function (done) {
            testData = [{country: 'US'}];
            done();
        });
        dataDriven(testData, function () {
            it('expecting updated testData for US', function (ctx, done) {
                assert.equal(ctx.country, 'US');
                done();
            });
        });
    });

    describe('@Updated testData for UK@', function () {
        before(function (done) {
            testData = [{country: 'UK'}];
            done();
        });
        dataDriven(testData, function () {
            it('expecting updated testData for UK', function (ctx, done) {
                assert.equal(ctx.country, 'UK');
                done();
            });
        });
    });
});

Here, the test suite will fail. We update the testData within 'before()' hook method which will be called just before the 'it' block. But, we use the testData variable within dataDriven() which will be executed before executing the 'it' block.

So, what we need here is a place to update the variable before the 'it' block execution begins. May be we need the before() hook method execution just before the 'describe' block(describe level hook method).

Hope, the issue is clear now.

@cloutiertyler
Copy link

Has there been any additional discussion on this?

@marqu3z
Copy link

marqu3z commented Jul 20, 2017

why is not possible to get something like this?

describe('Create Article', () => {
  beforeEach(() => console.log('CLEAN DB'))

  context('When Article is valid', () => {
    before(() => console.log("INSERTING VALID ARTICLE"))

    it('should not fail')
    it('should have some properties')    
    it('should send an email')  
  })

  context('When Article is not valid', () => {
    before(() => console.log("INSERTING NOT VALID ARTICLE"))

    it('should fail')
    it('should not send an email')  
  })
})
  • Clean the DB before each context block.
  • Set up the scenario before all it blocks

Isn't this a legit use case?

@ORESoftware
Copy link

ORESoftware commented Jul 20, 2017

@marqu3z I am trying to follow your use case. It is legit. It sounds like the problem is: the before hook runs prior to the beforeEach hook? but you need the before hook to run subsequent to the beforeEach hook?

(do you really want to clean the DB for every test case? seems like only your first test case will succeed, the others will have no data. But I will assume you want to clean out the DB for every test case).

Here are my two attempts to solve your use case given Mocha as is:

(1) use a beforeEach, but flip a boolean, so the beforeEach only runs once (this kinda sucks)

describe('Create Article', () => {
  beforeEach(() => console.log('CLEAN DB'))

  context('When Article is valid', () => {
    let flip = true;
    beforeEach(() => flip && (flip = false; console.log('INSERTING VALID ARTICLE')))

    it('should not fail')
    it('should have some properties')    
    it('should send an email')  
  })

  context('When Article is not valid', () => {
   let flip = true;
    beforeEach(() => flip && (flip = false; console.log('INSERTING NOT VALID ARTICLE')))

    it('should fail')
    it('should not send an email')  
  })
})

(2) The other way to do this - this is not much better!

describe('Create Article', () => {
  
  let cleanDB = function(fn){
    return function(done){
        actuallyCleanDB(function(err){
            if(err) return done(err);
            fn(done);
       });
    }
  };

  context('When Article is valid', () => {
    before(() => console.log('INSERTING VALID ARTICLE'))
    it('should not fail', cleanDB(function(done){})
    it('should have some properties', cleanDB(function(done){}))    
    it('should send an email', cleanDB(function(done){})) 
  })

  context('When Article is not valid', () => {
    before(() => console.log('INSERTING NOT VALID ARTICLE')))
    it('should fail', cleanDB(function(done){}))
    it('should not send an email',cleanDB(function(done){}))  
  })
})

Please let me know if I understand your problem, and I will spend more cycles thinking of a solution. Neither of my solutions are very good, but I just want to make sure I understand the problem you have.

@ORESoftware
Copy link

ORESoftware commented Jul 20, 2017

@RathaKM

are you registering your it() test cases asynchronously? If so, that is not allowed. You must register describe/after/afterEach/before/beforeEach all synchronously.

// event loop tick X
 dataDriven(testData, function () {
     // it() must be called in the same event loop tick as X above
        it('expecting updated testData for UK', function (ctx, done) {
            assert.equal(ctx.country, 'UK');
            done();
       });
  });

also, where is ctx coming from? Maybe I am not up-to-date on the latest Mocha features, but I have never seen

it(foo, function(ctx, done){
   // where is ctx coming from?
});

@makrandgupta
Copy link

I'm in the exact same scenario @marqu3z described. Would love to see support for a describe level beforeEach

@marqu3z
Copy link

marqu3z commented Jun 13, 2018

To everyone with this common scenario i created this helper function that iterates over all suites in the current block and add a beforeAll hook to each of them:

function beforeEachSuite (fn) {
  before(function () {
    let suites = this.test.parent.suites || []
    suites.forEach(s => {
      s.beforeAll(fn)
      let hook = s._beforeAll.pop()
      s._beforeAll.unshift(hook)
    })
  })
}

This unlock the use case i describe almost an year ago:

describe('Create Article', () => {
  beforeEachSuite(() => console.log('CLEAN DB'))

  context('When Article is valid', () => {
    before(() => console.log("INSERTING VALID ARTICLE"))

    it('should not fail')
    it('should have some properties')    
    it('should send an email')  
  })

  context('When Article is not valid', () => {
    before(() => console.log("INSERTING NOT VALID ARTICLE"))

    it('should fail')
    it('should not send an email')  
  })
})

Still don't get why this use case is not considered useful, or legit.
Maybe this solution can be turned in a PR for a beforeEachSuite and afterEachSuite

@elitan
Copy link

elitan commented Nov 6, 2018

Yes please we need this.

There are many use cases like #911 (comment)

@manan
Copy link

manan commented Dec 3, 2018

+1

1 similar comment
@sinkovsky
Copy link

+1

@owenfarrell
Copy link

I just stumbled on this thread looking for something similar. So to anyone else who stumbles on this post...

I've adapted the approach that @marqu3z outlined in to its own NPM package: mocha-suite-hooks.

So if you find yourself in a situation where before isn't enough, and beforeEach is too much, give beforeSuite a try.

The package is still pretty raw, so any feedback would be greatly appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests