Fixing a bug using React Testing Library

Join me in this post as I attempt to fix an elusive bug using tests to first spot the issue and then to provide a safety net which will protect me from introducing regressions as I fix it.

The words-search-game has a logical flow which goes like this:
When users press the “Edit” button in order to make some changes to the word bank it gives them the option to either remove a word or add a new word. If they choose to do either, when pressing the “Play” button to resume playing the game it will notify them that the game is about to reset and they should confirm it, but if they haven’t changed anything and decided to resume the game, it will immediately resume with no confirmation asked.

Now there is a bug here and its flow is - click to edit => change something => click on play to resume the game => click cancel on the confirmation => click on play again.
When you do that you get right back in the game with no confirmation required, although you’ve changed the word bank. Oh dear god.

Let’s fix that.

The “Edit” and “Play” are a single button which acts as a toggle between the modes. Its label is determined by the game mode state and it has a single callback for a click: onToggleGameMode:

function onToggleGameMode() {
       const shouldGoIntoEditMode = gameMode !== EDITING_GAME_MODE;
       let exitEditMode = () => dispatch(enterIdleMode());
       // If the words number are different we need to reset the game and
       // not just resume it
       if (wordsNumber !== words.length) {
           setWordsNumber(words.length);
           exitEditMode = onRefreshGame;
       }
       shouldGoIntoEditMode ? dispatch(enterEditingMode()) : exitEditMode();
   }

This callback is not something to brag about, and since this part of the code is not even tested (yeah, I know, bad me!) I will try to read it and figure out what I did there:
We have a boolean which defines whether or not the player needs to go into “edit” mode, and it does that by querying the game mode state. Then we set a default exit dispatching action which is to set the game mode state to “idle”.
Now we’re checking if the wordsNumber (which is defined as a component’s state) is different than the words.length (which is the game state value) we set the wordsNumber to the current words.length and redefine default exit dispatching to a callback function (oh...) which in turn triggers the refresh flow with the confirmation and all.
In the end the code toggles between entering the edit mode or exiting it according to the game mode it is in.

Wow.

I wrote some nasty logic here, I admit, and I have a good idea what causes the bug here, but before I try to mess with the code there I’d like to fixate the logic at hand with some tests so that I can feel more confident with future changes I’m about to make. Gladly for me the component already has a test suite for it and I will just need to add the missing bits.
Running the test coverage over this one and I see that this method mentioned above is not covered:

The test already wraps the react-testing-lib render function with the ability to inject state stores into it (both Mobx and Redux) so when we render it we can expect a certain UI to appear accordingly. Here’s how it looks:

import React from 'react';
import {render as rtlRender, fireEvent, screen} from '@testing-library/react';
import {Provider} from 'react-redux';
import {createStore, combineReducers} from 'redux';
import wordsReducer from '../reducers/words-reducer';
import WordSearchGameState from '../mobx/WordSearchGameState';
import pendingConfirmationReducer from '../reducers/pending-confirmation-reducer';
import gameModeReducer from '../reducers/game-mode-reducer';
import {EDITING_GAME_MODE, IDLE_GAME_MODE} from '../constants';
import {StateContext} from '../App';
import Masthead from './Masthead';

const combinedReducers = combineReducers({
   words: wordsReducer,
});

const render = (
   ui,
   {initialMobxState = new WordSearchGameState(), store = createStore(combinedReducers), ...renderOptions} = {}
) => {
   const Wrapper = ({children}) => (
       <Provider store={store}>
           <StateContext.Provider value={initialMobxState}>{children}</StateContext.Provider>
       </Provider>
   );

   return rtlRender(ui, {wrapper: Wrapper, ...renderOptions});
};

Note: It might be a bit overwhelming, having both Mobx and Redux supported, but if you read the code it is not that complicated - I simply created a component which wraps the UI component I want to render with store providers for both Redux and Mobx, and return it (read more about it here).

Ok, now that I have this in place, let’s start testing our Edit/Play button logic.

I will set a describe of “Edit toggle button” and set the first test in it:

describe('Edit toggle button', () => {
       it('should toggle the "Edit" button to "Play" when the edit button is clicked', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();
       });
   });

As you can see I’m asserting the Button here, for I know that the button’s label changes when the game mode state changes and that’s sufficient for me to know that the game mode state has changed.

Now I would like to check that if the user did not change the word bank, clicking on the “Play” button (which I remind you, is the same button) will resume the game with no confirmation asked, that is the label on the button should be “Edit” at the end of the cycle:

it('should toggle the "Play" button back to "Edit" upon click, when in "edit" mode and there was no change in the words bank', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();
       });

I know that the code which renders the component is repeating, and I keep it that way to give me the freedom to change a single test rendering logic without affecting the rest. Many, including yours truly, will argue that the concept of DRY is not applicable for tests, or should be very well thought about before jumping into refactoring the test.

Now I would like to check that when the user does change the word bank (by, say, adding a word) clicking on the “Play” button does not resume the game. We can check that by dispatching the action for adding a word and then see if the label on the button remains to be “Play” which indicates that the game was not resumed.

it('should not toggle back to "Edit" upon click, when in "edit" mode and there was change in the words bank', () => {
           const {getByRole} = screen;

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           store.dispatch(addWord('chuck'));

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();
       });

You know what? I will even go the extra mile and check the game state to see if there is any confirmation pending, since the appearance of a confirmation dialog is triggered by a state change:

it('should not toggle back to "Edit" upon click, when in "edit" mode and there was change in the words bank', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
               pendingConfirmation: pendingConfirmationReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           store.dispatch(addWord('chuck'));

           // Check the confirmation state
           let state = store.getState();
           expect(state.pendingConfirmation).toBeNull();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           // Check the confirmation state
           state = store.getState();
           expect(state.pendingConfirmation).toBeDefined();
           expect(state.pendingConfirmation?.msg).toEqual(
               'All progress will reset. Are you sure you wanna refresh the game?'
           );
       });

So we know for sure that the confirmation is requested when the word bank has changed and the user asks to resume the game (Notice that I added the reducer for the confirmation state when rendering the component).

Many would argue that this is an implementation detail that the end user is not aware of, but I would argue back that I’m checking a single component here and not the entire application, and while the entire application listens to the state and changes the UI accordingly, I consider it an overkill to render the entire application for the sake of these tests and as I see it, it is out of the scope of the component at stake here.

Ok, now we’ve reached the point where it all began. The next test is the one which simulates the flow of the bug we mentioned at the start:

it('should not toggle back to "Edit" upon click, when there was a change in the words bank but confirmation was canceled', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
               pendingConfirmation: pendingConfirmationReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           // Add a word
           store.dispatch(addWord('chuck'));

           fireEvent.click(getByRole('button', {name: 'Play'}));

           // Cancel the confirmation
           store.dispatch(cancelConfirmation());

           let state = store.getState();
           expect(state.pendingConfirmation).toBeNull();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           // Check the confirmation state
           state = store.getState();
           expect(state.pendingConfirmation).toBeDefined();
           expect(state.pendingConfirmation?.msg).toEqual(
               'All progress will reset. Are you sure you wanna refresh the game?'
           );
       });

It currently fails since the button on the document is not “Play” but rather “Edit”, which means that we’re back on the game, and we also see that there is no pending confirmation since there is no game state to indicate that.

BTW - if I run the coverage again I will see that it is 100% covered, but we know that there is still a bug there. This is one of the caveats of writing test-after and relying on coverage percent:

Once we nailed that, it is time finally to solve the bug -
Back to the function at hand:

function onToggleGameMode() {
       const shouldGoIntoEditMode = gameMode !== EDITING_GAME_MODE;
       let exitEditMode = () => dispatch(enterIdleMode());
       // If the words number are different we need to reset the game and
       // not just resume it
       if (wordsNumber !== words.length) {
           setWordsNumber(words.length);
           exitEditMode = onRefreshGame;
       }
       shouldGoIntoEditMode ? dispatch(enterEditingMode()) : exitEditMode();
   }

The issue resides on this line:

setWordsNumber(words.length);

I am setting the component inner state to the new value from the game state before the confirmation was done. So even though the user did not confirm the component already holds the update words number and therefore it won’t ask for confirmation once again.
I’m removing the line from there. Updating the component’s wordsNumber state should be only after the confirmation is done.

Luckily the confirmation implementation can accept a callback to execute when the confirmation is done and this can come in handy in our case. Currently what it is used for in this context is to reset the score:

function onRefreshGame() {
       const pendingConfirmationAction = resetGame();
       const pendingConfirmationCallback = stateContext.reset.bind(stateContext);
       const confirmResetGameAction = createConfirmAction({
           pendingConfirmationAction,
           msg: 'All progress will reset. Are you sure you wanna refresh the game?',
           pendingConfirmationCallback,
       });
       dispatch(confirmResetGameAction);
   }

Let’s add the words number setter in it and see what happens:

const pendingConfirmationCallback = () => {
           stateContext.reset();
           setWordsNumber(words.length);
       };

What’dya know it works :) All tests are happy and no more bugs (for now). Here is the final describe section:

describe('Edit toggle button', () => {
       it('should toggle the "Edit" button to "Play" when the edit button is clicked', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();
       });

       it('should toggle the "Play" button back to "Edit" upon click, when in "edit" mode and there was no change in the words bank', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Edit'})).toBeInTheDocument();
       });

       it('should not toggle back to "Edit" upon click, when in "edit" mode and there was change in the words bank', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
               pendingConfirmation: pendingConfirmationReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           store.dispatch(addWord('chuck'));

           // Check the confirmation state
           let state = store.getState();
           expect(state.pendingConfirmation).toBeNull();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           // Check the confirmation state
           state = store.getState();
           expect(state.pendingConfirmation).toBeDefined();
           expect(state.pendingConfirmation?.msg).toEqual(
               'All progress will reset. Are you sure you wanna refresh the game?'
           );
       });

       it('should not toggle back to "Edit" upon click, when there was a change in the words bank but confirmation was canceled', () => {
           const combinedReducers = combineReducers({
               words: wordsReducer,
               gameMode: gameModeReducer,
               pendingConfirmation: pendingConfirmationReducer,
           });
           const store = createStore(combinedReducers);
           const {getByRole} = render(<Masthead />, {store});

           fireEvent.click(getByRole('button', {name: 'Edit'}));

           // Add a word
           store.dispatch(addWord('chuck'));

           fireEvent.click(getByRole('button', {name: 'Play'}));

           // Cancel the confirmation
           store.dispatch(cancelConfirmation());

           let state = store.getState();
           expect(state.pendingConfirmation).toBeNull();

           fireEvent.click(getByRole('button', {name: 'Play'}));

           expect(getByRole('button', {name: 'Play'})).toBeInTheDocument();

           // Check the confirmation state
           state = store.getState();
           expect(state.pendingConfirmation).toBeDefined();
           expect(state.pendingConfirmation?.msg).toEqual(
               'All progress will reset. Are you sure you wanna refresh the game?'
           );
       });
   });

Phew... That was a long ride, but as you can see, while writing the tests even before we attempted to solve the issue I gained a better understanding of the code I wrote myself, so when the time came to fix it I had 2 things in my belt - better understanding and a safety net to keep me from introducing regressions as part of the fix.

As always, if you have any ideas on how to make this better or any other technique, be sure to share with the rest of us!

Cheers

Hey! If you liked what you've just read check out @mattibarzeev on Twitter 🍻

Photo by Dmitry Bukhantsov on Unsplash