Continuous Improvement of Automation Tests for the Jago App

Testing can be a very effective way to show the presence of bugs, but it is hopelessly inadequate for showing their absence.

- Edsger W. Dijkstra

Tests are an important arsenal in a developer’s toolbox. They can detect breaking changes and help understand the expected behavior of the code. Sadly, they're often considered a chore and overlooked by strict quality checks applied to the codebase. Developers don’t pay as much attention to writing good code for test cases as they do for the functionality, resulting in a test suite that is full of test smells.

With the Jago App's ever-increasing codebase and new devs joining the teams, the need for good tests is apparent. Our focus is now not only on covering all the edge cases in the code but also on making the test readable. Test cases should be clear and concise so that anyone reading them can easily understand what they are testing.

In this article, I will focus on two key areas of improvement:

1. Removing noise from the test to make it readable.
2. Remove unnecessary use of mocking.

Let's take an example by improving the unit test of a react component. The component AdditionalNotes is expected to be used in a form by Jago Customer Service agents. The component should trim whitespaces from the content. The component implementation is not the focus here; we will just focus on testing.

Note: The code shown here is not the actual code we fixed but it’s similar.

import React from 'react';
import { Form } from 'antd';
import { UserEvent, userEvent } from '@testing-library/user-event';
import { render, renderHook } from '@testing-library/react';
import AdditionalNotes from '../AdditionalNotes';
import constants from '../constants';

const createMockForm = () => {
  const { result } = renderHook(() => Form.useForm());
  const form = result.current[0];

  return {
    ...form,
    getFieldInstance: jest.fn(),
    setFieldValue: jest.fn()
  };
};

describe('AdditionalNotes', () => {
  let user: UserEvent;

  beforeEach(() => {
    user = userEvent.setup();
  })


  it('calls onChange with the new value when typed into text area', async () => {
    const mockOnChange = jest.fn();
    const mockForm = createMockForm();

    const { getByRole } = render(
      <AdditionalNotes
        confirmationForm={mockForm}
        value=""
        onChange={mockOnChange}
      />
    );

    const textArea = getByRole('textbox');
    const testValue = 'New note text';

    await user.type(textArea, testValue);

    expect(mockOnChange).toHaveBeenCalledWith(testValue);
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      testValue
    );
  });

  it('calls onChange with the new value when typed into text area with whitespaces removed', async () => {
    const mockOnChange = jest.fn();
    const mockForm = createMockForm();

    const { getByRole } = render(
      <AdditionalNotes
        confirmationForm={mockForm}
        value=""
        onChange={mockOnChange}
      />
    );

    const textArea = getByRole('textbox');
    const testValue = '      New note text      ';

    await user.type(textArea, testValue);

    expect(mockOnChange).toHaveBeenCalledWith(testValue.trim());
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      testValue.trim()
    );
  });

  it('calls onChange with the new value when typed into text area with one whitespace at the end', async () => {
    const mockOnChange = jest.fn();
    const mockForm = createMockForm();

    const { getByRole } = render(
      <AdditionalNotes
        confirmationForm={mockForm}
        value=""
        onChange={mockOnChange}
      />
    );

    const textArea = getByRole('textbox');
    const testValue = 'New note text ';

    await user.type(textArea, testValue);

    expect(mockOnChange).toHaveBeenCalledWith(testValue);
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      testValue
    );
  });

  it('calls onChange with the new value with one space at end when typed into text area with two whitespace at the end', async () => {
    const mockOnChange = jest.fn();
    const mockForm = createMockForm();

    const { getByRole } = render(
      <AdditionalNotes
        confirmationForm={mockForm}
        value=""
        onChange={mockOnChange}
      />
    );

    const textArea = getByRole('textbox');
    const testValue = 'New note text  ';

    await user.type(textArea, testValue);

    const expectedValue = testValue.trim().concat(' ');

    expect(mockOnChange).toHaveBeenCalledWith(expectedValue);
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      expectedValue
    );
  });
});

As shown in the above example, even when the requirements are really simple the test cases are not. Someone reading it for the first time can get confused about what’s going on here. A few setup lines are repeated over and over again for every test case. The expectation is also not very clear.

Improvements

Improving Readability

First, let us figure out what can be improved. We can see the setup part till the component rendering is repeated in every test.

Let’s use jest’s beforeEach method to set it all up:

// snip...

