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

before/after have wrong "this" scope? #253

Closed
zzen opened this issue Feb 7, 2012 · 11 comments
Closed

before/after have wrong "this" scope? #253

zzen opened this issue Feb 7, 2012 · 11 comments
Labels
type: feature enhancement proposal

Comments

@zzen
Copy link

zzen commented Feb 7, 2012

beforeEach/afterEach correctly invoke with "this" set to the scope of the suite. before/after however use global object instead. Maybe I'm missing something, but it seems weird why this would be intentional.

Test case:

assert = require('assert')

describe('scope is lost',function() {
    before(function() { this.foo="world"}) // also this leaks into global scope
    describe('when using before/after', function() {
        it('should have value set for foo', function() { assert.equal(this.foo,'world') })
    })
})

describe('scope works fine',function() {
    beforeEach(function() { this.bar="hello"})
    describe('when using beforeEach/afterEach', function() {
        it('should have value set for bar', function() { assert.equal(this.bar,'hello') })
    })
})

Result:

scope is lost
  0) "before all" hook
  when using before/after
    1) should have value set for foo

scope works fine
  when using beforeEach/afterEach
    ✓ should have value set for bar 


✖ 2 of 2 tests failed:

0) scope is lost "before all" hook:
   Error: global leak detected: foo
    at Runner.checkGlobals (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:103:21)
    at Runner.<anonymous> (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:44:44)
    at Runner.emit (events.js:64:17)
    at /Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:170:12
    at Hook.run (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runnable.js:155:5)
    at next (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:167:10)
    at Array.<anonymous> (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:177:5)
    at EventEmitter._tickCallback (node.js:126:26)

1) scope is lost when using before/after should have value set for foo:

ssertionError: "world" == "undefined"
    at Test.fn (/Users/jakub/Documents/test.js:6:59)
    at Test.run (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runnable.js:153:32)
    at Runner.runTest (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:272:10)
    at /Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:313:12
    at next (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:200:14)
    at /Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:209:7
    at next (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:157:23)
    at Array.<anonymous> (/Users/jakub/.nvm/v0.4.12/lib/node_modules/mocha/lib/runner.js:177:5)
    at EventEmitter._tickCallback (node.js:126:26)
@tj
Copy link
Contributor

tj commented Feb 7, 2012

beforeEach/afterEach use "this" as the Test, which doesn't make sense for before/after, since it's all serial we could use Suite, or a custom Context object would be better since then we could delegate this.timeout(n) -> this.test.timeout(n)

@zzen
Copy link
Author

zzen commented Feb 7, 2012

Maybe I'm doing this wrong then. I'm trying to get a setup into place, then run a bunch of tests and have a teardown. Pretty common use-case, right? Part of my setup is a creating a tobi browser instanceI want to reuse in the tests. But how do I push that instance into the tests (without having it in beforeEach)?

@tj
Copy link
Contributor

tj commented Feb 7, 2012

well you can do the same thing with closures:

describe('something', function(){
  var foo;

   ...before...
     foo = stuff
   ..

blah blah haha which is also helpful for:

describe('utils.parseCookie()', function(){
  var parse = utils.parseCookie;

  it('should do something', function(){
    parse(foo)
  })

  it('should do something', function(){
    parse(foo)
  })

  it('should do something', function(){
    parse(foo)
  })
})

but this sort of thing would be nice for shared behaviours since they're not lexically scoped

@zzen
Copy link
Author

zzen commented Feb 7, 2012

True, somehow that escaped me, late night :) Thanks.
Going back to topic - anything sounds better then running before/after in global context.

@zzen
Copy link
Author

zzen commented Feb 7, 2012

Right - that's why I didn't originally click with closures: I got async tests that need async setup...

describe('Async tests', function(){
    before(function(done) {
      initDb(function(err, conn) {
            this.connection = conn;
            done()
        })
    })

    describe('Query Database', function() {
        before(function(done) {
            this.conn.query('select something', done)
        })
        it('should return results', function(data) {
            assert ...
        })
    })

@tj
Copy link
Contributor

tj commented Feb 7, 2012

well you can still use enclosed vars for that:

describe('something', function(){
  var connection;
  before(function(done){
    initwhatever(function(err, conn){
      connection = conn;
      done();
      ...
  })

^ is effectively identical

@tj
Copy link
Contributor

tj commented Feb 7, 2012

https://github.com/visionmedia/mocha/wiki/Shared-Behaviours is the only reason we need "this" as anything special

@zzen
Copy link
Author

zzen commented Feb 7, 2012

Thanks TJ

@fernandoPalaciosGit
Copy link

Thanks, the only way is reference by the chain testing, it -> it -> it, thats not preaty nice

this.test.parent.parent

@fernandoPalaciosGit
Copy link

Maybe creating a simple Object for the main ñcontext testing

var Test = {
    connection: null
};

beforeEach(function () {
    this.connection = new MongoDbConn();
}.bind(Test));

describe('something', function(){
    it('do a query', function(done){
        initwhatever(function(err, conn){
            this.connection.query('select something', done);
            done();
        }.bind(Test));
    });
});

A simple Restfull API testing

@boneskull
Copy link
Contributor

My recommendation is to use closures instead of assigning things to this.

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

No branches or pull requests

4 participants