Skip to content

Instantly share code, notes, and snippets.

@Sequoia
Last active August 8, 2019 14:01
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Sequoia/e0e615a9340966ebd280dd65cd6e61f7 to your computer and use it in GitHub Desktop.
Save Sequoia/e0e615a9340966ebd280dd65cd6e61f7 to your computer and use it in GitHub Desktop.
Why not to export & test internal functions

Why Is It Bad To Export Functions Exclusively For Testing?

ℹ️ Summary: Test the destination, not the route.

We have a module that doubles numbers. It does this by bit-shifting the supplied number once left.

// double.js

// exported to allow testing...
export const shiftLeft = (num, places) => num << places;

export const double = x => shiftLeft(x, 1);

We test it and it works!

import {double, shiftLeft} from 'double.js';

// test double
assert.equal(20, double(10));
assert.equal(30, double(15));

// test shiftLeft
assert.equal(2, shiftLeft(1,1));
assert.equal(4, shiftLeft(1,2));

Trouble in Paradise

An vexatious colleage sees that our double function uses bit shifting and decides to have a bit of fun with us. She adds the following test:

assert.equal(18014398509481982, double(Number.MAX_SAFE_INTEGER)); // trollface.webp
// 1 TEST FAILED ========================
// AssertionError [ERR_ASSERTION]: 18014398509481982 == -2

Huh?! -2? It looks like she's revealed an issue with our use of bit shifting. No matter! We can use a multiplying function instead:

// double.js

// exported to allow testing...
export const shiftLeft = (num, places) => num << places;
const mul = (multiplicand, multiplier) => multiplicand * multiplier;

export const double = x => mul(x, 2);

Our tests now pass! We clean up our file by removing unused functions...

 // double.js

-// exported to allow testing...
-export const shiftLeft = (num, places) => num << places;
 const mul = (multiplicand, multiplier) => multiplicand * multiplier;

 export const double = x => mul(x, 2);

But now our tests fail! Removing shiftLeft causes the tests to fail, despite the fact that it's not relied upon anywhere.

What's Up?

Our tests are failing, so one of two things is true. Either:

  1. Our module really is broken (the tests work) or
  2. Our module is not broken, but the tests are still failing

Number 2 indicates that we are testing something unrelated to whether or not the module works.

So what do we do now? We update the tests, even though our code is working and nothing is broken. This is a waste of time, which is added to the time we wasted writing the test in the first place.

Conclusion

Test that when you call double(2) it returns 4, don't test what method it used to arrive at that solution, because for the purposes of testing the double function, it doesn't matter. If the module is supposed to get from point a to point b, and initially it does it by going "right, forward, left" then we change it to go "left, forward, right" but it still ends up at point b, the tests should not fail. We are concerned with the destination, not how we got there.

@Sequoia
Copy link
Author

Sequoia commented Aug 8, 2019

If it is a module that has 100s of private functions and only one function exposed

Another person (sth) brought this up, and I must ask: have you ever actually seen a module with "hundreds of private functions" with only one function exposed? I agree that my "guideline" above is not absolute and doesn't scale infinitely, but general guidelines like "put related functions in the same file" aren't nullified by fantasy scenarios like "well what if I have 100,000 related functions?? Should I put them all in the same file?"

My counter argument you can find above: "what was gained by testing reverse, mocking, and rewriting all the tests after?"

Your (@umayr) and STH's comments are relevant and good points tho–I agree that my initial pass at writing this "guideline" was way too blunt and ended up unclear & bad. TY for pointing this out! I want to revise to "don't export every function just for testing" and note that the level of complexity and how many places the function is used help answer that question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment