Unit tests for a single JavaScript function

Question

Here is the question: What is the better way to unit test this specific function? Should all the test items be included in one test or is it better to break up the tests so each test is testing for something more specific?

Function being tested:

$scope.saveFailure = function (response) {
 var errs = [];
 response.data.forEach((entry) => errs.push("options." + entry.dataField));
 if (errs.length > 0) $scope.$emit('datafields:errors', errs);
 $scope.gotoTop();
 $scope.processing = false;
 };

Method 1: Multiple Unit Tests

var mockFailureResponse;
beforeEach(function () {
 mockFailureResponse = {
 data: [],
 };
});
it('saveFailure should set processing to false', function () {
 $scope.processing = true;
 $scope.saveFailure(mockFailureResponse);
 expect($scope.processing).toBe(false);
});
it('saveFailure should call goToTop()', function () {
 spyOn($scope, 'gotoTop');
 $scope.saveFailure(mockFailureResponse);
 expect($scope.gotoTop).toHaveBeenCalled();
});
it('saveFailure should emit datafield errors when present', function () {
 spyOn($scope, '$emit');
 mockFailureResponse = {
 data: [{dataField: "field"}],
 };
 $scope.saveFailure(mockFailureResponse);
 expect($scope.$emit).toHaveBeenCalledWith('datafields:errors', ['options.field']);
});
it('saveFailure should not emit datafield errors non are present', function () {
 spyOn($scope, '$emit');
 $scope.saveFailure(mockFailureResponse);
 expect($scope.$emit.calls.count()).toEqual(0);
});

Method 2: Single Unit Test

it('saveFailure should handle failed requests', function () {
 spyOn($scope, '$emit');
 let mockFailureResponse = {
 data: [],
 };
 $scope.saveFailure(mockFailureResponse);
 expect($scope.$emit.calls.count()).toEqual(0);
 mockFailureResponse = {
 data: [{ dataField: "field" }],
 };
 $scope.saveFailure(mockFailureResponse);
 expect($scope.$emit).toHaveBeenCalledWith('datafields:errors', ['options.field']);
 spyOn($scope, 'gotoTop');
 $scope.saveFailure(mockFailureResponse);
 expect($scope.gotoTop).toHaveBeenCalled();
 $scope.processing = true;
 $scope.saveFailure(mockFailureResponse);
 expect($scope.processing).toBe(false);
});

Answer 1

I would say that, of course, the first version when you split your tests into granular logical blocks is much better and far more explicit.

There is even a practice to have a single assertion per test which sometimes is too strict of a rule, but it helps to follow the Arrange Act Assert test organizational pattern.

• nweg: I also agree with you. Does the concept of trying to make a function do "one thing" also play here? If the test is testing multiple things I could see that as not a good thing. Each function/test should be focused on testing one thing. Do you agree with that?
• alecxe: @nweg absolutely. You would also get a clearer feedback as a bonus - in case a test fails, the output would be more useful and explicit. Thanks.
• nweg: Have and other recommendations to clean up this code?
• alecxe: @nweg just one minor thing and it's probably just my thing - I usually have a newline before an expect() call just to visually distinguish expectations from the rest of the test code. Also, not sure if you need to repeat saveFailure in your it descriptions - we tend to start our it descriptions with should all the time.
• nweg: How would you name one of these tests then? How would you know what function you are testing?

Answer 2

You can use array.map to create errs instead of creating an array and pushing to it.

const errs = response.data.map(entry => `options. ${entry.dataField}`)

With that out of the way, there's only two things that can happen with this code:

• No errors emitted, app goes to top, processing is false.
• Error emitted, app goes to top, processing is false.

Suggesting you create 2 tests, one for each branch and testing all 3 things. The function is small enough, there's no need to get too complicated.