Refactoring – It's just me

I am working on a project that allows customers to purchase credits to be applied towards so circles (buckets) for use to perform certain actions. The code is pretty simple, but there are bugs and I think it is the right time to refactor. The app is a SPA written in React, so it is all JavaScript and the tests are in Jest. It’s also a multi-lingual so we are pulling in the react-intl library which is a great library. Here it is what we are starting with:

useEffect(
        () => {
            let totalAllocated = plan.totalCaseCreditsAllocatedInCircle ? plan.totalCaseCreditsAllocatedInCircle : 0
            let totalUnallocated = plan.totalCaseCreditsUnallocated ? plan.totalCaseCreditsUnallocated : 0
            if (casesInputValue !== totalAllocated) {
                if (casesInputValue < totalAllocated) {
                    let x = totalAllocated - casesInputValue
                    if (x < 0) x = 0
                    setExplanationText(
                        intl
                            .formatMessage({ id: 'app.circle.manageAllocations.casesTooltipRemoving' })
                            .replace('%N%', x)
                    )
                    setOneTimeCasePrice(0)
                } else if (casesInputValue >= totalUnallocated) {
                    //if they are asking for more than is available
                    let overage = 0
                    overage = casesInputValue - (totalUnallocated + totalAllocated)
                    if (overage > 0) {
                        setExplanationText(
                            intl
                                .formatMessage({ id: 'app.circle.manageAllocations.casesTooltipAdding' })
                                .replace('%N%', overage)
                        )
                        setOverage(overage)
                        setOneTimeCasePrice(overage * casePrice)
                    } else {
                        setExplanationText()
                        setOneTimeCasePrice(0)
                    }
                } else {
                    setExplanationText()
                    setOneTimeCasePrice(0)
                }
            } else {
                setExplanationText()
                setOneTimeCasePrice(0)
            }
        },
        [ casesInputValue ]
    )

The scenario is that the user is trying to apply credits to a group (not interesting, just a term). If they do not have enough credits they will be charged for the overage. So you can see from the spaghetti above that there are lots of work to do (maybe not a lot). There are five use cases that need to be implemented:

User is making no changes
User has zero credits and is trying to add more
User has credits and is applying less than the total that they have paid for
User has is applying credits, some of which they have in the bank and then an overage that must be charged
User has specified less than what is currently allocated, then the difference will be removed and made available for reallocation

Now that we know what we were supposed to be doing we can add some tests to prove that our refactoring was correct. Like I mentioned, for the testing I will be using Jest. To get started with Jest, there are two scenarios in a React app. If you have built your react app using create-react-app then jest is already configured for you. If you have built from scratch or you have ejected your app you will need to hop over to their website and follow the instructions. Our app uses create-react-app so things start a little easier.

Since we used the builder and it is already configured we can just run npm run test. It will scour our workspace for files that use a naming convention, I use class.test.js. Now I am using VS Code for Mac, so for convenience I configured the launcher to run the test automatically. To get it running the way I wanted I needed to add a json file: launch.json. Here is the configuration that I used:

{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "node",
            "name": "vscode-jest-tests",
            "request": "launch",
            "runtimeExecutable": "${workspaceFolder}/node_modules/.bin/react-scripts",
            "args": [
                "test",
                "--env=jsdom",
                "--runInBand"
            ],
            "cwd": "${workspaceFolder}",
            "console": "integratedTerminal",
            "protocol": "inspector",
            "internalConsoleOptions": "neverOpen",
            "disableOptimisticBPs": true
        }
    ]
}

Now from the debugger view, I can use it’s run buttons to debug my tests. So for the first test case: User is making no changes I start with an empty method that does nothing more than returns an empty result:

export const onManagePlanCaseRequestChanged = (input, plan, intl, price) => {
	let result = {
		additions: 0,
		newNonFreeAdditions: 0,
		price: 0,
		explanationText: null
	}
	if (plan && plan.totalCaseCreditsAllocatedInCircle !== input) {
	}
	return result
}

Pretty straightforward, so lets add a test:

test('input and plan are equal', () => {
	expect(onManagePlanCaseRequestChanged(1, 1, intl, price)).toEqual({
		additions: 0,
		newNonFreeAdditions: 0,
		price: 0,
		explanationText: null
	})
})

