Don't name your tests 'it("should ...")'

Published on 2024-03-17 by Aleksey

Common pattern

If you’ve ever worked with tests in a language like TypeScript, chances are you’ve seen something like this:

describe("readFileToString", () => {
  it("should throw when file is missing", () => {
    // ...
  });

  it("should throw on empty file name", () => {
    // ...
  });

  it("should return file content as string", () => {
    // ...
  });

  // ...
});

This contrived example illustrates the pattern that seems very popular among the developers: naming your tests should ....

I seriously struggle to understand the popularity. There must be a logical reason for such a pattern to merge, but I just don’t see one. It’s almost like everybody uses it because everybody else does.

Not evident enough?

The single biggest problem for me with the should prefix is that it’s completely redundant. The very existence of a test already implies that a piece of logic should work in a certain way, exactly how the test documents it. I don’t need a prefix to remind me about that. It’s a tautology. If I omit the prefix

it("throws when file is missing", () => {
  // ...
});

I’m losing zero useful information. In fact, the test name is now more concise and reads naturally.

The inevitable consequence of this point is that should becomes a visual noise if you read tests long enough. At least I have definitely noticed about myself that over time I tend to semi-conciously skip the prefix in order to reach the meaningful part of the name quicker. Any unnecessary visual noise in code is bad because it increases the cognitive load on the reader, who is already burdened enough due to the nature of the job.

The visual noise becomes more apparent in the test runner output. A whole column is wasted on a word that’s repeated in 100% of the tests and brings zero value.

PASS apps/tests/recurrent-payment-endpoint.spec.ts
  Recurrent payment endpoint
    request is authorized
      amount is negative
        ✓ should return HTTP 200
        ✓ should return failed transaction
        ✓ should send transaction metric
        ✓ should log request details

Beyond unit tests

Another problem with should is that it kinda fits well only with it() and mostly in unit tests. I don’t know who invented the idea that tests should read like natural language and therefore start with “it should”, but the benefit of this hasn’t exactly been very apparent, to put it mildly.

Once you start writing integration or e2e tests, the context of the test becomes broader and higher-level. The meaning of it may no longer be obvious for each particular test, especially if it’s nested within multiple describes. Consequently, should also loses this connection to something being tested and becomes even less useful. At best, the need to follow the “it should” pattern becomes a hinderance in places where a more concise name would work better.

To give an example, imagine I’m testing a complex flow that is triggered by an external event. Following the pattern I’d write:

describe("Authorization confirmation", () => {
  // ...

  describe("'order placed' event is received", () => {
    it("should confirm authorization", () => {});

    it("should emit 'auth confirmed' event", () => {});
  });

  // ...
});

The fixture name is the name of the flow: “Authorization confirmation”. Then we read “order placed event is received” followed by “should confirm authorization”. But what exactly does that mean? The event confirms an authorization? This vaguely makes sense but also not really because the event itself cannot confirm anything, the service does that. However the service is not mentioned anywhere, going up the chain of describes we stop only at the flow name. But the flow itself cannot confirm anything either, it’s just a generic name of a set of processes.

What about the second test it("should emit 'auth confirmed' event")? It’s even more confusing because it basically says that the order placed event should emit the auth confirmed event. But the event itself cannot emit anything! It’s just a trigger that launches a flow that may or may not eventually emit the output event. At best such test name in this context is misleading.

Just don’t

The point of all this is that the pattern is too opinionated and may require unnecessary effort to follow if you also care about concise and clear test names in good English. Just drop the prefix, re-word the names and ideally also use a less opinionated alternative to it(), for example in Jest it’s just a synonim for test().

Given that, the previous example could be reworded:

describe("Authorization confirmation", () => {
  // ...

  describe("'order placed' event is received", () => {
    test("authorization is confirmed", () => {});

    test("'auth confirmed' event is emitted", () => {});
  });

  // ...
});

Without it it’s now possible to use passive voice in test names. You might not want to overuse it because it can sound too beaurocratic, however in certain situations it leads to the shortest test name.

The very first example, re-worded without the passive voice:

describe("readFileToString", () => {
  test("throws when file is missing", () => {
    // ...
  });

  test("throws on empty file name", () => {
    // ...
  });

  test("returns file content as string", () => {
    // ...
  });

  // ...
});

We no longer have it() but it is still self-evident from the narrow context of a unit test.

Summary

Don’t follow the trend blindly. Test names don’t have to match a rigid structure if readability suffers. They don’t have to imitate the natural language, for programmers who are used to code it simply brings inconvenience and non-tech people won’t read your code anyway.

What matters is a balance between readability, clear meaning and conciseness. Use a more generic schema test("'does something' or 'something is done'") everywhere and you’ll have less small things to worry about (if you care).