• Open Source plugin
  • no-useless-matcher-to-be-defined
  • no-useless-matcher-to-be-null
  • no-mixed-expectation-groups
  • no-long-arrays-in-test-each
• Add more power to your ESLint rules with TypeScript

Recently I joined a project with many typical microservices, where each one is a standalone NestJS application with many unit and e2e tests.

The good thing - developers write really many tests there, covering all the API endpoints, validation, transactions, etc.

The bad thing - tests were looking messy, so we decided to

• fix the most popular mistakes and spread best practices across all developers of different microservices
• somehow control it, to not allow writing messy tests in the future

To achieve the second point in an automatic way, it was decided to write custom ESLint Rules so that developers immediately notified right in their IDE when tests look bad, and which is more important - build is failed if our best practices are not followed.

Below, I would like to show the most interesting rules that I've created and then applied to this work project.

# Open Source plugin

eslint-plugin-proper-tests - let's quickly look into some of the rules from this plugin.

# no-useless-matcher-to-be-defined

This rule disallows using expect(...).toBeDefined() matcher when it is obvious that a variable is always defined.

I was surprised how many times I saw this in the codebases. And I was pretty sure that there should be a rule for this, but seems like not.

As an OSS contributor, I understand that creating new libs/repos is not always a good idea, so I tried to propose this rule for eslint-plugin-jest (btw, it's awesome!), but for some reason didn't even get a response (and at the time of writing of this post, this is still the case).

See original proposal eslint-plugin-jest#1616

Ok, let's get back to the rule. Here are a couple of real-life examples of bad code:

const response = await request.post('/api/some-resource');

expect(response).toBeDefined();
expect(response.status).toBe(200);

What's the problem with this code? The problem is that expect(response).toBeDefined(); always passes and is really the useless check here.

The type of response variable is Response. It physically can not be undefined. Does it make sense to check it with .toBeDefined()? No.

Reported error:

Type of "response" variable is "Response", it can not be undefined. It is useless to check it with `.toBeDefined()` matcher.

So let's just remove this line:

const response = await request.post(`/api/some-resource`);

- expect(response).toBeDefined();
expect(response.status).toBe(200);

Another example that is hard to spot looking into the code for the first time:

const response = await request.post(`/api/some-resource`);

// ...

const userCacheKeys = await redisClient.keys('USER_*');

expect(userCacheKeys).toBeDefined();

The purpose of this test was to check that after calling API endpoint, user's data is cached to Redis. But does it really test anything?

Absolutely no, because the type of userCacheKeys is string[]. So redisClient.keys() always returns an array, which is always "defined".

Again, we have expectation that always passes, and which is worse, we don't test anything. Whether Redis has these keys or not - tests don't really catch it.

The proper change for this case is the following:

const userCacheKeys = await redisClient.keys('USER_*');

- expect(userCacheKeys).toBeDefined();

+ const cacheValues = await redisClient.mGet(userCacheKeys);
+
+ expect(cacheValues).toEqual(['cache-value-1', 'cache-value-2']);

So, now we check the number of keys and exact values returned from Redis. When at some point developers will do a change that will lead to adding new keys or removing keys, such test will catch it, which is what we want.

Read below to understand how to use TypeScript types to add more power to your ESLint rules

# no-useless-matcher-to-be-null

Similar rule to the previous one, this rule complains where not.toBeNull() is used when it shouldn't:

const user = repository.findByIdOrThrow(123); // return type is `User`

expect(user).not.toBeNull();

From types point of view, user can not be null, so this check is useless and will be reported by this ESLint rule. Just remove it and replace with a proper expectation.

# no-mixed-expectation-groups

This rule reports an error if expectations for different variables are mixed with each other.

Here is a very simplified example of a bad test:

const response = await request.post(`/api/some-resource`);

const entity = await repository.getById(123);

expect(response.status).toBe(201);
expect(entity).toMatchObject({...});
expect(response.headers).toMatchObject({...});
expect(response.body).toMatchObject({...});

In real life when you have multiple lines the things look much worse. So what is the problem here? The problem is that we are checking different variables, mixing them with each other.

Instead, it's better to have "groups" of expectations. The same code can be rewritten like:

const response = await request.post(`/api/some-resource`);

expect(response.status).toBe(201);
expect(response.headers).toMatchObject({...});
expect(response.body).toMatchObject({...});

const entity = await repository.getById(123);

expect(entity).toMatchObject({...});

# no-long-arrays-in-test-each

This rule disallows the usage of long arrays in test.each() calls.

The following code is bad:

test.each([
  {
    description: 'test case name #1',
    inputValue: 'a',
    expectedOutput: 'aa',
  },
  {
    description: 'test case name #2',
    inputValue: 'b',
    expectedOutput: 'bb',
  },
  {
    description: 'test case name #3',
    inputValue: 'c',
    expectedOutput: 'cc',
  },
  {
    description: 'test case name #4',
    inputValue: 'd',
    expectedOutput: 'dd',
  },
  {
    description: 'test case name #5',
    inputValue: 'e',
    expectedOutput: 'ee',
  },
  {
    description: 'test case name #6',
    inputValue: 'f',
    expectedOutput: 'ff',
  },
])('$description', ({ clientCountry, expectedPaymentMethod, processorName }) => {
  // ...
});

Consider extracting such long arrays to a separate files with for example .data.ts postfix.

The following code is much better:

// some-service.data.ts
export type TestCase = Readonly<{
  description: string;
  inputValue: string;
  expectedOutput: string;
}>;

export const testCases: TestCase[] = [
  {
    description: 'test case name #1',
    inputValue: 'a',
    expectedOutput: 'aa',
  },
  {
    description: 'test case name #2',
    inputValue: 'b',
    expectedOutput: 'bb',
  },
  {
    description: 'test case name #3',
    inputValue: 'c',
    expectedOutput: 'cc',
  },
  {
    description: 'test case name #4',
    inputValue: 'd',
    expectedOutput: 'dd',
  },
  {
    description: 'test case name #5',
    inputValue: 'e',
    expectedOutput: 'ee',
  },
  {
    description: 'test case name #6',
    inputValue: 'f',
    expectedOutput: 'ff',
  },
];

and now test is more readable:

test.each(testCases)('$description', ({ inputValue, expectedOutput }: TestCase) => {
  // ...
});

# Add more power to your ESLint rules with TypeScript

eslint-plugin-proper-tests plugin uses TypeScript to provide more accurate results and get the power of TypeScript to understand variables types.

To enable it properly, you need to configure ESLint to work with TypeScript:

// .eslintrc.js

module.exports = {
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "project": true,
    "tsconfigRootDir": __dirname,
  }
}

