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.