Given a function that has already been unit tested, but is present in another unit, should I bother testing it again?

Question

Say, for instance, I have this simple function in my domain layer:

function canCreateNewUsers (principal: User): boolean {
  return principal.isSuperAdmin || principal.isAdmin // || ... a bunch of conditions
}

And this function has already been unit tested in my domain tests.

Now say I have another layer, the service layer, that has a function which creates a user:

// service layer
function createUser (principal: User, email: string) {
  // TODO 
}

which, in full, would look something such as:

// service layer
function createUser (principal: User, email: string) {
  const exists = await userRepository.checkExistsUserWithEmail(email)

  if (exists) {
    // return Error
  }

  if (!canCreateNewUsers(principal) {
    // return Error
  }

  const newUser = createUserWithEmail(email)
  
  await userRepository.insertUser(newUser)

  return newUser
}

Now, my question is, how would I unit test this service layer function properly? Right now, I am considering two options:

1. Stub the canCreateNewUsers function.

PROS:

I don't need to make more (duplicate, because canCreateNewUsers is already tested) tests.

CONS:

I can't think of any major cons, besides having to rename canCreateNewUsers in the test file, if it ever gets renamed.

Also, isn't it weird to stub domain functions/objects?

2. Do not stub the canCreateNewUsers.

PROS:

My tests don't care about implementation details.

CONS:

I will end up writing the same tests I did for canCreateNewUsers, but now for createUsers. Which is okay, if you consider these two to be different units? However, if I ever change canCreateNewUsers (by adding or removing conditions, for example), I will have to mimic the changes in both of the domain and service tests.

What would you guys do in this situation? Thanks.

(Note: I am using fakes for the repository bit)

Answer 1

canCreateNewUsers is an implementation detail for createUsers. Write your tests accordingly.

In other words, your unit tests for createUsers should include some test cases that exercise the permissions function canCreateNewUsers provides, within the context ofcreateUsers.

You don't have to write these additional tests if:

1. You are already satisfied by the original unit tests that canCreateNewUsers functions properly, and
2. You are already convinced that you are calling canCreateNewUsers properly so that it functions correctly in the context in which you are calling it.

But your fellow software developers aren't necessarily thinking about these things. Wouldn't it be nice if you had a test for createUsers that fails if someone else makes a change to canCreateNewUsers that breaks your code?

Answer 2

In this context, canCreateNewUsers is a "foundation brick" that createUsers relies on. createUsers should not have to re-verify "that it works." (That level of testing should have already been run, if necessary ...)

But it should(?), in some reasonable way, "exercise it," to make sure that it actually performs the duties which createUsers relies on. (Such as: "create a dummy user – okay, it works," now "try to create the same user again – okay, it didn't let you," now "delete this dummy user – it works," finally, "try to access the user again – okay, it's not there." Good enough.) Once the basic functionality has been confirmed, createUsers proceeds to focus on testing itself, now having established reasons to be confident that any related errors which are uncovered are probably somewhere within itself.

Answer 3

There are different testing styles and therefore different valid solutions to your problem.

What is usually not a good solution is to mock parts of your business logic, i.e. to artificially split up a DDD bounded context. Mocks are useful for isolating a part of the object graph in order to make it testable. For example, we might replace a repository that performs real database interaction with a mock so that the unit tests run quick enough to enable a TDD-style red–green–refactor loop. However, mocks are very problematic if they reach into the implementation details of the system under test – you should not tear apart intertwined bits of the business logic. Instead, it is best to design the system with clear seams where mocks may or may not be injected. For example, we might add another parameter to the function, and require the caller to wire up the dependencies correctly. Or we might use a full dependency injection framework.

Now back to the code. There are three important test cases we need to specify most of the behaviour:

• Given that I can create users
  When I create a new user
  Then the new user was added to the database
  And I receive the new user
  

• Given that I can create users
  When I try to recreate an existing user
  Then the database is unmodified
  And I receive an error
  

• Given that I cannot create new users
  When I try to create a new user
  Then the database is unmodified
  And I receive an error
  

There are different approaches for implementing these test cases, similar to how there are different mathematical quantifications ∃ (exists) and ∀ (forall):

• For each of these scenarios, we pick one example principal and run the test. If it works for one value, it has a good chance of working for all values.

  This is an entirely reasonable thing to do and works well. If this doesn't properly characterize the behaviour of the system under test, that means my tests are not specific enough. Maybe I need more scenarios: Given I am a super-admin, Given I am a member, …. The good news is that the resulting tests are a good specification of the software, the drawback is that picking all the examples is a rather tedious endeavour.

  The interesting questions is what a “representative example” is, and when we have enough of them. Most likely, you will pick more detailed examples for testing the canCreateNewUsers() function. After all, different principal types are super important in the context of that function. But when testing the createUser() function, we should focus our tests on the value added by that function. This value is e.g. to reject unauthorized principals, but the value of this function isn't to determine when a principal is unauthorized. So in the tests of that function, we would likely pick far fewer examples of different principals.

• Generate multiple values for each input equivalence class in the scenario, and then run them multiple times. We don't really care about the specific example as it should work for all values in the equivalence class.

  This can be an extremely powerful approach especially if the input generation is randomized, since it might find combinations that your code didn't consider. This shifts our way of thinking away from example values towards universal properties of the system under test that should always hold. On the unit test level, this isn't even inefficient: when the unit is properly isolated (e.g. by mocking out database interaction) then running the tests is a CPU-limited problem that can even be parallelized.

  While there probably is a library for such property-based testing in your language, the poor man's approach is to write parametrized tests. The function that creates example values can then be reused across test suites! For example, the following snippet (roughly emulating Mocha.js syntax) reuses the same examples for the canCreateNewUsers() and createUser() tests:

  const examplePrincipalsThatCanCreateNewUsers = () => [
    { name: "admin", principal: ... },
    ...
  ];
  
  const examplePrincipalsThatCannotCreateNewUsers = () => [ ... ];
  
  describe("canCreateNewUsers()", () => {
    for (const { name, principal } of examplePricipalsThatCanCreateNewUsers()) {
      it(`allows ${name} principals`, () => ...);
    }
  
    for (const { name, principal } of examplePricipalsThatCannotCreateNewUsers()) {
      it(`rejects ${name} principals`, () => ...);
    }
  });
  
  describe("createUser()", () => {
    for (const { name, principal } of examplePrincipalsThatCanCreateNewUsers()) {
      it(`lets ${name} principals create new users`, () => ...);
    }
  
    for (const { name, principal } of examplePrincipalsThatCanCreateNewUsers()) {
      it(`rejects duplicate users (principal=${name})`, () => ...);
    }
  
    for (const { name, principal } of examplePrincipalsThatCannotCreateNewUsers()) {
      it(`prevents ${name} principals from creating new users`, () => ...);
    }
  });
  

  Is this duplication? Kind of, yes. A single bug will lead to failures of multiple tests. But that is somewhat unavoidable anyway.

  The big advantage is that we're writing the same number of tests when just picking a single example, but that little bit of parametrization lets us cover a large range of configurations, thus increasing our confidence that the software is behaving correctly. And unlike with creating separate tests for different principal types (e.g. super-admin, admin, member) it's now reasonably simple to keep different tests in sync because you just have to adjust the example generation code.

This sounds like I am advertising the latter approach, testing a large range of configurations. That is not necessarily the case, as that is mostly just appropriate for well-isolated unit tests that can run quickly. This is overkill in a lot of cases. Often, an integration test (without isolating the database) with a few representative examples is sufficient for building confidence in the software – and can even give confidence in the whole system that an isolated unit test cannot. How to prioritize these different testing approaches is largely a matter of personal taste.