1

So I am just starting to use a linter and I like the no-magic-numbers rule in generally. But in unit test files it is giving me 1000 warnings. Here is what they look like:

it('getWeekNumber', function() {
    expect(getWeekNumber(1538918924615)).to.deep.equal([40, 2018])
    expect(getWeekNumber(1536918924615)).to.deep.equal([37, 2018])
    expect(getWeekNumber(1528918924615)).to.deep.equal([24, 2018])
    expect(getWeekNumber(1438918924615)).to.deep.equal([32, 2015])
})

There is no real logic here, every programmer can read what is going on in this test case. Changing like this won't provide any value in my opinion:

let someRandomTime1 = 1538918924615
let week40 = 40
let year2018 = 2018
expect(getWeekNumber(someRandomTime1)).to.deep.equal([week40, year2018])

Here is another example:

it('timePadding', function() {
    expect(timePadding(1)).to.equal('01')
    expect(timePadding(5)).to.equal('05')
    expect(timePadding(19)).to.equal('19')
    expect(timePadding(111)).to.equal('111')
})

While this one can clearly have been tested more programmatically, it is simple, it is readable and it gets the job done.

Should I disable linter no-magic-numbers in unit test files completely?

10

Any linter rule anywhere should be disabled whenever it fails its purpose.

The main arguments against magic numbers are that

  • the information of a literal should be maintained in one place only rather than spreading this information throughout the code, and that
  • the use of named variables can make code more self-documenting.

In test programs, these arguments generally fail because our tests contain concrete example scenarios. The meaning of the number is given through its context. This doesn't mean that magic numbers are OK in tests, just that most literals in test programs are not very magic.

Ideally, your linter allows you to disable individual policies within a confined scope. Failing that, perform post-processing on linter messages, or exclude the whole file from this problematic policy.

Similar to forbidding magic numbers, linter rules that require documentation comments for functions or impose a maximum function size are mostly useless in test programs.

4

In the specific case of literal values in asserts statements it may help readability and maintainability to use “magic numbers”.

When choosing to disable linter warnings it makes sense to disable them at the narrowest scope. If your linter lets you disable checking at statement level then prefer that by default. This is because we can think of examples where the magic numbers rule may help:

var mockWidget = new Mock(3.14d);
var result = testObject.process(mockWidget);
assert( 3.14d, result);

The above the code would be improved by defining a constant such as var expectedResult = 3.14d; and the magic number lint warning may give you a hint that the code can be improved. That example is slightly contrived but shows that way it is usually a good idea to review all warnings rather than globally disable them. They can then act as a as a form of rubber duck debugging when you justify to yourself why the linter is wrong in a particular case.

In large teams trying to agree which rules to disable can lead to religious wars. That is avoided by taking the approach above of disabling at the narrowest scope on a case by case basis. Some linters name their rule sets and have one called "contentious" for rules that are often disputed. That is the only set of rules I would recommend either disabling or cherry picking from.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.