SAN-6094 Code review comments

Author: damienrunnable

client/directives/components/lists/branchTestSelector/branchTestSelectorController.js

@@ -39,9 +39,7 @@ function BranchTestSelectorController(
   BTSC.selectCommit = function (commit) {
     BTSC.commit = commit;
 
-    if (!calculateHistoricalTestResult.isPassed(BTSC.commit.test) &&
-        !calculateHistoricalTestResult.isFailed(BTSC.commit.test) &&
-        !calculateHistoricalTestResult.isUnknown(BTSC.commit.test)) {
+    if (!calculateHistoricalTestResult.isValidState(BTSC.commit.test)) {
       BTSC.updateInstance();
     }
 

client/services/calculateHistoricalTestResults.js

@@ -21,6 +21,9 @@ function calculateHistoricalTestResult(
     isUnknown: function(state) {
       return state === UNKNOWN;
     },
+    isValidState: function(state) {
+      return state === PASSED || state === FAILED || state === UNKNOWN;
+    },
     addResults: function (tests) {
       tests.forEach(function(test) {
         if (test && keypather.get(test, 'build.stop') !== jesusBirthday) {

test/unit/directives/components/lists/branchTestListController.unit.js

@@ -180,13 +180,13 @@ describe('BranchTestListController'.bold.underline.blue, function () {
       $scope.$digest();
       // No test found
       expect(branch.commits.models[0].test).to.equal(null);
-      // Test found but build exit time of jessus birthday
+      // Test found but build exit time of jesus birthday
       expect(branch.commits.models[1].test).to.equal('unknown');
       // Build failed is true
       expect(branch.commits.models[2].test).to.equal('failed');
       // // Build passed but exit code > 0
       expect(branch.commits.models[3].test).to.equal('failed');
-      // Application stop is jessus birthday
+      // Application stop is jesus birthday
       expect(branch.commits.models[4].test).to.equal('unknown');
       expect(branch.commits.models[5].test).to.equal('passed');
     });

test/unit/services/calculateHistoricalTestResults.unit.js

@@ -32,7 +32,7 @@ describe('calculateHistoricalTestResult'.bold.underline.blue, function () {
       }];
       calculateHistoricalTestResult.addResults(tests);
 
-      expect(calculateHistoricalTestResult.isUnknown(tests[0].testState)).to.be.truthy;
+      expect(calculateHistoricalTestResult.isUnknown(tests[0].testState)).to.be.true;
     });
 
     it('addResults build failed', function () {
@@ -48,7 +48,7 @@ describe('calculateHistoricalTestResult'.bold.underline.blue, function () {
       }];
       calculateHistoricalTestResult.addResults(tests);
 
-      expect(calculateHistoricalTestResult.isFailed(tests[0].testState)).to.be.truthy;
+      expect(calculateHistoricalTestResult.isFailed(tests[0].testState)).to.be.true;
     });
 
     it('addResults exit code not 0', function () {
@@ -64,7 +64,7 @@ describe('calculateHistoricalTestResult'.bold.underline.blue, function () {
       }];
       calculateHistoricalTestResult.addResults(tests);
 
-      expect(calculateHistoricalTestResult.isFailed(tests[0].testState)).to.be.truthy;
+      expect(calculateHistoricalTestResult.isFailed(tests[0].testState)).to.be.true;
     });
 
     it('addResults application stop epoch', function () {
@@ -80,7 +80,7 @@ describe('calculateHistoricalTestResult'.bold.underline.blue, function () {
       }];
       calculateHistoricalTestResult.addResults(tests);
 
-      expect(calculateHistoricalTestResult.isUnknown(tests[0].testState)).to.be.truthy;
+      expect(calculateHistoricalTestResult.isUnknown(tests[0].testState)).to.be.true;
     });
 
     it('addResults passed', function () {
@@ -96,7 +96,7 @@ describe('calculateHistoricalTestResult'.bold.underline.blue, function () {
       }];
       calculateHistoricalTestResult.addResults(tests);
 
-      expect(calculateHistoricalTestResult.isUnknown(tests[0].testState)).to.be.truthy;
+      expect(calculateHistoricalTestResult.isPassed(tests[0].testState)).to.be.true;
     });
   });
 });