Prevent assert in topics #203

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@Filirom1
Contributor

Filirom1 commented Mar 30, 2012

Users should not use asserts in topics.
Unfortunately there is no warning in vows to prevent this.

Before the following test fails with a strange message, at the wrong assert :

var vows = require('vows'),
  assert = require('assert');


vows.describe('A vows test').addBatch({
  'Given a failing topic': {
    topic: function(){
      var self = this;
      setTimeout(function(){
        self.callback(null, 1)
      }, 100);
    },
    'When using a sub-topic': {
      topic: function(value){
        assert.equal(value, 0);
        var self = this;
        setTimeout(function(){
          self.callback(null, 2)
        }, 100);
      },
      'Then this function should not be called': function(err, value){
        assert.ifError(err);
        assert.equal(value, 2);
      }
    },
  }
}).export(module);

The message :

$ vows test/myTest.js --spec

  ♢ A vows test 

  Given a failing topic When using a sub-topic
    ✗ Then this function should not be called 
        » expected 2, 
    got  { 
      stack: '\033[33mexpected \033[1m0\033[22m,\n\011got\011 \033[1m1\033[22m (\033[1m==\033[22m)\033[39m\n    at Object.<anonymous> (/home/romain/test-vows/test/totoTest.js:15:16)\n    at run (/home/romain/vows/lib/vows/suite.js:134:35)\n    at EventEmitter.<anonymous> (/home/romain/vows/lib/vows/suite.js:234:40)\n    at EventEmitter.<anonymous> (events.js:88:20)\n    at EventEmitter.emit (/home/romain/vows/lib/vows.js:236:24)\n    at /home/romain/vows/lib/vows/context.js:32:52\n    at Object.callback (/home/romain/vows/lib/vows/context.js:46:29)\n    at Object._onTimeout (/home/romain/test-vows/test/totoTest.js:10:14)\n    at Timer.ontimeout (timers.js:94:19)', 
      actual: 1, 
      operator: '==', 
      expected: 0, 
      name: 'AssertionError', 
      message: 'expected 2,\n\011got\011 {actual} (==)' 
  } (==) // myTest.js:15

✗ Broken » 1 broken (0.107s) 

Now it fails with a warning :

$ vows test/myTest.js 

timers.js:96
            if (!process.listeners('uncaughtException').length) throw e;
                                                                      ^
Error: topic should not contains assert
    at run (/home/romain/vows/lib/vows/suite.js:137:62)
    at EventEmitter.<anonymous> (/home/romain/vows/lib/vows/suite.js:234:40)
    at EventEmitter.<anonymous> (events.js:88:20)
    at EventEmitter.emit (/home/romain/vows/lib/vows.js:236:24)
    at /home/romain/vows/lib/vows/context.js:32:52
    at Object.callback (/home/romain/vows/lib/vows/context.js:46:29)
    at Object._onTimeout (/home/romain/test-vows/test/myTest.js:10:14)
    at Timer.ontimeout (timers.js:94:19)
@JerrySievert

This comment has been minimized.

Show comment Hide comment
@JerrySievert

JerrySievert Aug 10, 2012

Contributor

anyone have any comments? (except the grammar)

Contributor

JerrySievert commented Aug 10, 2012

anyone have any comments? (except the grammar)

@adamstallard

This comment has been minimized.

Show comment Hide comment
@adamstallard

adamstallard Jan 22, 2013

Contributor

I don't see how you could differentiate between an assertion error in a module under test, and an assertion error in the topic (aside from parsing the javascript in the topic, which is probably too much).

The strange output in Filirom's example comes because vows currently pushes a null onto the arguments that it sends to the vow with if you have suite.options.error set to true and more than one input parameter to the vow. So the error is going to be the second argument, and the first argument is going to be null. This is what really needs to be fixed. Once that is fixed, the test would error with an assertion error instead of giving the weird output.

Contributor

adamstallard commented Jan 22, 2013

I don't see how you could differentiate between an assertion error in a module under test, and an assertion error in the topic (aside from parsing the javascript in the topic, which is probably too much).

The strange output in Filirom's example comes because vows currently pushes a null onto the arguments that it sends to the vow with if you have suite.options.error set to true and more than one input parameter to the vow. So the error is going to be the second argument, and the first argument is going to be null. This is what really needs to be fixed. Once that is fixed, the test would error with an assertion error instead of giving the weird output.

@adamstallard

This comment has been minimized.

Show comment Hide comment
@adamstallard

adamstallard Jan 23, 2013

Contributor

I fixed this in pull request #263

Contributor

adamstallard commented Jan 23, 2013

I fixed this in pull request #263

@indexzero

This comment has been minimized.

Show comment Hide comment
@indexzero

indexzero Nov 4, 2014

Owner

Closing since this was fixed in #263

Owner

indexzero commented Nov 4, 2014

Closing since this was fixed in #263

@indexzero indexzero closed this Nov 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment