Unit Testing Style

[clarkmalmgren]

Background

Following the style guide is great although there is a bit of a problem around the way a controller's activate() function works in practice. Let's say that I have a controller for populating users. Looks nice:

File: user_list.controller.js

/**
 * @ngInject
 */
function UserListController(UserService, $modal) {
  var vm = this;
  vm.users = {};

  activate();

  return vm;

  function activate() {
    UserService.list()
      .then(function(users) {
        vm.users = users;
      })
      .catch(function(err) {
        $modal.displayError('Unable to retrieve user list');
      })
  }
}

The Problem

The problem occurs when you try to test it. Your unit tests end up looking unwieldy due to the fact that the boilerplate code to get this setup. This is actually because what we have here is a testability flaw (see Flaw: Constructor does Real Work).

Here is an example with all of the boilerplate code: File: user_list.controller.spec.js

describe('UserListController', function() {
  beforeEach(module('app'));

  var $controller, $q;

  beforeEach(inject(function(_$controller_, _$q_){
    $controller = _$controller_;
    $q = _$q_;
  }));

  describe('on activate', function() {
      var mockPromise = {
        then : function () { return mockPromise; },
        catch : function () { return mockPromise; }
      };
      var mockUserService = {
        list = function() { return mockPromise; }
      };

    it('sets the user when service call succeeds', function() {
      var list = {};

      spyOn(deps.mockUserService, 'list').and.callFake(function() {
        var deferred = $q.defer();
        deferred.success(list);
        return deferred.promise;
      });

      var controller = $controller('UserListController', { UserService: mockUserService });

      expect(controller.list).toBe(list);
    });
  });
});

Note that for this test, there are nearly 15 lines of boilerplate code. Furthermore, this boilerplate code (especially the mock objects) are not shareable. There are several solutions to reduce the boilerplate code including ng-describe. But you still have a problem with repeated code in your unsharable mocks.

Proposed Solution

1. Create a "appName.mock" module
2. Create sharable mock implementations in a file patterned {{service_name}}.service.mock.js.
3. Place that file in the same folder (i.e. right next to) the real service
4. Build the mock such that EVERY action is a NOOP (does nothing) but will also not cause any runtime errors
5. Bind the activate() method to the vm variable so that activate may be called after construction.

   Example

File: user.service.mock.js

(function () {

  angular
    .module('app.mock')
    .factory('MockUserService', MockUserService);

  function MockUserService() {
    var NoOpPromise = {
      then : function() { return NoOpPromise; },
      catch : function() { return NoOpPromise; },
      finally : function() { return NoOpPromise; }
    };

    return {
      list: NoOpPromise
    };
  }
})();

This can now be used in any test that talks to a UserService.

File: user_list.controller.spec.js

describe('UserListController', function() {
  beforeEach(function() {
    module('app');
    module('app.mock');
  });

  var controller, $q;
  var mocks = {};

  beforeEach(inject(function(_$q_, $controller, MockUserService){
    $q = _$q_;
    mocks.MockUserService = MockUserService;
    controller = $controller('UserListController', { UserService:  MockUserService });
  }));

  describe('on activate', function() {
    it('sets the user when service call succeeds', function() {
      var list = {};

      spyOn(mocks.MockUserService, 'list').and.callFake(function() {
        var deferred = $q.defer();
        deferred.success(list);
        return deferred.promise;
      });

      controller.activate();

      expect(controller.list).toBe(list);
    });
  });
});

Note that this gets even better when you use tools like ng-describe.

The other thing that isn't shown here is that when I have a test for a function that gets called post-construction, I don't have to write any additional code related to the calling of the activate() method, I can just inject my values.

[mlassi]

I agree that tests in Angular that uses promises are the most painful to test and I would favor a simpler way to test it. However you can cut down on some of the boiler plate code and have it similar to below. Or put it in a separate mock service as you propose.

describe('on activate', function() { it('sets the user when service call succeeds', function() { var list = {}; var deferred = $q.defer(); spyOn(UserService, 'list').and.returnValue(deferred.promise);
deferred.resolve(list);

  controller.activate();
  scope.$digest();

  expect(controller.list).toBe(list);
});

[johnpapa]

activate testing is interesting. ideally we'd have an activate event so we can test before it happens. but we do not.

i'm curious to see where this issue leads. comments welcome

[MarcLoupias]

@clarkmalmgren

I don't see what you are testing in your controller. There is no logic in it.

For me, you are just testing that vm.users = users;, so the answer here is : this unit test is irrelevant. Just a waste of time.

Anyway, ng-describe is really good to avoid boilerplate. Love it !

[clarkmalmgren]

@MarcLoupias,

For this example, you are right, I am only testing that the Service is actually setting the appropriate value. But in practice, there is almost always more complex code in the activate path. For my internal specific use case that prompted this post I was doing both:

• Semi-complex failure handling
• Chaining multiple service calls together to get more detailed information

Also, just because the assertion is easy to write doesn't necessarily discount the utility of the unit test. This isn't a getter/setter test. This is ensuring that a Service call is invoked as expected and that the chained response (or part of the response) ends up being populated in the right place.

[johnpapa]

PR anyone?