(WIP) Level-up Jest & Enzyme

[vojtechszocs]

Dependency changes

• upgrades:
  • jest, jest-cli, ts-jest from 21.x to 24.x (latest)
  • enzyme-adapter-react-16 from 1.12.1 to 1.13.2 (latest)
• additions:
  • @types/jest
  • enzyme-to-json

TS config changes

• add compilerOptions.types referencing Jest and Jasmine typings

Jest config changes

• extract config to jest.config.js
• use jsWithTs preset which reflects the allowJs:true setting in tsconfig.json
• run before-tests.js via setupFilesAfterEnv following Enzyme setup recommendation
• run ts-jest with isolatedModules:true (otherwise, the updated Jest runner gets stuck)
• use snapshotResolver.js that allows test vs. snapshot co-location

Test vs. Snapshot co-location

// packages/console-demo-plugin/src/foo.tsx

import * as React from 'react';

const Bar: React.SFC<any> = (props) => {
  return <div className="bar">
    Hello {props.text}!
  </div>
};

export const Foo: React.SFC = () => {
  return <div className="foo">
    <Bar text="Bar" />
  </div>
};

// packages/console-demo-plugin/src/__tests__/foo.spec.tsx

import * as React from 'react';
import { shallow } from 'enzyme';
import toJson from 'enzyme-to-json';

import { Foo } from '../foo';

it('renders correctly', () => {
  const wrapper = shallow(<Foo />);
  expect(toJson(wrapper)).toMatchSnapshot();
});

// packages/console-demo-plugin/src/__tests__/foo.snap.tsx

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`renders correctly 1`] = `
<div
  className="foo"
>
  <Bar
    text="Bar"
  />
</div>
`;

Jest snapshot files have the same extension as their test counterparts.

foo.tsx
__tests__/foo.spec.tsx
__tests__/foo.snap.tsx

---

/cc @christianvogt @spadgett

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vojtechszocs
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: spadgett

If they are not already assigned, you can assign the PR to them by writing /assign @spadgett in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vojtechszocs]

@spadgett There are currently some post-upgrade Enzyme related errors, I need to look into those.

/cc @alecmerdler

[vojtechszocs]

@mareklibra @rawagner FYI, this PR customizes Jest snapshot file resolution, putting snapshot files next to test files.

[christianvogt]

@vojtechszocs please setup the snapshotSerializers for enzyme-to-json to simplify tests.

[openshift-ci-robot]

@vojtechszocs: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vojtechszocs]

Note to self: add typescript-snapshots-plugin to this PR.

[christianvogt]

@vojtechszocs are you going to complete this soon? The dev console needs snapshots. Otherwise we'll need to do some updates so our plugin functions in the interim

[alecmerdler]

What is the value of snapshot tests versus normal unit tests? It seems to unnecessarily couple the DOM output to the implementation, doesn't test the component logic, and makes it harder to follow TDD.

[christianvogt]

Snapshots are useful for testing data transformations. ie input -> json output

I also find snapshots to be useful for components which are rendering html elements which have CSS that couples the rules to the structure. eg. using > and other nested selectors.

But simply snapshotting all react components does not make sense.

[vojtechszocs]

@vojtechszocs are you going to complete this soon? The dev console needs snapshots. Otherwise we'll need to do some updates so our plugin functions in the interim

@christianvogt I'm currently finalizing my work on testing actual plugins, i.e. validating their extensions, as part of the Console build (like we discussed before). I'll get back to this PR right after that.

This PR is still WIP since I've encountered some weird Enzyme-related errors once I bumped Jest & related deps from 21.x to 24.x so I'll look into that.

What is the value of snapshot tests versus normal unit tests?

@alecmerdler Snapshot tests ensure that serialization of the given object yields the same results every time you run those tests.

The object being serialized can be anything, but developers often associate snapshot tests with React components. Based on the premise that a React component is essentially a function taking inputs (props, state, context) and producing rendered markup (JSX), snapshot tests guard against regressions that might occur in component's rendered JSX.

So, for React components, it's a way to ensure rendered UI doesn't change unexpectedly. If it does, and the change was intentional/expected, you can update the snapshot to make the test pass again.

It seems to unnecessarily couple the DOM output to the implementation, doesn't test the component logic, and makes it harder to follow TDD.

React components should be snapshot-tested in isolation, i.e. rendered in jsdom that emulates "real" DOM env. Snapshots of React components should not include rendered markup of their children, for example:

const Bar: React.SFC<any> = (props) => {
  return <div className="bar">
    Hello {props.text}!
  </div>
};

const Foo: React.SFC = () => {
  return <div className="foo">
    <Bar text="Bar" />
  </div>
};

Foo component snapshot should look like this:

<div className="foo">
  <Bar text="Bar" />
</div>

Component logic can be tested by simulating interaction with component's DOM elements, i.e. using Enzyme wrapper. You are testing a React component as a unit, instead of testing its individual methods.

As for TDD, I think it's best to complement it with different techniques.

But simply snapshotting all react components does not make sense.

Fully agreed. Just like 100% test coverage just for the sake of having it doesn't make sense.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-ci-robot]

@vojtechszocs: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp	4f208d8	link	/test e2e-gcp
ci/prow/e2e-gcp-console	4f208d8	link	/test e2e-gcp-console
ci/prow/analyze	4f208d8	link	/test analyze
ci/prow/kubevirt-plugin	4f208d8	link	/test kubevirt-plugin

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

[openshift-bot]

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

[openshift-ci-robot]

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.