javascript - What functions should I mock during unit testing

Question

I've been reading some articles, and posts here on Stack Overflow about when I should mock a function and when I shouldn't, but I have a case where I'm not sure about what to do.

I have a UserService class which uses dependency injection concept to receive dependencies through its constructor.

class UserService {

 constructor(userRepository) {
    this.userRepository = userRepository;
 }

 async getUserByEmail(userEmail) {
    // would perform some validations to check if the value is an e-mail

    const user = await this.userRepository.findByEmail(email);

    return user;
 }

 async createUser(userData) {
    const isEmailInUse = await this.getUserByEmail(userData.email);

    if(isEmailInUse) {
        return "error";
    } 

    const user = await this.userRepository.create(userData);

    return user;
 }

} 

I want to test if the createUser method works properly, and for my tests, I created a fake userRepository which is basically a object with mocked methods that I will use while instantiating UserService Class

const UserService = require('./UserService.js');

describe("User Service tests", () => {

let userService;
let userRepository;

beforeEach(() => {
    userRepository = {
        findOne: jest.fn(),
        create: jest.fn(),
    }

    userService = new UserService(userRepository);
});

afterEach(() => {
    resetAllMocks();
});

describe("createUser", () => {

    it("should be able to create a new user", async () => {
        const newUserData = { name: 'User', email: '[email protected]' }
        const user = { id: 1, name: 'User', email: '[email protected]' }

        userRepository.create.mockResolvedValue(user);

        const result = await userService.createUser();

        expect(result).toStrictEqual(user);
    })
})

})

Note that in the createUser method, there is a call to the getUserByEmail method which is also a method of UserService class, and that is where I got confused.

Should I mock the getUserByEmail method even it is a method of the class I'm testing? If it is not the correct approach, what should I do?

Answer 1

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...".