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