Writing better tests

Current problems:

false negatives - tests break when functionality isn't broken (i.e. snapshots)
tests don't work well with React Hooks

The fundamental problem is not with the tool used (Enzyme in our case), it's with the
testing methodology.

Good test principles:

No false positives (test should break if behavior is broken).
No false negatives (test should not break if implementation is changed, but behavior in not broken).

What in our existing codebase makes these principles difficult to follow?

relying heavily on shallow rendering* (good unit test coverage is expensive to write and maintain, provides diminishing returns)
testing implementation instead of behavior

* more arguments against shallow rendering: https://kentcdodds.com/blog/why-i-never-use-shallow-rendering

Examples of implementation detail testing:

overusing snapshots
asserting on components internals (e.g. `wrapper.instance` in Enzyme)

Solution:

treat components as 'black boxes' - instead of testing state, test the behavior visible on the outside (which matters for the end user).

import { mount } from "enzyme";

it("should render correct", () => {
  expect(
    mount(<FAQItem question="Question?" answer="Answer" />)
  ).toMatchSnapshot();
});

// lots of false negatives, test breaks after any change to the implementation

import { render } from "@testing-library/react";
import "@testing-library/jest-dom/extend-expect";

it("renders critical UI elements", () => {
  const { getByLabelText } = render(
    <FAQItem question="Question?" answer="Answer" />
  );
  const button = getByLabelText(/expand content/i);
  const content = getByLabelText(/expanded content/i);
  expect(button).toBeInTheDocument();
  expect(content).toBeInTheDocument();
});

// asserts on existence of critical ui elements - test doesn't break after refactoring

import { shallow } from 'enzyme';

it("should call toggleAnswer correctly", () => {
  const instance = shallow(
    <FAQItem question="Question?" answer="Answer" />
  ).instance();

  instance.toggleAnswer();
  expect(instance.state.isOpen).toBe(true);
  instance.toggleAnswer();
  expect(instance.state.isOpen).toBe(false);
});

// uses shallow rendering 
// asserts on implementation detail

import { fireEvent, render } from '@testing-library/react';
import "@testing-library/jest-dom/extend-expect";

it("expands when clicked", () => {
  const { getByLabelText } = render(
    <FAQItem question="Question?" answer="Answer" />
  );
  const button = getByLabelText(/expand content/i);
  const content = getByLabelText(/expanded content/i);

  expect(button).toHaveAttribute("aria-expanded", "false");
  expect(content).toHaveAttribute("aria-hidden", "true");
  fireEvent.click(button);
  expect(button).toHaveAttribute("aria-expanded", "true");
  expect(content).toHaveAttribute("aria-hidden", "false");
});

// - fully renders the component 
// - asserts on behavior that matters to the end user

import styled from 'styled-components';
import { toMatchDiffSnapshot } from 'snapshot-diff';
expect.extend({ toMatchDiffSnapshot });

const MyInput = styled.input`
	padding: ${p => p.iconPosition === 'left' ? '12px 12px 12px 45px' : '12px 45px 12px 12px'};
`

it('iconPosition prop change affects render', () => {
    expect(
      render(<MyInput iconPosition="left" />)
    ).toMatchDiffSnapshot(
      render(<MyInput iconPosition="right" />)
    );
  });

BONUS SLIDE: better use case for snapshots

How do we enforce consistent good practices on the project?

Style guide and constant vigilance (not ideal - additional costs during onboarding and code review)
Using an opinionated tool which disallows bad practice

React testing library

no shallow rendering (encourages integration over unit tests, supports hooks out of the box)
no access to state - forces to write meaningful tests that will survive refactoring
API strongly encourages accessible selectors (with data attribute as an escape hatch): https://testing-library.com/docs/dom-testing-library/api-queries

POC:

https://gitlab.carnext.io/carnext/frontend/carnext-ui/merge_requests/304/diffs