Now, let's quickly look into how to use TypeScript types to improve your ESLint rules with an example of no-useless-matcher-to-be-defined rule.

Read official docs on how to use typescript-eslint for typed rules here

In our rule we need to use typeChecker service:

const services = ESLintUtils.getParserServices(context);
const typeChecker = services.program.getTypeChecker();

>> Look into the code

Then, to understand if a variable is always defined, we need to:

• get the type of the variable
• check if it can be undefined

To get the declaration type of variable, we use the following code:

// argumentNode - is a node of the variable inside expect(variable) call

const symbol = services.getSymbolAtLocation(argumentNode);

const declarationType = typeChecker.getTypeOfSymbolAtLocation(symbol!, symbol!.valueDeclaration!);

It's a bit tricky, but the good thing you shouldn't do it often. What we did is got the declaration type of the variable.

Now, we need to check if it "contains" undefined sub-type inside (like string | undefined).

For this, I use isTypeFlagSet helper:

const isVariableCanBeUndefined = isTypeFlagSet(
  declarationType,
  ts.TypeFlags.Undefined
);

which under the hood gets all the union types of a variable and by bit mask checks if undefined is there:

const getTypeFlags = (type: ts.Type): ts.TypeFlags => {
  let flags: ts.TypeFlags = 0;

  for (const t of tsutils.unionTypeParts(type)) {
    flags |= t.flags;
  }

  return flags;
};

const isTypeFlagSet = (type: ts.Type, flagsToCheck: ts.TypeFlags): boolean => {
  const flags = getTypeFlags(type);

  return (flags & flagsToCheck) !== 0;
};

Look to the final code here

This is how you can use TypeScript types to improve your ESLint rules and make them more accurate. Possibilities are endless!

I encourage you to try these rules in your projects and see how they can improve the quality of your tests.

If you like this plugin and the work I'm doing, consider giving it a Star on GitHub.

Find this interesting? Let's continue the conversation on Twitter.