Cleaning up Our Tests: Last Tweaks

Before we finish up with testing, we're going to make two more changes to make our tests more useful and less likely to fall over in the future.

First, just testing that the rendered.state.forks.length property is equal to 30 is a good start, but it would be nice to make sure that all 30 of those got rendered correctly by React. Each fork is rendered in our code using a <p> tag, so you might think we could write something like this in the last test:

const forks = TestUtils.scryRenderedDOMComponentsWithTag(rendered, 'p');
expect(forks.length).toEqual(30);

Sadly, that won't work: Jest will find 31 <p> tags in the page and fail the test. This happens because our page already has one <p> tag on there showing our breadcrumbs, so we have the 30 <p> tags from the forks plus one more from the breadcrumbs.

There are a few solutions here. Option 1: remove the breadcrumbs. This would work, but means giving up a nice feature of our app. Option 2: render the commits, forks and pulls using a different tag name, such as <li>. This would also work, and doesn't require losing a feature, so this is certainly possible.

But there's a third option, and it's the one we'll be using here: scryRenderedDOMComponentsWithClass(). This lets you find all tags based on their CSS class name rather than their tag name. This class name doesn't actually need any style information attached to it, so all it takes is to adjust the renderCommits(), renderForks(), and renderPulls() methods of our Detail component from this:

src/pages/Detail.js

return (<p key={index}>…

…to this:

src/pages/Detail.js

return (<p key={index} className="github">…

Back in the test code, we can now use scryRenderedDOMComponentsWithClass() to pull out exactly the things we mean, regardless of whether they are <p>, <li> or anything else:

__tests__/Detail-test.js

const forks = TestUtils.scryRenderedDOMComponentsWithClass(rendered, 'github');
expect(forks.length).toEqual(30);

There's just one more thing we're going to do before we're finished with testing, which is to take a cold, hard look at this line:

__tests__/Detail-test.js

rendered.setState({mode: 'forks', forks: testData});

This is another example of code that works great but is still less than ideal. This time it's because we're breaking the fourth wall of object-oriented programming: our test is forcing a new state on our component rather than making a method call. If in the future you update the Detail component so that setting the forks state also calls some other code, you'll have to copy that code into your test too, which is messy and hard to maintain.

The correct solution here is to use an OOP technique called encapsulation, which means our test shouldn't try to peek into and adjust the internals of our Detail component. Right now all our tests do exactly that: they read and write the state freely, which isn't very flexible going forward. I'm going to fix one of these with you right now, but you can fix the others yourself if you want to.

We need a new method in the Detail component that updates the component state. This can then be called by our test to inject the saved JSON cleanly rather than by forcing a state. Realistically all we need is to move one line of code out of the fetchFeed() method and wrap it into its own method.

Find this line:

src/pages/Detail.js

this.setState({ [type]: response.body });

That uses a computed property name along with the response body from SuperAgent in order to update our component state. We're going to make that a new method called saveFeed(), which will take the type and contents of the feed as its parameters:

src/pages/Detail.js

saveFeed(type, contents) {
    this.setState({ [type]: contents });
}

You can now call that straight from the fetchFeed() method:

src/pages/Detail.js

if (!error && response) {
    this.saveFeed(type, response.body);
} else {
    console.log(`Error fetching ${type}.`, error);
}

If you've made the correct changes, the two methods should look like this:

src/pages/Detail.js

fetchFeed(type) {
    if (this.props.params.repo === '') {
        // empty repo name – bail out!
        return;
    }

    const baseURL = 'https://api.github.com/repos/facebook';
    ajax.get(`${baseURL}/${this.props.params.repo}/${type}`)
        .end((error, response) => {
            if (!error && response) {
                this.saveFeed(type, response.body);
            } else {
                console.log(`Error fetching ${type}.`, error);
            }
        }
    );
}

saveFeed(type, contents) {
    this.setState({ [type]: contents });
}

With that new saveFeed() method in place, we can update the fifth test to use it rather than forcing a state on the component:

__tests__/Detail-test.js

it('fetches forks from a local source', () => {
    const rendered = TestUtils.renderIntoDocument(
        <Detail params={{repo: ''}} />
    );

    const testData = require('./forks.json');
    rendered.saveFeed('forks', testData);
    rendered.selectMode('forks');

    const forks =
        TestUtils.scryRenderedDOMComponentsWithClass(rendered, 'github');

    expect(forks.length).toEqual(30);
});

That shows you the technique of encapsulating your component's state behind a method call, which will make your code much more maintainable in the future. Yes, it's extra work in the short term, but it will pay off when you aren't up at 3am trying to debug an obscure problem!

I'm not going to go through and adjust the rest of the tests, because that's just pointless repetition – you're welcome to consider it homework if you want to try.

Copyright ©2016 Paul Hudson. Follow me: @twostraws.