Unit Test makes your software better in terms of not only quality but also class structure because you need to separate classes well in order to write the test for the “Unit”. If you have good unit test you can confidently add new features, refactor the code and fix bugs without breaking existing functionalities. But what is Good Unit Test? How does Good Unit Test look like? I think Good Unit Test should have following characters.
- Fast enough
- Test against Unit
- Isolated
- One Assertion
- Good test title
- Simple
It looks there are many characters but actually not. Let’s see one by one.
Tests should be Fast enough
If the execution time per test is less than 10 ms I’m very comfortable. But how do you develop if it takes 5 seconds for each test? I don’t want to run the test frequently. Maybe I execute the test at the end of the modification in this case. 5 seconds per test is too much if the number of tests is big. It depends on the number of tests whether time consuming test is acceptable or not because I don’t want to wait for a long time for each test execution. I want to execute test suite when I modify something in my code especially while refactoring. The bigger change you have, the harder you find the root cause for the test failure. This habit to execute test suite frequently let you go forward faster because you can save time to investigate the root cause of the bug which you added. Test should be fast enough to keep small steps. You should consider to make it faster if it takes 1000 ms per test. But this is not applied to integration test because it takes normally longer than unit test.
A test case tests against an Unit
Test against Unit
Sometimes a unit test tests against combination of classes. This is kind of integration test. The test result should be the same unless you modify something in the class. If the changes to a class break other class’s test it does NOT test against unit. You should consider to refactor it in this case. Following test is kind of integration test.
// Editor.ts
export class Editor {
public edit(text: string, type?: DecoratorType): string {
const decorator = this.getDecorator(type);
const decoratedText = decorator.decorate(text);
if (decoratedText.length > 100) {
throw new Error("Length must be less than 100.");
}
return decoratedText;
}
private getDecorator(type?: DecoratorType): Decorator {
switch (type) {
case DecoratorType.Asterisk:
return new AsteriskDecorator();
case DecoratorType.Equal:
return new EqualDecorator();
default:
return new DefaultDecorator();
}
}
}
// Editor_spec.ts
describe("Editor", () => {
describe("edit", () => {
it("should not throw an error when text length is 96 with default decorator", () => {
const editor = new Editor();
const result = () => editor.edit("x".repeat(96));
expect(result).to.not.throw();
});
it("should throw an error when text length is 97 with default decorator", () => {
const editor = new Editor();
const result = () => editor.edit("x".repeat(97));
expect(result).to.throw();
});
});
});When I change the implementation of DefaultDecotrator class it breaks this test. In order to test the logic in the edit function I should somehow stub getDecorator function in order to get rid of the dependency in the test. You can learn how to do it in this post.
Unit test should be Isolated
Each test should be isolated which means that one of test results should not affect other test result. If you need to run some tests in fixed order they are not isolated. If you need to change one of environment variables it must be set back to the default at the end of each test.
One test should have only one Assertion
A test should have only one assertion. It means that a test should test one matter. If there are two assertions it tests two things in one test. If the test fails you need to firstly check which assertion throw the error. In addition to that, we need to read the logic in each test in order to check whether there are any missing tests or not. It takes more time to do that. If one test case tests only one thing we can check it without reading the logic.
You might want to put multiple test data and expected data if the result is the same. However, it is not good practice because we have to check which value causes the problem. Let’s see a simple example.
// calc.ts
export function calc1(value: number) {
if (value === 1 ||
value === 3 ||
value === 5 ||
value === 12
) {
return 20;
}
return 10;
}
// calc_spec.ts
describe("calc1", () => {
it("should return 20 when value is 1 or 3 or 5 or 12", () => {
[1, 3, 5, 12].forEach((value) => {
const result = calc1(value);
expect(result).to.equal(20);
});
});
[1, 3, 5, 12].forEach((value) => {
it(`should return 20 when value is ${value}`, () => {
const result = calc1(value);
expect(result).to.equal(20);
});
});
});First test case tests all values that return 20. If it fails we have to check which one causes the problem. Second test case looks similar but its test case is separated. Each test case tests only one value. Even if one of them fails other tests still succeed.
Each test should have Good test title
Test title should be named according to what the test case tests. When using mocha in JavaScript/TypeScript the title should look like this. Top level describe is class name. Second level describe is function name and then, test title comes. This means “Editor.edit function should not throw an …”.
describe("Editor", () => {
describe("edit", () => {
it("should not throw an error when text length is 98 with default decorator", () => {
...
});
});
});When using C# for example, it looks like following.
- MethodName_WhenDoingSomething_ResultDescription()
When using a framework such a mocha it is easy to describe the title even if the test case tests two things but when you need to describe it as method name it gets too long. Therefore, test should be small enough.
Title examples
| Bad | Good | Bad Reason |
|---|---|---|
| should return correct value | should return 5 when the value is 10 | unclear what it tests |
| should return 5 when the value is less than 11 | should return 5 when the value is 10 | unclear what value is used |
Unit test should be Simple enough
Unit test can be a kind of sample code to know how to use the class. If the unit tests are simple enough
- It is easy to understand how to use it
- A reviewer can easily check whether those tests are correct or not
- Easy to modify the code when changing the function’s behavior
But how does a simple test look like?
Short
Test should be short enough. It’s long if it has 50 lines. The longer the test code is, the more the bugs comes into the test code. We don’t want to read long test code. If preparation step is big it might be better to extract the logic into a function.
Less configuration
If the target class requires lots of configurations you should consider to extract the configuration part into another class or function. Another solution would be refactoring the target function or class itself. If the target function is small enough big configuration may no longer be necessary. If we can do it the target class gets smaller and more maintainable.
No logic
Unit test is to test production code. If some logic exist in a unit test a bug may exist in the test code especially when the same logic as production code exists there. In this case unit test will be green and test result doesn’t tell you anything.
Some want to remove duplicated code but it is not always good idea to keep DRY for unit test. Hard coded value should be used in tests as much as possible. The test code should tell which value is used as test data and expected data. The code written in the clause should tell us everything that we need.
Don’t calculate test value and expected value in the test code. Don’t use conditional clauses such if-else, switch-case and etc… I have found before that one of tests were not executed because of if-else. It looked something like this.
const testData = [
testData1: {status1: 1, status2: 2},
testData2: {status2: 3},
testData1: {status1: 4, status2: 3},
...
];
testData.forEach((testData) => {
if(testData.status1){
it("should do something", () => {
// test here
});
}
if(testData.status2){
it("should do something", () => {
// test here
});
}
});The test code expects that the all test data has status1 property but one of test data doesn’t have it. It can happen when adding new feature or changing existing logic. Unit test doesn’t make much sense in this case. All tests should be executed.
Test should have minimum test cases
Functions can have conditional clauses. Don’t you prepare many test values in order to confirm the behavior? In the following simple case, we should prepare only 4 values.
export function calc2(value: number) {
if (value < 12 || value > 17) {
return 50;
}
return 0;
}
describe("calc2", () => {
it("should return 50 when value is 11", () => {
const result = calc2(11);
expect(result).to.equal(50);
});
it("should return 50 when value is 18", () => {
const result = calc2(18);
expect(result).to.equal(50);
});
it("should return 0 when value is 12", () => {
const result = calc2(12);
expect(result).to.equal(0);
});
it("should return 0 when value is 17", () => {
const result = calc2(17);
expect(result).to.equal(0);
});
});We should pick only boundary values. Following values are too much.
[
{ input: 10, expected: 50 },
{ input: 11, expected: 50 },
{ input: 12, expected: 0 },
{ input: 13, expected: 0 },
{ input: 14, expected: 0 },
{ input: 15, expected: 0 },
{ input: 16, expected: 0 },
{ input: 17, expected: 0 },
{ input: 18, expected: 50 },
{ input: 19, expected: 50 },
].forEach((testData) => {
it(`should return ${testData.expected} when value is ${testData.input}`, () => {
const result = calc2(testData.input);
expect(result).to.equal(testData.expected);
});
});Conclusion
We learnt how to write good unit test. However, it is not always easy to follow all of them when tackling legacy code. In this case we should get rid of the impediments one by one and improve the code. If we write good code other members can easily follow the way and code can be kept clean. If you find dirty code you should try to refactor the code when you find it while your knowledge is fresh around the code. If the test code is good enough the production code should also be good.
If you want to try to run yourself you can find the code here.
Comments