Juri Strumpflohner

RSS

An Example of Test-Driven Bugfixing

Author profile pic
Juri Strumpflohner
Published

Unfortunately many people still don’t use a test-driven approach during development. That’s sad given the bunch of advantages automated tests give you. One of my favorite is regression testing. Since I just spotted a bug in my codebase, I’ll take the opportunity to quickly document the bugfixing approach I’m usually applying.

The bug is the one I described in my previous post. Please read it to be able to properly follow this one here.

Intro

I basically have the following function defined in my ajax.js file of our JavaScriptMVC extension library.
$.Model.findAllPaged = function (params, currentPage, pageSize, success, error) {
return $.ajax({
url: this.resolveUrl(this.baseUrl || this.shortName),
type: "GET",
dataType: "json " + resolveClassNameForConverter(this.shortName) + ".models", //"account.models" for autowiring
data: $.extend(params, { currentPage: currentPage, pageSize: pageSize }),
success: success,
error: error,
fixture: "-restFindAll" // uses $.fixture.restDestroy for response.
});
};
This function wraps a call with paging support to our server API. It can be invoked as follows:
Test.PersonAccount.findAllPaged(
{},
currentPage,
numPages,
function (resultData) { ... },
function (error) { ... });
Pretty self-explanatory.

Create a failing test

The bug is basically in line 6 of the findAllPaged function as mentioned in my previous post . So the first step before even going to touch the buggy code is to create a failing test. This is extremely important as it
  • proves the presence of the bug
  • gives a positive feedback once the bug has been resolved
  • functions as a regression test, preventing the introduction of the bug in the future by someone else
For the test I'm using QUnit , written by the jQuery team and already included by default in JavaScriptMVC. The test looks as follows:
test("Calling Model.findAllPaged should not modify the passed parameter object", 1, function () {
//arrange
$.fixture["-restFindAll"] = function (settings, cbType) {
var result = {
page: 3,
total: 7,
records: 200,
rows: [
{
id: 5,
firstname: "juri"
},
{
id: 6,
firstname: "juri123"
}
]
};

return [result];
};

//act
stop(500);
var param = { currentPage: 0 };
Test.PersonAccount.findAllPaged(param, 1, 10,
function (data) {
equals(param.currentPage, 0);
start();
},
function (error) {
ok(false, "Error: " + error.responseText);
start();
}
);
});

Some explanation to the test’s anatomy:

  • line 1 - this is the header of the test case specifying a test name in the form of a written string that should explain the purpose of the test. The "1" between the actual test function and the test name specifies the number of expected asserts. This is useful as JavaScript involves async actions and callbacks. So we want to make sure our test fails if no callbacks are being called for instance.
  • line 3 - this line defines a fixture that substitutes the actual ajax call that is being made by the function under test. This is a build-in feature provided by the JavaScriptMVC framework. It could be done manually as well . This is important as we don't want to actually contact the server during the execution of the test.
  • line 24 - stop(timeout-milliseconds) instructs the test-case to halt and only continue its execution when start() is invoked again. Read more here .
  • line 26 - the actual invocation of the function to be tested.
  • line 28 - the assert
Run the test...

...it fails. Perfect.

Now, go and fix the bug

To fix the bug I substitute the buggy line as already explained in my previous post . Basically, exchanging line 6 of the findAllPaged function with the following:
...
data: $.extend({}, params, { currentPage: currentPage, pageSize: pageSize }),
...

Re-run the test

Re-running the test...

.. success . The bug is fixed!
Refactor (if necessary) and re-run all the tests again to see if your modification didn't break any other tests.

That's it!