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

should use 'self.currentRunnable' or 'self.test' instead of 'test'? #189

Closed
wants to merge 1 commit into from
Closed

Conversation

dead-horse
Copy link

should use 'self.currentRunnable' or 'self.test' instead of 'test'? I'm not sure use which one, but 'test' may be covered when the callback function is called.

@tj
Copy link
Contributor

tj commented Jan 5, 2012

nope it should be fine as-is, currentRunnable may be the hooks as well

@tj tj closed this Jan 5, 2012
@dead-horse
Copy link
Author

var http = require('http');
var should = require('should');
var connect = require('connect');
var RedisStore = require('connect-redis')(connect);

var app = connect.createServer();
app.use(connect.cookieParser());
app.use(connect.session({
  secret : 'secret is this',
  store : new RedisStore()    //if use RedisStore, "TypeError: Cannot set property 'passed' of undefined" will apear
}))

app.use("/testPath", function(req, res){
    setTimeout(function(){
    res.end('{"status":"ok"}');
    res.setHeader('Content-Type', 'application/json');
    }, 200);
  })
describe('connect test', function(){
  before(function(){
    app.listen(3000);
  })
  describe('#get', function(){
    it('should get hello', function(done){
      var opt = {
        host : 'localhost',
        port : 3000,
        path : '/testPath'
      }
      http.get(opt, function(res){
        var bufs = [];
        res.on('data', function(data){
          bufs.push(data);
        })
        res.on('end', function(data){
          bufs.push(data||'');
          bufs.join().toString().should.include('ok');
          done();
        })
      })
    })
  })
})

When I test this, this error happened, I use connect 1.8.5 & connect-redis 1.2.0. I don't kown where the problem is, but it happened. If remove the RedisStore, it will be fine.

@tj
Copy link
Contributor

tj commented Jan 6, 2012

i dont get that error, but this is incorrect:

app.use("/testPath", function(req, res){
    setTimeout(function(){
    res.end('{"status":"ok"}');
    res.setHeader('Content-Type', 'application/json');
    }, 200);

you cant set header fields after responding, it should be:

app.use("/testPath", function(req, res){
  setTimeout(function(){
    res.setHeader('Content-Type', 'application/json');
    res.end('{"status":"ok"}');
  }, 200);
})
app.use("/testPath", function(req, res){
  setTimeout(function(){
    res.send({ status: 'ok' })
  }, 200);
})

@dead-horse
Copy link
Author

I kown,i did this on purpose, Dont fix this error and run the test, If this error throwed here, I got Error like this:

./node_modules/mocha/bin/mocha --reporter spec test.js
The "sys" module is now called "util". It should have a similar interface.

connect test
#get
0) should get hello
1) should get hello

✖ 2 of 1 tests failed:

  1. connect test #get should get hello:
    TypeError: Cannot set property 'passed' of undefined
    at /home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:319:21
    at done (/home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:131:5)
    at /home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:143:9
    at IncomingMessage. (/home/admin/cnae/git/tst/test.js:38:11)
    at IncomingMessage.emit (events.js:88:20)
    at HTTPParser.onMessageComplete (http.js:137:23)
    at Socket.ondata (http.js:1125:24)
    at TCP.onread (net.js:334:27)

  2. connect test #get should get hello:
    TypeError: Cannot set property 'passed' of undefined
    at /home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:319:21
    at done (/home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:131:5)
    at /home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:143:9
    at IncomingMessage. (/home/admin/cnae/git/tst/test.js:38:11)
    at IncomingMessage.emit (events.js:88:20)
    at HTTPParser.onMessageComplete (http.js:137:23)
    at Socket.ondata (http.js:1125:24)
    at TCP.onread (net.js:334:27)

In my own real code, I made this kind of mistake not obvious, then I test with mocha , this error made me puzzled.
This error appeared in both of my cumputers.

@tj
Copy link
Contributor

tj commented Jan 6, 2012

weiiird i didnt get that i'll try again

@tj
Copy link
Contributor

tj commented Jan 6, 2012


  0) connect test #get should get hello
  1) connect test #get should get hello

  ✖ 2 of 1 tests failed:

  0) connect test #get should get hello:
     Error: Can't set headers after they are sent.
      at ServerResponse.<anonymous> (http.js:531:11)
      at ServerResponse.setHeader (/Users/tj/Projects/mocha/node_modules/connect/lib/patch.js:62:20)
      at Object._onTimeout (/Users/tj/Projects/mocha/test.js:16:9)
      at Timer.ontimeout (timers.js:84:39)

  1) connect test #get should get hello:
     Error: Can't set headers after they are sent.
      at ServerResponse.<anonymous> (http.js:531:11)
      at ServerResponse.setHeader (/Users/tj/Projects/mocha/node_modules/connect/lib/patch.js:62:20)
      at Object._onTimeout (/Users/tj/Projects/mocha/test.js:16:9)
      at Timer.ontimeout (timers.js:84:39)

@tj
Copy link
Contributor

tj commented Jan 6, 2012

oh wait now i got it with a few tweaks

@tj
Copy link
Contributor

tj commented Jan 6, 2012

this is a similar yet shitty problem that we face:

function foo() {
  setTimeout(function(){
    throw new Error('fail in foo()');
  }, 200);
}

describe('something', function(){
  it('should foo', function(done){
    process.nextTick(function(){
      foo();
      done();
    });
  })

  it('should bar', function(done){
    setTimeout(done, 200);
  })
})

however in practice I find this comes up extremely rare


  ✓ something should foo: 0ms
  0) something should bar

  ✖ 1 of 2 tests failed:

  0) something should bar:
     Error: fail in foo()
      at Object._onTimeout (/Users/tj/Projects/mocha/test.js:47:11)
      at Timer.ontimeout (timers.js:84:39)

@dead-horse
Copy link
Author

i sort of understand. when an error throwed after done(), some strange error will happened. Especially in async test, like this

function foo(fn) {
    fn();
    throw new Error('fail in foo()');
}

describe('something', function(){
  it('should foo', function(done){
    process.nextTick(function(){
      foo(done);
    })
  })
  it('should foo second', function(done){
    foo(done);
  })
})

I got this...

./node_modules/mocha/bin/mocha --reporter spec test1.js

  something
    ✓ should foo 
    0) should foo
    ◦ should foo second: 
  ✓ should foo second 
  1) should foo second

  ✖ 2 of 2 tests failed:

  0) something should foo:
     Error: fail in foo()
      at foo (/home/admin/cnae/git/tst/test1.js:3:11)
      at Array.0 (/home/admin/cnae/git/tst/test1.js:9:7)
      at EventEmitter._tickCallback (node.js:192:40)

  1) something should foo second:
     Error: done() called multiple times
      at multiple (/home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:122:24)
      at done (/home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:127:26)
      at Test.run (/home/admin/cnae/git/tst/node_modules/mocha/lib/runnable.js:146:7)
      at Runner.runTest (/home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:271:10)
      at /home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:312:12
      at next (/home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:199:14)
      at /home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:208:7
      at next (/home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:157:23)
      at Array.0 (/home/admin/cnae/git/tst/node_modules/mocha/lib/runner.js:176:5)
      at EventEmitter._tickCallback (node.js:192:40)

So weiiird. But I must avoid this kind of stupid error appear in my program.
Anyway, thank you for your frameworks, both connect and mocha helped me so mach. :D

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

Successfully merging this pull request may close these issues.

2 participants