3

I'm very new to unit testing and very excited about it. But I've uncertainty whether I should add defensive programming style or not to my production code. So, please see the following codes.

calclateFinalYearGPA.js (production code)

const _ = require('lodash'); const getGPAPoint = require('./getGPAPoint'); module.exports = function (gradings) { let totalPoint = 0; gradings.forEach((grading) => { const grade = grading.grading.grade.name; const point = getGPAPoint(grade); totalPoint += point; }); const finalYearGPA = _.round(totalPoint / gradings.length, 1); return finalYearGPA; }; 

The gradings is an array of objects which is pulled from the db, with the following structure.

"gradings": [ { "grading": { "gradeId": 3, "grade": { "gradeId": 3, "name": "A-" } } }, ... ] 

In my calculateFinalYearGPA.js, I loop through that array and extract grading.grade.name which is A- in the above structure, and pass it to getGPAPoint. It will return the decimal number.

So, now in my calculateFinalYearGPA.test.js test, I've add the following code which is test whether it fails when gradings is not an array.

const calculateFinalYearGPA = require('../../../utils/calculateFinalYearGPA'); describe('calculateFinalYearGPA', () => { it('should throw an exception if input is not an array', () => { const inputs = [1, 'a', null, undefined, {}]; inputs.forEach((i) => { expect(() => { calculateFinalYearGPA(i); }).toThrow(); }); }); }); 

So, now I need to add some defensive programming code to my production code.

if (gradings.constructor !== Array) throw new Error(`Expected to get an array, got ${typeof gradings}.`); 

The way I'm doing is correct? Should I test the gradings array has the proper structure as well? If so, I need to add some code that nothing with business logic is done but check the gradings array's structure. My production code will populate with that kind of code. Is it supposed to be?

What is the proper ways to test my calculateFinalYearGPA.js?

4
  • Please use a more focused question title and simplify your example down to what is relevant. Commented Aug 29, 2020 at 6:58
  • 2
    It is perfectly fine to test for errors, your tests are documenting your code so it's important to include edge cases. However here I think that you're going to far and fighting against the lack of type in JS. If a developer is misusing your code, it will never work, so you're not really catching bugs / helping a lot. -- However you could add a test for the scenario where gradings.length is 0, which currently throws an error where you might want to return 0. It's the kind of scenarios where it's useful to explicitely capture behaviour for edge cases. Commented Aug 29, 2020 at 12:50
  • Thanks @Arnaud. How about gradings array structure? Do I need to add some checking in production code and test it in my test as well? Commented Aug 29, 2020 at 13:17
  • 1
    Again, I think that's it's too much and the noise you'll add with the guard close is not worth it imo. Think about what you're trying to achieve. If the wrong type is passed here, it will fail no matter if it's tested. The purpose of the test is to prevent bugs / help developers to understand your code. By looking at your tests / production code, they will understand that it's an array. If they don't then their code will instantly fail, so I wouldn't even put the guard close. Think in term of behavior rather than technical details Commented Aug 30, 2020 at 16:42

1 Answer 1

1

Actually I don't think that correct, I mean to expect it will throw an error, throwing error logically means that something unexpected happened

so the right way to test the structure of the input is within calculateFinalYearGPA.js file, like the following:

module.exports = function (gradings) { if(!Array.isArray(gradings)) { throw new Error(`Expected to get an array, got ${typeof gradings}.`); } let totalPoint = 0; ... }; 

But in case you need to handle the error in jest or you might check it at least, I just recommend you to create a custom Error class, so you can manipulate as you wish, so:

class CustomError extends Error { constructor(code, message) { const fullMsg = message ? `${code}: ${message}` : code; super(fullMsg); this.name = code; this.code = code; this.message = fullMsg; } toString() { return this.message; } } 

Note: excpect(SOME_THING).toThrow() in jest will catch any error, so you have to be sure which error would fail the test and which not.

so now we have inside calculateFinalYearGPA.js file

module.exports = function (gradings) { if(!Array.isArray(gradings)) { throw new CustomError(799, `Expected to get an array, got ${typeof gradings}.`); } let totalPoint = 0; ... }; 

Finally for testing

const calculateFinalYearGPA = require('../../../utils/calculateFinalYearGPA'); describe('calculateFinalYearGPA', () => { it('should throw an exception if input is not an array', () => { const inputs = [1, 'a', null, undefined, {}]; inputs.forEach((i) => { try { calculateFinalYearGPA(i); } catch(e) { expect(e.code).toBe(799); // 799 is an optional number which refers the type mis-matched } }); }); }); 
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.