Is it a good practice to include "magic numbers" in test case name?

Question

Imagine a twitter-like app where users are allowed to follow to a limited amount of users; say 100.

I have this code:

fun follow(followerId: String, followedId: String) {
  if (repo.countFollowing(followerId) >= 100) {
    throw MaxFollowingReached()
  }
  repo.insertFollowingRelationship(followerId, followedId)
}

And this test case:

@Test
fun `users can following a maximum of 100 users`() {
   val followerId = "1"
   val followedId = "2"

   val repo = mockRepository()
   when(repo.countFollowing(followerId)).thenReturn(100)

   service = Service(repo)

   assertThrows {
     service.follow(followerId, followedId)
   }
}

As you see, the number 100 is present in the test name. Is this a good practice? Or should I avoid it and write a "more generic" test?

Answer 1

What’s the requirement? That there be some max number of followers or that it forever be 100?

This begs the question of whether this is a tuning variable that keeps the system happy and subject to change as the system changes, or if this is part of the user experience, like twitters character limit, and not to be changed lightly.

If it’s that unlikely to change then feel free to make it part of the tests name. Then pray that you’re right about the future. Otherwise make life a little easier and keep the places that know it’s value to as few as possible.

If it's likely to change, consider:

fun `users can only follow up to the maximum limit`()

To keep code and tests flexible, make decisions in one place.

Answer 2

If the code under test is not configurable, the unit test should not be configurable either.

If for any reason the code should change, the test will break. That is the idea. At that point you would fix the unit test so that it too is configurable, or you would mock and vary the configuration to ensure the code under test behaves accordingly.

Answer 3

Looks to me like the code has baked in business logic.

There is definitely a code smell here, but it has little to do with the magic number.

This would be better:

fun canAddFollowers(followerId: String) {
  return repo.countFollowing(followerId) >= 100)
}

fun follow(followerId: String, followedId: String) {
  if (!canAddFollowers(followerId)) {
    throw MaxFollowingReached()
  }
  repo.insertFollowingRelationship(followerId, followedId)
}

Better... Now we have spelled out the actual business logic.

• You can now write tests that verifies canAddFollowers to make sure it acts properly for different users. This will doubtlessly include magic numbers, as the business logic when described in a Human language will have magic numbers.

• You can also stub canAddFollowers out in the tests for follow. Here we don't care about what allows/disallows additional followers, only that a user can be followed only if it is allowed, and not otherwise.

You could push this further...

fun canAddFollowers(followerId: String) {
  return repo.countFollowing(followerId) >= 100)
}
fun assertCanAddFollowers(followerId: String) {
  if (!canAddFollowers(followerId)) {
    throw MaxFollowingReached()
  }
}

fun follow(followerId: String, followedId: String) {
  assertCanAddFollowers(followerId)

  repo.insertFollowingRelationship(followerId, followedId)
}

But that is more a matter of taste. In some languages this makes debugging harder, in others it improves consistency.

Answer 4

The magic number 100 should not be listed in the code like that. It should be given a name, defined in one place. The test can then reference that name in the name of the test. "users can follow a maximum of MAXUSERS users". Then, if your requirement/need changes, you can easily make that change in exactly one place.

Answer 5

Ultimately this is not important. I believe the names of methods in your program interface are very important, but this detail in the name of a test case is not. To change the name from 100 to 200 later or to something more generic now is a lot cheaper than arguing about it.