nodeunit icon indicating copy to clipboard operation
nodeunit copied to clipboard

Incomplete tests/setups/teardowns with RequireJS

Open thebigredgeek opened this issue 12 years ago • 11 comments

Here is my test (using grunt-contrib-nodeunit):

var requirejs = require('requirejs');

exports.test = function(test){
    test.expect(1);
    requirejs(['dist/test-hts'],function(obj){
        test.ok(true, 'hello');
        test.done();
    });
};

For some reason, this doesn't work! It appears as though RequireJS doesn't play nice with node unit. I've been trying to get testing working for the better part of 3 days with no luck so far, even on the most simple of tests like above!

This happens when I run with vanilla nodeunit from the command line:

test.js

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

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

If I move the .done() to the outside of the callback, it just skips the test all together and says that it expected 1 assertion but found none.

I have also tried it this way:

var requirejs = require('requirejs');
requirejs.config({
    nodeRequire: require,
    baseUrl: __dirname
});

requirejs(['../dist/test-hts'],function(obj){
    console.log('loaded...');
    exports.test = function(test){
        test.expect(1);
        test.ok(true, 'hello');
        test.done();
    };
});

It results in:

OK: 0 assertions (41ms)

Also, if I run this same file using node rather than nodeunit (just to check execution), I get the following output:

loaded...

So I know that the module is loading properly, just not with nodeunit!

Any help here would be great. I don't understand why nodeunit wouldn't work with such a well-known AMD tool like RequireJS. Thanks!

thebigredgeek avatar May 03 '13 14:05 thebigredgeek

Bump

thebigredgeek avatar May 06 '13 14:05 thebigredgeek

@rhyneandrew isn't there a necessary setup step prior to calling require(...) ? I might be wrong but I thought there was. usually when loading require in the browser you point at an setup.js file that configures paths, etc so require knows where to look for things.

mreinstein avatar May 07 '13 21:05 mreinstein

This works as expected when NOT using nodeunit, so I am not sure. I have been using require.js quite a bit lately, and it looked promising for node as it makes organizing/compiling/linking different code segments together trivial. Testing, however, has proven to be a nightmare.

thebigredgeek avatar May 07 '13 21:05 thebigredgeek

it looked promising for node as it makes organizing/compiling/linking different code segments together trivial

I agree requirejs is great...in a browser context. Because you have to deal with loading code over the network connection which is very asynchronous by nature. On the server side though, it feels like swatting a fly with a missile. Synchronous commonjs is a lot simpler internally, and it's what everyone is already using.

I'm not suggesting this as an excuse for requirejs giving you headaches with nodeunit. It's just a bit of an unusual convention for the server side.

mreinstein avatar May 07 '13 21:05 mreinstein

Right. The reason I went with RequireJS server-side was because I needed the ability to link/compile seamlessly - I want to be able to incorporate linking and compiling into my workflow so that I can compile dependencies INTO my project (keeps it dry). The R.JS descriptor notation allows me to deep-link all of the code within the project together and spit it out into a single .js file. It allows me to keep a well structured code base.

thebigredgeek avatar May 08 '13 14:05 thebigredgeek

I abandoned requirejs and just went the commonjs route. You can close the issue, but I'd recommend looking into it.

thebigredgeek avatar May 08 '13 15:05 thebigredgeek

I don't think this has to be closed, as far as I can tell there's nothing wrong with wanting to use requirejs on the server side. It's just a bit uncommon. From requirejs's front page:

It is optimized for in-browser use, but it can be used in other JavaScript environments, like Rhino and Node.

I'll reopen it, and if this comes up again we can revisit

mreinstein avatar May 09 '13 00:05 mreinstein

I'm getting the same error, though not with RequireJS.

var path = require('path');

exports[path.basename(__filename)] = function (test) {
    var async = require('async'),
            CUBRID = require('node-cubrid');

    test.expect(1);

    function query(client, cb) {
        async.waterfall([
            function (cb) {
                console.log('before connect');
                client.connect(cb);
            },
            function (cb) {
                console.log('before query');
                client.query('SELECT * FROM nation', cb);
            },
            function (result, queryHandle, cb) {
                client.closeQuery(queryHandle, cb);
            },
            function (queryHandle, cb) {
                client.close(cb);
            }
        ], cb);
    }

    console.log('before parallel');

    async.parallel([
        function (cb) {
            console.log('before first client');
            query(CUBRID.createDefaultCUBRIDDemodbConnection(), cb);
        },
        function (cb) {
            console.log('before second client');
            query(CUBRID.createDefaultCUBRIDDemodbConnection(), cb);
        }
    ], function (err) {
            console.log('done');
        if (err) {
            throw err;
        } else {
            test.ok(true);
            test.done();
        }
    });
};

console.log('after parallel');

Output:

before parallel
before first client
before second client
after parallel
before connect
before connect

Notice that "before connect" gets called for both clients, but then nodeunit somehow mysteriously finishes the test before the client has received a response from the server which obviously leads to Undone tests error.

This code is a part of the tests suite of node-cubrid module. Just yesterday this code was working. The only thing I did today was installed a new version of Node.js 0.10.13 on Ubuntu 12.04 x64.

Forgot to mention that if I run nodeunit on this test only, it passes as expected. However, when I run the entire suite in test directory, it fails on this test script.

One suspicion I have is that nodeunit doesn't play well when the async callbacks are called in process.nextTick(). It's just a guessing.

kadishmal avatar Jul 17 '13 07:07 kadishmal

@caolan, @mreinstein would you please suggest any solution for this problem or tell how to debug nodeunit process? Our QA is stuck hard on this async test.

If you want, you can replicate this issue by forking node-cubrid repo. I can give you access to a test DB so you don't have to install CUBRID database.

kadishmal avatar Jul 18 '13 02:07 kadishmal

I'm experiencing exactly the same problem as kadishmal...

I'm testing a JSON API with http.get(). Calling test.done() from within the response.on("data") callback is fine, but if I add a further nested call, it breaks with the same "Undone tests" error.

dynamite-ready avatar Feb 25 '14 13:02 dynamite-ready

@dynamite-ready, I have solved my node unit problems by catching process UncaughtException at the very beginning of the file and add console.trace(err) to find out where the error is coming from. In this case, if some error occurs which I don't have and due to which the node unit thinks the test is finished, I can find the problem, fix, and make sure the error is handled. Once the test passes, I remove the process UncaughtException from the top of the file. Hope this helps.

kadishmal avatar Feb 26 '14 03:02 kadishmal