# Testing a javascript function that performs side effects

Application context

I am working on a game I call "Apes and Snakes". So far the code only allows players to connect to a host using a room code and WebRTC. Players and the host send messages back and forth to each other over WebRTC. When a player receives a message, it is passed to a "handler". This kind of pattern is somewhat comparable to the way an HTTP request to a certain route gets passed on to a method of a controller in an MVC architecture.

The full repository can be found at https://github.com/colesam/apes-and-snakes, but I will be posting the relevant code snippets below. It's a react application but the portion I'm dealing with for this post mainly has to do regular javascript/typescript logic and jest, as well as reading/writing from a zustand (simplified redux) flux storage.

What I want reviewed

The code I want reviewed is the "handler" for when the host receives a reconnect message. Specifically I would like my strategy for testing this function reviewed.

The function I am testing is called handleReconnect. What this function does in short is allow players to reconnect to the game from a new peerId if they still have their old secret key (what I use to auth/link a player to their data). My original implementation of the method looks like this:

import { TActionHandlerProps } from "../handleAction";
import GeneralError from "../../error/GeneralError";
import { getPrivate } from "../../store/privateStore";
import { StoreAction } from "../../store/StoreAction";

const handleReconnect = ({
peerId,
payload,
respond,
error,
}: TActionHandlerProps) => {
// SIDE EFFECT 1: Pull secret key map from flux storage
const { secretKeyPlayerIdMap } = getPrivate();

const playerId = secretKeyPlayerIdMap.get(payload.secretKey);

if (!playerId) {
return error(
new GeneralError("Could not find playerId. Failed to reconnect.")
);
}

// SIDE EFFECT 2: Update the peerId of the player in flux storage
StoreAction.setPlayerConnection(playerId, { peerId });

return respond();
};

export default handleReconnect;


This function has two side-effects that make this difficult to test:

1. getPrivate is a function that returns the current state of my flux storage.

2. StoreAction.setPlayerConnection is a function that updates my store. The StoreAction object is really just used as a namespace for functions that mutate the store because a lot of the actions have very general names that collide with other areas of my application.

To get around this, I decided to make a factory function that returns the handler, injecting the necessary side-effects. The default export still exports the function as it exists in the previous code snippet, while exposing a way for me to inject mocks when testing.

Here is how I rewrote handleReconnect:

import { TActionHandlerProps } from "../handleAction";
import GeneralError from "../../error/GeneralError";
import { getPrivate } from "../../store/privateStore";
import { StoreAction } from "../../store/StoreAction";

export const makeHandleReconnect = (
_getPrivate: typeof getPrivate,
_StoreAction: typeof StoreAction
) => ({ peerId, payload, respond, error }: TActionHandlerProps) => {
const { secretKeyPlayerIdMap } = _getPrivate();

const playerId = secretKeyPlayerIdMap.get(payload.secretKey);

if (!playerId) {
return error(
new GeneralError("Could not find playerId. Failed to reconnect.")
);
}

_StoreAction.setPlayerConnection(playerId, { peerId });

return respond();
};

const handleReconnect = makeHandleReconnect(getPrivate, StoreAction);

export default handleReconnect;


Here is how I tested this rewritten version:

/* eslint-disable */
import { makeHandleReconnect } from "./handleReconnect";
import { Map } from "immutable";

const makeParams = (opts = {}) => ({
peerId: "123",
payload: { secretKey: "existing-secret" },
respond: jest.fn(),
error: jest.fn(),
...opts,
});

const makeGetPrivate = (opts = {}) => () => ({
secretKeyPlayerIdMap: Map([["existing-secret", "player-id"]]),
...opts,
});

const makeStoreAction = (opts = {}) => ({
setPlayerConnection: jest.fn(),
...opts,
});

let callHandleReconnect;
beforeEach(() => {
callHandleReconnect = ({
params = makeParams(),
getPrivate = makeGetPrivate(),
StoreAction = makeStoreAction(),
}) => {
// noinspection JSCheckFunctionSignatures
const handleReconnect = makeHandleReconnect(getPrivate, StoreAction);
return handleReconnect(params);
};
});

test("responds if secret key is found", () => {
const params = makeParams();

callHandleReconnect({ params });

expect(params.respond.mock.calls.length).toBe(1);
});

test("updates player's peer id if secret key is found", () => {
const params = makeParams();
const StoreAction = makeStoreAction();

callHandleReconnect({ params, StoreAction });

const { mock } = StoreAction.setPlayerConnection;
expect(mock.calls.length).toBe(1);

const [playerId, { peerId }] = mock.calls[0];
expect(playerId).toBe("player-id");
expect(peerId).toBe("123");
});

test("errors if secret key is not found", () => {
const params = makeParams({
payload: { secretKey: "missing-secret" },
});

callHandleReconnect({ params });

expect(params.error.mock.calls.length).toBe(1);
});


I'm open to any and all feedback on the project as a whole, but specifically I am interested in:

1. If my strategy makes sense or if it's way off base
2. How it can be improved
3. How my tests can be made more readable/improved