Unit-testing function

Question

I've got this javascript function which was written a bit ad-hoc and I'm not really sure how to go about refactoring and improving it.

It's basically an implementation of draft Unit-Testing/1.1 specification.

// Runs the object as a test. The async paramater is an optional function
// To be passed in if the test is to be run async.
// notRoot is an internal paramater
var run = function(object, async, notRoot) {
    // if its the root call then cache the async func for us in _pushTests
    if (!notRoot) {
        func = async;
    }
    // tests to run
    var tests = [];
    // Assert constructor to be used is either on the object or from assert
    var Assert = object.Assert || assert.Assert;
    var failures = 0;
    // Push the tests to the tests array
    _pushTests(object, tests);
    var len = tests.length;
    // function that runs the test
    var testRunner;
    // If async Do I have to document it? It's full of the hacks.
    if (async) {
        // results object stores the calls to pass/fail/error
        var results = {};
        results.passes = [];
        results.fails = [];
        results.errors = [];
        // tests passed in the test object
        var testsPassed = 0;
        testRunner = function(val, key) {
            // Local assert object
            var _assert = new Assert;
            // Forward the mute property to the assert
            if (object.mute === true) {
                _assert.mute = true;    
            }
            // cache pass, fail & error
            var _pass = _assert.pass;
            var _fail = _assert.fail;
            var _error = _assert.error;
            // Wrap pass. Push the pass massge to the results object
            _assert.pass = function(message) {
                _pass.apply(_assert, arguments);
                results.passes.push(message);
                // If an assert passed after done then throw an error in 
                // assert.error
                if (doneCalled) {
                    _assert.error(new Error(
                        "assertion passed after done was called" + 
                        "for test : " + key + " and message was : " +
                        message
                    ));
                }
            }
            // Wrap fail. Push the fail message to the results object
            _assert.fail = function(message) {
                _fail.apply(_assert, arguments);
                results.failures.push(message);
                // Throw an error if assertion failed after done has been
                // called
                if (doneCalled) {
                    _assert.error(new Error(
                        "assertion failed after done was called" + 
                        "for test : " + key + " and message was : " +
                        message
                    ));
                }
            }
            // Wrap error. Log calls to error
            _assert.error = function(error) {
                _error.apply(_assert, arguments);
                results.errors.push(error);
            }
            // Done has not been called
            var doneCalled = false;
            var done = function() {
                // If its not been called then set it to be called
                if (!doneCalled) {
                    doneCalled = true;
                    // Increment the number of tests that have passed
                    // If we have passed all then call the async function
                    // with the results
                    if (++testsPassed === len) {
                        async(results, object.name);    
                    };
                } else {
                    // Done has already been called thrown an error
                    _assert.error(new Error(
                        "done already called for test : " + key
                    ));
                }
            };
            // Try running the test function.
            try {
                val(_assert, done);    
            } catch(e) {
                // If a failure occurs log it when its an AssertionError
                if (e instanceof assert.AssertionError) {
                    console.log("failed : " + e.message);
                } else {
                    // and throw if its another error
                    _assert.error(e);
                }
                failures++;
            }
        };
    } else {
        // the test runner takes the test function as a paramater
        testRunner = function(val) {
            // create a local assert
            var _assert = new Assert;
            // If we want to mute it then pass mute to the assert
            if (object.mute === true) {
                _assert.mute = true;    
            }
            // Try the test
            try {
                val(_assert);
            } catch(e) {
                // If it throws an assertion error then log it
                if (e instanceof assert.AssertionError) {
                    console.log("failed : " + e.message);
                } else {
                    // Other error thrown so pass it to assert.error
                    _assert.error(e);    
                }
                failures++;
            }
        };
    }
    // For each test run it.
    _.each(tests, testRunner);
    // If the object had a name and was not async then print a message
    // saying we've finishe
    if (object.name && !async) {
        var string = "Completed " + len + 
            " tests. Finished : " + object.name +
            " with " + failures + " tests failing";
        console.log(string);        
    }
    // Return the count of failures (Kind of useless for async).
    return failures;
};

For more information the full file can be found here. It also relies on assert object which is documented here.

There are some (limited) unit tests and they can be run in the browser here.

Please tell me if more information is needed.

I was going to write some proper high level documentation on how to use it because the commonJS spec is a bit vague. I'll edit when that's done.

Answer 1

I would start out by cleaning the code along these lines:

1. Refactor variable names and functions so as to limit the use comments (code doesn't' lie, comments sometimes do)
2. Use better descriptive names (fx key/val relates to test's in your code but has me thinking of object literals).
3. Split the function in smaller functions so they are easier to test and comprehend by them self:

i.e:

testRunner = (async) ? asyncTestRunner : functionTestRunner;

followed by the the functions them self

// 
function asyncTestRunner(descriptiveParameterNames) {
  :
}
// 
function functionTestRunner(anotherDescriptiveParameter) {
  :
}

Try to build up your code as a series of progressions, each building logically on the former without needing to many commentary's along the way, in a similar manner as a well written textbook.