Integration test for database access

Question

This integration test involves database access and hence opening and closing a database connection. Does it look correct? I am concerned about ensuring the the database connection is closed should the test fail.

"use strict";
describe("MyCtorFunction", function () {
 describe("myMethod", function () {
 var _db = null,
 _testContext = null;
 beforeEach(function () {
 _testContext = {};
 new DbHelper().openConnection(function (err, dbConnection) {
 if (err) {
 throw err;
 }
 _db = dbConnection;
 });
 waitsFor(function () {
 return _db;
 }, "establishing a connection to the database.", 5000);
 });
 afterEach(function () {
 waitsFor(function () {
 return _testContext.assertions.callCount === 1;
 }, "waiting for the assertions to be called.", 5000);
 runs(function () {
 if (_db) {
 _db.close();
 _db = null;
 }
 });
 });
 it("should do something", function () {
 runs(function () {
 //arrange
 _testContext.assertions = assertions;
 spyOn(_testContext, "assertions").andCallThrough();
 //act (_testContext.assertions invoked as callback)
 new MyCtorFunction(_db).myMethod(_testContext.assertions);
 //assert
 function assertions(err, config) {
 expect(config).toNotBe(null);
 //etc.
 }
 });
 });
 });
});

Answer 1

From a once over and reading some Jasmine docs:

• I am not a big fan of _db and _testContext, I see no reason for the underscores.

• It is not guaranteed that _db will be set in
  new MyCtorFunction(_db).myMethod(_testContext.assertions);,
  so you should check for that prior to calling.

• You did not tell us what database (API) you are using, so we cannot tell whether you close the connection properly.

• I wish you had provided us the code in assertions, it would have helped with the review

• JsHint could find nothing wrong

• I would advise you to make sure an assertion fails and put a log statement in your db closing code, this way you can be assured that the db connection will be closed. Though from reading the docs you should be fine.

• This:

  waitsFor(function () {
   return _testContext.assertions.callCount === 1;
  }, "waiting for the assertions to be called.", 5000);
  

  Seems a bit paranoid, afterEach is guaranteed to run after your assertions if I read your code well. If I am right then you can clean up your code tremendously since you will not need _testContext any more.

• I hope MyCtorFunction and myMethod are not the true names in your code, those are horrible names ;)

All in all I dont really like your code, I blame Jasmine for that. I dont like their approach to naming functions nor their approach to asynchronous processing.

Update:

I realized on the subway that in my mind, the waiting for the db and the waiting for the test to finish ought to be part of the test, not part of the setup/teardown. This approach will definitely simplify since you wont need runs anymore in afterEach, plus you will not need testContext any more.

I cannot test this, but you should get the picture.

"use strict";
describe("MyCtorFunction", function () {
 describe("myMethod", function () {
 var db;
 beforeEach(function () {
 new DbHelper().openConnection(function (err, dbConnection) {
 if (err) {
 throw err;
 }
 db = dbConnection;
 });
 });
 afterEach(function () {
 if (db) {
 db.close();
 db = null;
 }
 });
 it("should do something", function () {
 //Wait for the green light
 waitsFor(function () {
 return db;
 }, "establishing a connection to the database.", 5000); 
 //Start acting!
 runs(function () {
 //arrange
 spyOn(_testContext, "assertions").andCallThrough();
 //act (assertions invoked as callback)
 new MyCtorFunction(_db).myMethod( assertions );
 //assert
 function assertions(err, config) {
 expect(config).toNotBe(null);
 //etc.
 }
 });
 //Wait for the action to be done
 waitsFor(function () {
 return assertions.callCount === 1;
 }, "waiting for the assertions to be called.", 5000); 
 //Done 
 });
 });
});

Finally, from a library perspective, check out https://github.com/derickbailey/jasmine.async

• 52d6c6af: Thanks! The waitsFor(... return _testContext.assertions.callCount === 1; ...) is required for test runner to wait until assertions have been run asynchronously before closing the db conn. Aware of stylistic camps on _. It can help with readability denoting variable scoping. Introduced testContext to do trickery with the assertions function (and maintain the arr/act/ass ordering in the test). Names changed in this code from the actual source. The beforeEach waitsFor checks _db is set no? Can you recommend another async test runner/something that might help me improve the code?
• konijn: @Ben the beforeEach will only wait 5 seconds, that might still fail.. I will do some research on better async test runners, they must be out there..