0
\$\begingroup\$

Recently I needed to expand the exports in a file to all methods in order to allow for Jest tests to cover more of the code (without using Rewire). Beforehand, only the functions used by other production files were exported, meaning unit tests only covered the exposed methods and not the non-exported ones. I'm wondering if there are any disadvantages to exporting all of the functions within a file - or if there is a better way that this code can be written such that I can unit test all of the methods within individually. I made a simple example of the code in question:

Calculator.js


const sum = (addend1, addend2) => addend1 + addend2;

const difference = (minuend, subtrahend) => subtrahend - minuend;

const product = (multiplicand, multiplier) => multiplicand * multiplier;

const quotient = (dividend, divisor) => dividend / divisor;

const findAbsDifference = (number1, number2) => difference(Math.abs(number1), Math.abs(number2));

export {
sum,
product,
quotient,
findAbsDifference
} // difference is not exported, because it's not used in any other file

Calculator.test.js

import * as Calculator from "./Calculator.js";

describe('Calculator', () => {

  test('sum(5, 5)', () => {

    const addend1 = 5;

    const addend2 = 5;

    const expectedSum = 10;

    expect(Calculator.sum(addend1, addend2)).toEqual(expectedSum);

  });

  test('findAbsDifference(-5, 2)', () => {

    const minuend = -5;

    const subtrahend = 2;

    const expectedDiff = 3;

    expect(Calculator.findAbsDifference(minuend, subtrahend)).toEqual(expectedDiff);

   });
/**
*
* I'd like to test the difference(minuend, subtrahend) method here, 
* although I am unable to do this unless it is exported.
* Is there any disadvantage to exporting this function when it's only used in the test
* and within findAbsDifference which is in the same file?
*
**/
  test('product(11, 2)', () => {

    const multiplicand = 11;    

    const multiplier = 2;

    const expectedProduct = 22;

    expect(Calculator.product(multiplicand, multiplier)).toEqual(expectedProduct);

   });

  test('quotient(20, 5)', () => {

    const dividend = 20;    

    const divisor = 5;

    const expectedQuotient = 4;

    expect(Calculator.quotient(dividend, divisor)).toEqual(expectedQuotient);

   });
});

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ Jun 30, 2021 at 19:39

1 Answer 1

0
\$\begingroup\$

Well in your specific case, you are implementing the functionality of a calculator. The calculator has a public interface of what it can do. I do not think it makes sense to hide difference just because it is not used anywhere. You can still export it regardless of it being used or not.

Also, you seem to be testing functions (implementation) instead of functionality or behavior. Have your tests assert that the calculator does something you want it to do, not that the function of a calculator does something you want it to do. It's a subtle difference, but it makes your tests much more readable when you are dealing with more complex behavior.

This might be just my personal preference, but I think it makes tests also more readable: split the setup, execution and assertion into three different parts. This has many names, but is mostly known by given-when-then.

Taking all this into account, if you want your calculator to also subtract, export the difference function and restructure the tests:

...
  test('sums two numbers', () => {
    // given
    const addend1 = 5;
    const addend2 = 5;

    // when
    const sum = Calculator.sum(addend1, addend2)

    // then
    expect(sum).toEqual(10);
  });

  test('finds the difference between two numbers', () => {
    // given
    const number1 = 5;
    const number2 = 5;

    // when
    const diff = Calculator.difference(number1, number2)

    // then
    expect(diff).toEqual(0);
  });
...

It would be a totally different story if your difference function was a helper function specific to the calculator. Then the function would not be a part of the interface of the calculator and you should not test it directly, as it is a bad practice to test "private" functions, because you tie the tests to the implementation. This means that whenever you will refactor, the tests will also break. You can make the tests for a function while you develop it, but delete these tests afterwards, as they are not useful anymore. You should test the private functions indirectly, by testing the "publicly visible" interface, which is your main concern in the first place.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you @Blaž Mrak, that perfectly answers my question and gives me a good direction to move forwards! I like the separation of given-when-then, makes tests concise in their intended outcomes. Appreciated! \$\endgroup\$ Jul 12, 2021 at 3:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.