4
\$\begingroup\$

I'm trying to master testing. I struggle, conceptually, as to what should be tested.

  • Is this Jest / Enzyme test sufficient for this react component?

  • Is is too much?

  • Are there better practices that I should be implementing?

Here is my component.

import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';
import {AverageText} from './index';

const Row = styled.div`
  display: flex;
  justify-content: center;
`;

const AvgSpeed = ({tests}) => {
  const downloadSpeeds = extractSpeeds(tests, 'download');
  const uploadSpeeds = extractSpeeds(tests, 'upload');
  const downloadAverage = getAverage(downloadSpeeds);
  const uploadAverage = getAverage(uploadSpeeds);
  return (
    <Row>
      <AverageText label="download" value={downloadAverage} />
      <AverageText label="upload" value={uploadAverage} />
    </Row>
  );
};

AvgSpeed.propTypes = {
  tests: PropTypes.object,
};

//returns average from array of numbers fixed at 1
function getAverage(array) {
  return (
    array.reduce((a, b) => {
      return a + b;
    }) / array.length
  ).toFixed(1);
}

//extracts float value from object property string value
function extractSpeeds(obj, property) {
  return Object.keys(obj).map(item => parseFloat(obj[item][property]));
}

//export to test
export {getAverage, extractSpeeds};

export default AvgSpeed;

My test

import React from 'react';
import AvgSpeed, {getAverage, extractSpeeds} from './AvgSpeed.js';
import {shallow, mount} from 'enzyme';
import renderer from 'react-test-renderer';

describe('AvgSpeed Component', () => {
  const inputObj = {
    1: {
      download: '22 mbs',
      upload: '9 mbs',
    },
    2: {
      download: '24 mbs',
      upload: '18 mbs',
    },
  };

  it('matches the snapshot', () => {
    const tree = renderer.create(<AvgSpeed tests={inputObj} />).toJSON();
    expect(tree).toMatchSnapshot();
  });

  describe('getAverage function', () => {
    it('returns the average from an array (fixed at 1)', () => {
      let input = [1, 2, 3];
      let output = '2.0';

      expect(getAverage(input)).toBe(output);

      //2,3,4
      input = input.map(n => (n += 1));
      output = '3.0';
      expect(getAverage(input)).toBe(output);

      //4,5,6
      input = input.map(n => (n += 2));
      output = '5.0';
    });
  });

  describe('extractSpeeds function', () => {
    it('return an array of float numbers', () => {
      const downloadIn = 'download';
      const uploadIn = 'upload';

      const downloadOut = [22, 24];
      const uploadOut = [9, 18];

      expect(extractSpeeds(inputObj, downloadIn)).toEqual(downloadOut);
      expect(extractSpeeds(inputObj, uploadIn)).toEqual(uploadOut);
    });
  });
});
\$\endgroup\$
5
+50
\$\begingroup\$

There are two distinct parts in your test:

  • Testing the rendering of AvgSpeed component.
  • Testing the logic of getAverage() and extractSpeeds().

This separation between components and logic is good. I would suggest separating them even more by moving the getAverage() and extractSpeeds() functions to separate file, which you can then test completely independently from the component.

The component rendering test however is dependent on the logic. That's not good:

  • You're re-testing the logic through the component test, which is duplication.
  • When there's a change in logic, the component test will also break, resulting in more work maintaining the tests.

So your component test really is more of an integration test, not a pure unit test. There are basically two ways to improve on that:

  • Change the component. Instead of the component importing these functions and calling them, perhaps you could perform the calculation outside of the component and just pass in download and upload props.
  • Change the test. Instead of letting the component call actual getAverage() and extractSpeeds() you could replace them with mock implementations. Though, the less mocking you need, the better. So, you could consider combining the two functions into one, which will be easier to mock out.

Usually the first approach is better and requires no extra tooling. If you decide to go the mocking route, my personal choice is the rewire library, of which there exist various versions for various environments (e.g. for Babel).

Regarding specific tests

Avoid temporary variables when you can write the values inline. Instead of:

expect(extractSpeeds(inputObj, downloadIn)).toEqual(downloadOut);

prefer:

expect(extractSpeeds(inputObj, "download")).toEqual([22, 24]);

Avoid logic in tests. Logic is error-prone. While hard-coded values are a big no-no in normal code, in test code they're usually OK. Instead of:

input = [1, 2, 3];
input = input.map(n => (n += 1));

just hard-code it:

input = [2, 3, 4];

Try to write one assertion (expect) per it() block. This way you can give a more descriptive name for each scenario you want to test.

Don't simply repeat the same test multiple times. For example, you're testing the getAverage with [1,2,3], [2,3,4], [3,4,5] - it's essentially the same test: three sequential numbers. Try to come up with more fundamentally different scenarios:

  • different lengths of array.
  • positive/negative numbers, fractional numbers, zero.
  • array of similar numbers.

Think of the edge cases:

  • What if there is no data? (e.g. empty object or array)
  • What kind of input data would be problematic? (e.g. negative numbers)
\$\endgroup\$
2
  • \$\begingroup\$ To make sure I understand correctly, you are suggesting that I separate getAverage() and extractSpeeds() function by moving them to a separate file. In addition, I should perform the calculation outside of the component and just pass in download and upload props. In this case, should I be importing these functions into a parent component where I first receive the data from my api call? From there, I test the functions separately as well as that parent component? \$\endgroup\$ Apr 25 '18 at 22:14
  • \$\begingroup\$ Yeah. It seems like it would be better to import them to the parent component. And then it'll be the question of how do you test the parent component... which I can't tell, as I don't know what the parent looks like. \$\endgroup\$ Apr 26 '18 at 6:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.