Rubric for unit test pull requests

Narrative Context: Learning to write unit tests has been a bit of a mind-twist for me. Initially it was really frustrating and unsatisfying, but eventually became pretty fun. Learning how to read Pull Requests for new unit test files, or edits to existing ones, has compounded the mind-twist and therefore required a back-to-basics What Are Tests Supposed To Be Doing Again consideration. This is my reminder rubric for reading these PRs. It de facto doubles as a guide to writing tests that I can trust, maintain, and hand to others with confidence.

Use: The order below is my own prescribed priority, and not all are mandatory. The lower priority steps are ones that can be ~unlocked~, so to speak, as higher priority steps becomes breezier for writer/reviewer. I’m using the pseudo-code based off of a few frameworks for example, but the principles hopefully can be extended beyond that.

Let’s begin.

isGonnaBeOkay.calledWithMatch( "a-okay" ) // passes

/* enacting your pernicious testing interrogation: */

isGonnaBeOkay.calledWithMatch( "def-not-okay" ) // should fail
/* sufficient */
someFunc.getCall( 0 ).arg[ 1 ].should.equal( false )

/* ideal */
const isEnabledForUser = someFunc.getCall( 0 ).arg[ 1 ];
isEnabledForUser.should.equal( false );
/* more like an integration test */
describe( "Some desired behavior that requires context", () => {} );

/* ideal and explicit unit test */
describe( "Missing parameter and resulting code path", () => {} );
/* lots of code, hard to tell what's meant to happen */

const someFunction = stub().returns( "hello" );
const someOtherFunction = stub();
const someThirdFunction = spy();
const helloFactory = stub().returns( () => someFunction() );

describe( "helloFactory was called once, does stuff", () => {} );

/* simple, easy to modify as-needed if module changes/grows */

const someFunction = () => "hello";
const helloFactory = stub().returns( () => someFunction() );

describe( "helloFactory was called once, does stuff", () => {} );

Are the expected results variable-ized and far away from the assertion, or hardcoded and/or nearby?

/* in large modules, can be hard to see what's actually breaking */

enumFunc.should.return( someExampleObjectToScrollTo );

/* easier to see where/why something might have broken */

enumFunc.should.return( {
  firstValue: "one",
  secondValue: "two",
  thirdValue: "three" // test fails by returning "gamma"
} );

How to consider coverage: Automated code coverage is not necessarily code coverage. It’s there as a guard rail to verify that each line was hit by the code path. If there are few assertions for the crucial steps along the way, why bother writing test files at all? Coverage reports can be very helpful for a reviewer if some code path wasn’t hit that a human describe block claims it did. Of course, 100% test coverage is great. Just be sure that behavioral coverage is as high as possible, as well.