1

Trying to get up to speed with node.js and nodeunit but am finding an issue with nodeunit where it's not seeing the call to test.done() in one of the tests.

The code:

// Added for clarity.
var client = require("restify").createJsonClient({
    "version": "*",
    "url": "http://localhost:" + server.Port
});

exports["tests"] = {
    "setUp": function (callback) {
        server.StartServer();
        callback();
    },
    "tearDown": function (callback) {
        callback();
    },
    "CanIHaveSomeTeaPlease?": function (test) {
        test.expect(4);
        client.get("/tea", function (err, req, res, data) {
            test.equal(err.statusCode, 418, "Expected ImATeapot Error.");
            test.equal(err.message, "Want a biscuit?", "Expected to be asked if I want a buscuit.");
            test.equal(err.restCode, "ImATeapotError");
            test.equal(err.name, "ImATeapotError");
            test.done();
        });
    },

    // Note: I expect this test to fail as it is a copy of the above
    //       test on a different url that doesn't return the ImATeapot
    //       HTTP error. But it doesn't look like it's detecting it
    //       properly.

    "TakeThisInfo": function (test) {
        test.expect(4);
        client.put("/push", {
            "hello": "world"
        }, function (err, req, res, data) {
            test.equal(err.statusCode, 418, "Expected ImATeapot Error.");
            test.equal(err.message, "Want a biscuit?", "Expected to be asked if I want a buscuit.");
            test.equal(err.restCode, "ImATeapotError");
            test.equal(err.name, "ImATeapotError");
            test.done();
        });
    }
};

Output:

FAILURES: Undone tests (or their setups/teardowns):
- tests - TakeThisInfo

To fix this, make sure all tests call test.done()

I'm hoping it is something stupid.

Versions:-

Node: 0.10.21
NPM: 1.3.11
Nodeunit: 0.8.2
Grunt-CLI: 0.1.10
Grunt: 0.4.1
Nalum
  • 4,143
  • 5
  • 38
  • 55
  • Does a put to `/push` actually work (regardless of what status it returns) when you use something like curl? – robertklep Nov 09 '13 at 13:50

4 Answers4

3

First, I don't know what "server" is in your code, but I would expect it to be asynchronous, so to have something more like this in your setUp function:

function (callback) {
  server.StartServer(function(){
    callback();
  });
}

Second, keep present that nodeunit executes the startUp and the tearDown functions after and before EVERY test so I suspect you are starting your server 2 times (as in the tearDown you are not really closing it).

matteofigus
  • 960
  • 9
  • 11
  • Adding the callback as you suggested didn't seem to work. I'm putting it down to the fact that I wasn't killing the server in the teardown function. See my answer for that I did. – Nalum Nov 11 '13 at 10:10
2

I've spent the last couple of hours messing with this issue, and what has become clear is that nodeunit has no ability to catch and display exceptions thrown in functions which are triggered later by an IO or setTimeout type process. Considering the way JavaScript runs, this isn't surprising. Everything works once you are sure there are no exceptions, but if you have an error in your code, you will get "undone tests" message and nothing else. Here is what I did to resolve my issues (using a restify route as an example):

function myRoute(req, res, next) {
    try {
        // your code goes here...
    }
    catch (err) {
        // write it to the console (for unit testing)
        console.log(err);
        // pass the error to the next function.
        next(err);
    }
}

Once I understood the problem in this way, fixing it because a lot more clear and I was able to get all of my tests to pass!

Steven Hunt
  • 2,321
  • 19
  • 18
  • Good catch! They should put this in the help test for Nodeunit, as it's almost always been my issue when working on tests. – Brad Apr 12 '15 at 23:52
  • Also, check this out for a solid solution: http://stackoverflow.com/a/20038890/362536 – Brad Apr 12 '15 at 23:54
1

I suspect you're not actually calling test.done() in that second test. Put a console.log() call in there to verify you're actually making that call.

FWIW, I repro'd the described problem using a simplified version of your test, below. If you omit the on('error', function() {...}) handler, then the 2nd test fails to complete. Thus, my theory is that your /push endpoint is triggering a different behavior in the restify module. I.e. are you sure restify is invoking your callback with an err property there, or is it doing something different? ... like, for example, emitting an event like http.get does, below.

var http = require('http');

exports.test1 = function (test) {
  test.expect(1);
  http.get({hostname: "www.broofa.com", path: "/"}, function (res) {
    test.equal(res.statusCode, 200, 'got 200');
    test.done();
  });
};

exports.test2 = function (test) {
  test.expect(1);
  http.get({hostname: "www.no-such-domain.com", path: "/"}, function (res) {
    test.equal(res.statusCode, 200, 'got 200');
    test.done();
  }).on('error', function() {
    // Comment line below out to repro the "Undone tests" error
    test.done();
  });
};
broofa
  • 37,461
  • 11
  • 73
  • 73
  • My understanding of Restify JSON Client is that it takes one callback on put/get/post/etc. request which receives error and success responses. So `test.done()` should be called regardless. I've put `console.log` in 3 places and it looks like the callback function isn't being run. – Nalum Nov 09 '13 at 15:00
  • so if the callback isn't run, then the issue isn't with nodeunit. it's with either your code, or with restify. Consider trying to reproduce the problem using node's native `http.get()` API. You may discover some unexpected behavior in your server code. (e.g. not sending back headers, or unexpected http status code... or something.) – broofa Nov 11 '13 at 01:44
1

I'm working around it by forking the server into it's own process in the setup and then killing it in the teardown. Think the issue was to do with the server being created and not shutdown. Thanks @matteofigus for that.

var cp = null; // child process
exports["tests"] = {
    "setUp": function (callback) {
        cp = fork("./lib/server.js", {"silent": true});
        callback();
    },
    "tearDown": function (callback) {
        cp.kill("SIGHUP");
        callback();
    },
    "CanIHaveSomeTeaPlease?": function (test) {
        test.expect(4);
        client.get("/tea", function (err, req, res, data) {
            test.equal(err.statusCode, 418, "Expected ImATeapot Error.");
            test.equal(err.message, "Want a biscuit?", "Expected to be asked if I want a buscuit.");
            test.equal(err.restCode, "ImATeapotError");
            test.equal(err.name, "ImATeapotError");
            test.done();
        });
    },
    "TakeThisInfo": function (test) {
        test.expect(1);
        client.put("/push", {
            "hello": "world"
        }, function (err, req, res, data) {
            test.ok(false);
            test.done();
        });
    }
};
Nalum
  • 4,143
  • 5
  • 38
  • 55