What functions should I mock during unit testing

You should almost always prefer not to mock parts of the thing you’re supposed to be testing, in this case UserService. To illustrate why, consider these two tests:

1. Provides a test double implementation for findByEmail on the repo object:

   it("throws an error if the user already exists", async () => {
       const email = "[email protected]";
       const user = { email, name: "Foo Barrington" };
       const service = new UserService({
           findByEmail: (_email) => Promise.resolve(_email === email ? user : null),
       });
   
       await expect(service.createUser(user)).rejects.toThrow("User already exists");
   });
   

2. Stubs out the service’s own getUserByEmail method:

   it("throws an error if the user already exists", async () => {
       const email = "[email protected]";
       const user = { email, name: "Foo Barrington" };
       const service = new UserService({});
       service.getUserByEmail = (_email) => Promise.resolve(_email === email ? user : null);
   
       await expect(service.createUser(user)).rejects.toThrow("User already exists");
   });
   

For your current implementation, both pass just fine. But let’s think about how things might change.

---

Imagine we need to enrich the user model getUserByEmail provides at some point:

async getUserByEmail(userEmail) {
    const user = await this.userRepository.findByEmail(userEmail);
    user.moreStuff = await.this.userRepository.getSomething(user.id);
    return user;
}

Obviously we don’t need this extra data just to know whether or not the user exists, so we factor out the basic user object retrieval:

async getUserByEmail(userEmail) {
    const user = await this._getUser(userEmail);
    user.moreStuff = await.this.userRepository.getSomething(user.id);
    return user;
}

async createUser(userData) {
    if (await this._getUser(userData.email)) {
        throw new Error("User already exists");
    }
    return this.userRepository.create(userData);
}

async _getUser(userEmail) {
    return this.userRepository.findByEmail(userEmail);
}

If we were using test 1, we wouldn’t have to change it at all – we’re still consuming findByEmail on the repo, the fact that the internal implementation has changed is opaque to our test. But with test 2, that’s now failing even though the code still does the same things. This is a false negative; the functionality works but the test fails.

In fact you could have applied that refactor, extracting _getUser, prior to a new feature making the need so clear; the fact that createUser uses getUserByEmail directly reflects accidental duplication of this.userRepository.findByEmail(email) – they have different reasons to change.

---

Or imagine we make some change that breaks getUserByEmail. Let’s simulate a problem with the enrichment, for example:

async getUserByEmail(userEmail) {
    const user = await this.userRepository.findByEmail(userEmail);
    throw new Error("lol whoops!");
    return user;
}

If we’re using test 1, our test for createUser fails too, but that’s the correct outcome! The implementation is broken, a user cannot be created. With test 2 we have a false positive; the test passes but the functionality doesn’t work.

In this case, you could say that it’s better to see that only getUserByEmail is failing because that’s where the problem is, but I’d contend that would be pretty confusing when you looked at the code: “createUser calls that method too, but the tests say it’s fine…”.