I'm working on an enterprise that has some Angular/Typescript projects and to avoid repeating code (basically copying and pasting) between them, we decided to go for Monorepo and start write an util library, with unit tests, docs and everything on.
At the moment we're implementing an util function:
export const normalizeNames = (value: string): string => {
if (!isString(value)) {
// throw some error
}
// ...
}
Just as the company is relatively new to testing concepts in general, so am I.
Since we are at an impasse trying to establish a standard of how the tests should be structured and what we should test, I decided to open this question here.
The first thing that came to my mind was to separate them in two main groups:
- Invalid -> A test for each invalid type I could imagine, like: 1
null
, 1undefined
, 1NaN
, 1boolean
, 1number
, 1array
, and others likeBuffer
,Map
,Object
,RegExp
,Set
, etc.; - Valid;
... something like this:
describe('normalizeNames', () => {
describe('invalid', () => {
it(`should throw error for the value 'null'`, () => {
expect(() => normalizeNames(null as any)).toThrowError(
TypeError,
);
});
it(`should throw error for the value 'undefined'`, () => {
expect(() => normalizeNames(undefined as any)).toThrowError(
TypeError,
);
});
// other types
});
describe('valid', () => {
it(`should return '' for the value ''`, () => {
expect(normalizeNames('')).toBe('');
});
it(`should return 'Stack' for the value 'stack'`, () => {
expect(normalizeNames('stack')).toBe('Stack');
});
// ... more tests
});
});
...but then I noticed that if I test all the types I can imagine, the tests would be too big and maybe difficult to maintain.
Another solution that I thought is to create two Array
s and do something like below, to avoid the repetition:
const invalidTestCases = [
{ actual: null, expected: TypeError },
{ actual: undefined, expected: TypeError },
// more...
];
const validTestCases = [
{ actual: '', expected: '' },
{ actual: 'stack', expected: 'Stack' }, // it's just a sample data
// more...
];
describe('normalizeNames', () => {
describe('invalid', () => {
for (const { actual, expected } of invalidTestCases) {
it(`should throw error for the value '${actual}'`, () => {
expect(() => normalizeNames(actual as any)).toThrowError(
expected,
);
});
}
});
describe('valid', () => {
for (const { actual, expected } of validTestCases) {
it(`should return '${expected}' for the value '${actual}'`, () => {
expect(() => normalizeNames(actual as any)).toBe(expected);
});
}
});
});
So, the questions are basically:
- Is it okay to separate the tests in these two main "groups"?
- Is it acceptable to have tests for all possible "types"? Otherwise, which entries would you recommend for invalid tests?
- For the second solution: is it a good practice to write tests in that way, with loops?
3 Answers 3
As a general rule, whatever conventions your team comes up with and agrees upon is good. Just be consistent in your project.
I have worked on teams that use exactly the conventions you are describing, and it has worked well for us.
To give some detail to each of your questions:
Is it okay to separate the tests in these two main "groups"?
yes! describe
blocks are there to group tests to make them easy to read.
This of them like "headers" in an spec. Your tests are the documentation for your code. Group the tests so that a future reader can "scan" the describe
blocks to find the tests they care about looking at.
Is it acceptable to have tests for all possible "types"?
Of course! If you think it needs tested, you should test it. If you think it doesn't need to be tested, you may want to test it anyway. Unit tests run incredibly fast, and as long as you keep your test organized well, I have pretty much never heard people complain about having "too many unit tests".
is it a good practice to write tests with loops?
Sure! Your test code should be treated the same as your "production" code. Any principles you think matter for production code, also matters for test code. So tools to reduce duplication, and keep things organized and easy to read is a great idea.
One thing that we have done on teams in past to make this even easier is to add the "description" of the test case into the object you are passing in the loop. This help future readers see why the different cases matter. And we try to keep our "test cases" close to the it
block so that future readers don't have to scroll around a lot.
Something like this:
describe('normalizeNames', () => {
... // other tests
describe('valid input', () => {
[
{
description: 'empty strings normalize as empty string',
input: '',
expected: ''
},
{
description: 'names with hyphens are treated as a single word',
input: 'sOme-Named-pErSon',
expected: 'Some-named-person'
},
{
description: 'names with spaces are treated as multiple words',
input: 'some person name',
expected: 'Some Person Name'
},
// other test cases for your business logic...
].forEach({ input, expected, description }) {
it(`can normalizeNames for valid input - ${description}. input: '${input}', expected: '${expected}'`, () => {
expect(() => normalizeNames(actual)).toBe(expected);
});
}
});
});
-
Thanks a lot for your answer. It clarified things a little more :)dev_054– dev_0542020年04月12日 14:39:51 +00:00Commented Apr 12, 2020 at 14:39
Generally you want tests to test one single thing and have a name that describes what they are testing.
Of you put a loop in your test to make it test lots of things you can run into problems.
- does it stop when it hit the first error?
- when it errors does it tell you which case failed?
- Has it become complex enough that you might have an error in your tests
As @doc mentions in their comment, the way around this is data driven tests.
Data driven tests allow you to use the same code for multiple tests, generating a new name in the test output and using different input for each.
[TestCase(null)]
[TestCase(MaxInt)]
[etc]
InputShouldBeValid(input:any)
{
...test and assert
}
Adding on to the existing answers here -
Instead of enumerating all different cases for your tests, think about the properties of your code that will always be true regardless of the input.
After that, apply a property-based testing framework to help you perform the value generation + assertion. More details here: https://marmelab.com/blog/2019/04/18/property-based-testing-js.html
A Property Based Testing using library such as fast-check is an interesting alternative to exhaustive enumeration. It allows to find bugs both in the implementation code and in the specification of the property we want to check. In both cases this helps the developer to get confident in the product he is developing.
Explore related questions
See similar questions with these tags.
Jest
hastest.each
/it.each
and I'm going to use it right now.