describe('AdditionalNotes', () => {
  let user: UserEvent;
  let additionalNotes: React.JSX.Element;
  const mockOnChange = jest.fn();
  let mockForm: FormInstance<any>;
  let textArea: HTMLElement;

  beforeEach(() => {
    user = userEvent.setup();
    mockOnChange.mockReset();
    mockForm = createMockForm();

    additionalNotes = (
      <AdditionalNotes
        taskConfirmationForm={mockForm}
        value=""
        isEnable={true}
        onChange={mockOnChange}
      />
    );

    render(additionalNotes);

    textArea = screen.getByRole('textbox');
  });

  it('calls onChange with the new value when typed into text area', async () => {
    const testValue = 'New note text';

    await user.type(textArea, testValue);

    expect(mockOnChange).toHaveBeenCalledWith(testValue);
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      testValue
    );
  });

  // remove the same common code from other test cases

By separating setup logic, our test cases look less confusing.

Improving Mocking

It’s a common practice to mock objects that are passed to components while testing. For example, if you want to test a service, you mock the repository instance. This is a general tendency of developers to mock most of the things while testing.

But mocking is a double-edged sword. If you are not careful, you can find yourself in mocking hell where everything is mocked and no real testing is done. Mocking can also result in false positive test cases where a contract breaks with the third-party library, but we are not able to detect it due to mocks. Developers also mock the library to check some conditions just to increase the code coverage. It’s highly debatable what to mock and what not to. We should consider not using mock unless it’s absolutely required.

We should consider not using mock unless it’s absolutely required.

We can see that we are mocking FormInstance. We asked ourselves, “Do we really need this mock here? Can’t we use this without mocking?” The reason for mocking is to spy on the setFieldValue call. Is there a way to check if the correct form value is set?

After thinking over it we figured out a simple way to check the value in the form. We can use Form’s getFieldValue to get the value and use it to check with the expected value.

// snip...

const createMockForm = (): FormInstance<any> => {
  const { result } = renderHook(() => Form.useForm());
  return result.current[0];
};

// snip...

it('calls onChange with the new value when typed into text area', async () => {
    const testValue = 'New note text';

    await user.type(textArea, testValue);

    expect(mockOnChange).toHaveBeenCalledWith(testValue);
    expect(mockForm.getFieldValue(constants.formKey.note)).toBe(testValue);
  });

// same changes in other test cases

Now we are actually using the library functions to test our component. This removed the overhead of mocking, and if there is a breaking change in the library, our test case will catch that.

Final touch-ups

Now, the test cases are looking concise. But there is still room for more improvements. Let’s look at the test case below:

it('calls onChange with the new value with one space at end when typed into text area with two whitespace at the end', async () => {
    const testValue = 'New note text  ';

    await user.type(textArea, testValue);

    const expectedValue = testValue.trim().concat(' ');

    expect(mockOnChange).toHaveBeenCalledWith(expectedValue);
    expect(mockForm.setFieldValue).toBeCalledWith(
      constants.formKey.note,
      expectedValue
    );
});

Although the description says that it is acceptable to have one whitespace at the end of text, but the code looks confusing. Why are we trimming and concatenating the space? We can fix this by inlining the test and expected values.

it('calls onChange with the new value with one space at end when typed into text area with two whitespace at the end', async () => {
    const endingWithOneWhiteSpace = 'New note text ';

    await user.type(textArea, 'New note text  ');


    expect(mockOnChange).toHaveBeenCalledWith(endingWithOneWhiteSpace);
    expect(mockForm.getFieldValue(constants.formKey.note)).toBe(endingWithOneWhiteSpace);
});

We can further improve it by moving the constant value endingWithOneWhiteSpace to higher scope, wrapping mockForm.getFieldValue(constants.formKey.note) in a function and updating the descriptions.

import React from 'react';
import { Form, FormInstance } from 'antd';
import { UserEvent, userEvent } from '@testing-library/user-event';
import { screen, render, renderHook } from '@testing-library/react';
import AdditionalNotes from '../AdditionalNotes';
import constants from '../constants';

let mockForm: FormInstance<any>;

const createMockForm = (): FormInstance<any> => {
  const { result } = renderHook(() => Form.useForm());
  return result.current[0];
};

function notesFieldValue() {
  return mockForm.getFieldValue(
    constants.formKey.note
  );
}

describe('AdditionalNotes', () => {
  let additionalNotes: React.JSX.Element;
  const mockOnChange = jest.fn();
  let textArea: HTMLElement;
  let user: UserEvent;
  const noWhiteSpaceValue = 'New note text';
  const endingWithOneWhiteSpace = 'New note text ';

  beforeEach(() => {
    user = userEvent.setup();
    mockOnChange.mockReset();
    mockForm = createMockForm();

    additionalNotes = (
      <AdditionalNotes
        taskConfirmationForm={mockForm}
        value=""
        isEnable={true}
        onChange={mockOnChange}
      />
    );

    render(additionalNotes);

    textArea = screen.getByRole('textbox');
  });


  it('Input value with no whitespace', async () => {
    await user.type(textArea, noWhiteSpaceValue);

    expect(mockOnChange).toHaveBeenCalledWith(noWhiteSpaceValue);
    expect(notesFieldValue()).toBe(noWhiteSpaceValue);
  });

  it('Input value with whitespaces in begining and end', async () => {
    await user.type(textArea, '      New note text      ');

    expect(mockOnChange).toHaveBeenCalledWith(endingWithOneWhiteSpace);
    expect(notesFieldValue()).toBe(endingWithOneWhiteSpace);
  });

  it('Input value with one whitespace at end', async () => {
    await user.type(textArea, endingWithOneWhiteSpace);

    expect(mockOnChange).toHaveBeenCalledWith(endingWithOneWhiteSpace);
    expect(notesFieldValue()).toBe(endingWithOneWhiteSpace);
  });

  it('Input value with two whitespaces at end', async () => {
    await user.type(textArea, 'New note text  ');

    expect(mockOnChange).toHaveBeenCalledWith(endingWithOneWhiteSpace);
    expect(notesFieldValue()).toBe(endingWithOneWhiteSpace);
  });
});

The final result of all the refactoring is readable test cases that are not using any extra mocking than required. Now, when someone reads the test case, they are not overwhelmed by the setup and they can easily see what is the expected behaviour of the component.

Conclusion

Writing good test cases is difficult, but even small refactoring, like moving common setup logic in one place, can improve it. Developers change their mindset and don’t look at it as something that is just part of the Definition of Done. They should adhere to the same code quality as their code. Engineers at DKatalis are slowly trying to improve test suites and make automation tests more reliable.

(Shout out to Stanly Lau for guiding and helping my clan understand these test smells and how to remove them.)

Find more interesting tips and tricks from the DKatalis team on our blog, and don’t forget to hit that follow button!