I've following code, which maps few fields from an object to another and copies value:

//Function params
var source= {"f1": "v1", "f2": "v2", "f3":"v3"};
var fieldsMapping = {"f1": "c1", "f2":"c2"};

//function definition starts
var copiedObj = {};    
for (var name in source) {
  if (source.hasOwnProperty(name)) { //Line X
    if(fieldsMapping[name]){
        copiedObj[fieldsMapping[name]] = source[name];
    }
  }
}

console.log(copiedObj); //outputs {c1: "v1", c2: "v2"}

I've written test case for this function in jest, with 100% line coverage but branch coverage displays Line X is not covered. As per Standards similar in TSLint, for-in loop should be followed by if condition.

Can anybody suggest on how to have a test case to increase branch coverage for this?

I've following code, which maps few fields from an object to another and copies value:

//Function params
var source= {"f1": "v1", "f2": "v2", "f3":"v3"};
var fieldsMapping = {"f1": "c1", "f2":"c2"};

//function definition starts
var copiedObj = {};    
for (var name in source) {
  if (source.hasOwnProperty(name)) { //Line X
    if(fieldsMapping[name]){
        copiedObj[fieldsMapping[name]] = source[name];
    }
  }
}

console.log(copiedObj); //outputs {c1: "v1", c2: "v2"}

I've written test case for this function in jest, with 100% line coverage but branch coverage displays Line X is not covered. As per Standards similar in TSLint, for-in loop should be followed by if condition.

Can anybody suggest on how to have a test case to increase branch coverage for this?

• javascript
• typescript
• unit-testing
• jestjs
• tslint

• Surely that line is redundant if you are looping over the properties of that object? – andy mccullough Commented Jun 27, 2019 at 17:08
• @andymccullough Yes, line is redundant, but if removed, the function is not TSLint pliant. – Vishnu Commented Jun 28, 2019 at 9:03
• 1 fair point, you could use Object.keys().forEach() which isn't as performant. But I wouldn't be worrying about code coverage on lines like that, if serves no value. Quality over quantity of tests every time.. – andy mccullough Commented Jun 28, 2019 at 10:53

2 Answers 2

The extra if is for the robustness of your code. Such robustness checks are not testable in a sensible way. This will happen in various other parts of your code as well - it often happens in switch statements where all possible cases are explicitly covered and an extra default case is added just to throw an exception or otherwise handle this 'impossible' situation. Or, think of assertion statements added to the code: Since assertions should never fail, you will strictly speaking never be able to cover the else branch that is hidden inside the assertion statement - how do you test that the expression inside the assertion is good to actually detect the problem you want it to?

Removing such robustness code and assertions is not a good idea, because they also help you detect undesired side effects of future changes. In the end you will have to make an informed decision (by looking at the coverage report in detail, not only the overall percentage) which statements/branches etc. of your code really need to be covered and which not.

And, as a final note, be aware that a high code coverage is not necessarily an indication that your test suite has a high quality. Your test suite has a high quality if it will detect the bugs in the code that could likely exist. You can have a test suite with 100% coverage that will not detect any of the potential bugs.

So you see it means this if is executed for all your test cases. That's why line coverage shows 100%. But to get branch coverage of 100% you need to make that if is not running for some specific test case.

hasOwnProperty returns false for properties inherited from prototype. So we will use that to make that line is not executed:

const base = { a: 1};
const source = Object.create(base, {b: 2, c: 3} );
const fieldsMapping = {a: 'aa', b: 'bb', c: 'cc'};

expect(yourFunction(source, fieldsMapping)).toEqual({ bb: 2, cc: 3 }); 

PS try not being addicted with 100%-coverage-goal.