Honoring Vows and Events in Node-SPDY

‹prev | My Chain | next›

I found yesterday that tests in node-spdy master were failing with the most recent version of vows.js. Since I am working with edge node-spdy, edge node.js and a version of vows.js that is less than a week old, it took some time to track down the issue.

The failure looks like:

➜  node-spdy  vows --spec ./test/spdy-basic-test.js

♢ SPDY/basic test

✗ Errored » callback not fired
in spdy.createServer
in SPDY/basic test
in test/spdy-basic-test.js
✗ Errored » 1 errored ∙ 1 dropped

Since vows is meant to test a reactor patten framework, it has the concept of testing callbacks built-in. But the thing is, the failing test is not testing a callback, it is just trying to test whether or not the createServer function returns a SPDY server thingy:

vows.describe('SPDY/basic test').addBatch({
'spdy.createServer': {
topic: function() {
return spdy.createServer(options);
},
'should return spdy.Server instance': function (_server) {
assert.instanceOf(_server, spdy.Server);
server = _server;
}
}
}).addBatch({
//...

Clever placement of console.log statements proves that spdy.createServer() is, indeed returning a SPDY server thingy, but vows is not even making it into the assertion function. It is failing because the supposed callback is not being fired. What gives?

Well, I tracked this down to a recent commit in vows.js: 76565ef. Specifically, the heuristic used to decide if a topic is a callback thingy changed from looking at the constructor:

// If the topic doesn't return an event emitter (such as a promise),
// we create it ourselves, and emit the value on the next tick.
if (! (topic && topic.constructor === events.EventEmitter)) {
// ... 

Instead, vows now asks if the topic is an instanceof events.EventEmitter:

// If the topic doesn't return an event emitter (such as a promise),
// we create it ourselves, and emit the value on the next tick.
if (! (topic && (topic instanceof events.EventEmitter))) {
// ... 

In node-spdy, createServer uses the Server constructor:

core.createServer = function(options, requestListener) {
  return new Server(options, requestListener);
};

Thus, the earlier version of vows would have moved right past such a topic—the constructor is Server, not events.EventEmitter. But... events.EventEmitter is part of the prototype chain by virtue of an util.inherits:

var Server = core.Server = function(options, requestListener) {
// ...
};
util.inherits(Server, tls.Server);

So, now that vows is checking the entire prototype chain (and probably rightly so) to see if the topic is an events.EventEmitter, the first node-spdy test is no longer valid:

vows.describe('SPDY/basic test').addBatch({
'spdy.createServer': {
topic: function() {
      return spdy.createServer(options);
},
'should return spdy.Server instance': function (_server) {
assert.instanceOf(_server, spdy.Server);
server = _server;
}
}
}).addBatch({
//...

But how to fix it?

I could argue that the test itself is weak and should go away. Most, if not all of the remaining tests would fail if, somehow, spdy.createServer were to start returning a String. Still, it's a nice sanity check.

So I wrap the topic in a thin, non-events.EventEmitter wrapper. More specifically, I wrap it in an object literal:

vows.describe('SPDY/basic test').addBatch({
'spdy.createServer': {
topic: function() {
      return {server: spdy.createServer(options)};
},
'should return spdy.Server instance': function (topic) {
      server = topic.server;
assert.instanceOf(server, spdy.Server);
}
}
}).addBatch({
//...

That is kind of a hack, but for a weak test, it is not that much of a stretch. Or maybe I am just rationalizing. I've been known to do that.

Anyhow, I have the test passing again:

vows runner running SPDY/basic test 
♢ SPDY/basic test

spdy.createServer
✓ should return spdy.Server instance

That is a fine stopping point for today. Up tomorrow, hopefully I can actually get back to the bug that I had hoped to investigate before I came across my latest rabbit hole.

Day #119