If we break this down setting a name for our test 'input and plan are equal' then the callback that will be passed to execute the test expect(onManagePlanCaseRequestChanged(1, 1, intl, price)). All thats left is to specify what will be the result. In this case, I am returning an object so we can use toEqual to validate the result. When we run the test we get a print out with the results:

Test Suites: 1 skipped, 0 of 1 total
Tests: 1 skipped, 1 total
Snapshots: 0 total
Time: 3.464s, estimated 4s

One down, let’s attack use case 2: User has zero credits and is trying to add more. Lets add the new logic inside the if state for the case when they are adding:

if (plan && plan.totalCaseCreditsAllocatedInCircle !== input) {
	if (input > plan.totalCaseCreditsAllocatedInCircle) {
		//adding cases

		result.additions = input - plan.totalCaseCreditsAllocatedInCircle
		result.explanationText = intl
			.formatMessage({ id: 'app.circle.manageAllocations.casesTooltipAdding' })
			.replace('%N%', result.additions)
	}
}

Now that we are starting to check if they are asking to add more to the bucket, so we will pass in a request for more items over what is already in there.

test('input is greater than allocation, but less that total unallocated.  returns overage and message  ', () => {
	let plan = {
		totalCaseCreditsAllocatedInCircle: 10,
		totalCaseCreditsUnallocated: 10
	}

	const formatMessage = jest.fn((x) => 'Adding %N% Case Credits to this Circle')
	let intl = {
		formatMessage: formatMessage
	}

	expect(onManagePlanCaseRequestChanged(11, plan, intl, price)).toEqual({
		additions: 1,
		newNonFreeAdditions: 0,
		price: 0,
		explanationText: 'Adding 1 Case Credits to this Circle'
	})
})

This become interesting because now I have a dependency it required Intl. This object has a single method that we will need to mock formatMessage. Jest provide a simple utility to mock it out. We need to specify using a fat arrow a response to a callback:

const formatMessage = jest.fn((x) => 'Adding %N% Case Credits to this Circle')
//Assign it to the intl object
let intl =
{
    formatMessage: formatMessage
}

Now whenever the formatMessage is called on the intl object, the message above will be returned. We can run this test and see that it passes also:

Test Suites: 1 passed, 1 total
Tests: 3 skipped, 1 passed, 4 total
Snapshots: 0 total
Time: 3.464s, estimated 4s

Jumping ahead here is the final code

export const onManagePlanCaseRequestChanged = (input, plan, intl, price) => {
	let result = {
		additions: 0,
		newNonFreeAdditions: 0,
		price: 0,
		explanationText: null
	}

	if (plan && plan.totalCaseCreditsAllocatedInCircle !== input) {
		if (input > plan.totalCaseCreditsAllocatedInCircle) {
			//adding cases

			result.additions = input - plan.totalCaseCreditsAllocatedInCircle
			result.explanationText = intl
				.formatMessage({ id: 'app.circle.manageAllocations.casesTooltipAdding' })
				.replace('%N%', result.additions)

			if (result.additions > plan.totalCaseCreditsUnallocated) {
				result.newNonFreeAdditions = result.additions - plan.totalCaseCreditsUnallocated
				result.price = result.newNonFreeAdditions * price
			}
		} else if (input < plan.totalCaseCreditsAllocatedInCircle) {
			//removing cases
			result.explanationText = intl
				.formatMessage({ id: 'app.circle.manageAllocations.casesTooltipAdding' })
				.replace('%N%', plan.totalCaseCreditsAllocatedInCircle - input)
		}
	}
	return result
}

Happily, all cases are covered and the tests now pass:

Test Suites: 1 passed, 1 total
Tests: 4 passed, 4 total
Snapshots: 0 total
Time: 3.431s, estimated 8s
Ran all test suites.

Issues:

I had a problem where Jest was not able to import the export I was testing and this was because it was looking in the wrong spot. Once I set the configuration like it is above with was able to load the method and execute the tests.

Lesson:

I should have did this the first time I had to touch this code. Instead I was lazy and added a bandaid which is why I am having to do this today. Will I have learned from this lesson, probably not given how lazy I am. Unit test are a obviously a necessity for business logic and even more so when refactoring. This is my experience and mileage may vary.

Tag: Refactoring

I Jest, but still I test