Is this proper way to unit test?

Question

I'm trying to unit test some repositories and have no idea what I'm doing if someone could point in the right direction that would be good, currently, I'm testing behaviour of creating a new user

    [Fact]
    public void Create_New_User_Test()
    {
        var userRepository = new Mock<IUserRepository>();
        var user = new User();
        userRepository.Setup(s => s.CreateNewUser(user)).Returns(Task.FromResult(1));

        Assert.IsAssignableFrom<User>(user);
    }

I'm not sure how to setup the right methods, what to pass, what to assert, is this even right unit test? because it doesn't feel like it

Answer 1

No, it is not. Consider what you are actually testing here. Lets walk through your code.

[Fact]
public void Create_New_User_Test()
{
    var userRepository = new Mock<IUserRepository>();
    // Ok, so userRepository is a _mock_ of an interface that you have somewhere in your code base. 

    var user = new User();
    // Next your create a user, this is probably (Im guessing here but 
    // it doesnt really matter) just a POCO with a set of properties, right?
    // So here you `new` it up.

    userRepository.Setup(s => s.CreateNewUser(user)).Returns(Task.FromResult(1));
    // Next you setup the mock so that when the mock is called with 
    // this specific instance of the user it returns a `Task` with the result `1`.

    Assert.IsAssignableFrom<User>(user);
    // And finally you assert that your user is assignable from the type `User`.   
    // Which will always be true because your user is of the type `User`.
}

The userrepository mock is never used anywhere. And even if you would use it by calling it, you would just be calling a mock not your actual code so you would be testing the mock library - not your code. And you should not be testing the mock library, thats up to the mock library authors to do.

So basically we could simplify your test to this

[Fact]
public void Create_New_User_Test()
{
    var user = new User();
    Assert.IsAssignableFrom<User>(user);
}

Which in turn looks pretty weird right? Its obviously true. Its basically the equivalent of this test

[Fact]
public void Create_New_User_Test()
{
    var truthValue = true;
    Assert.IsTrue(truthValue);
}

And I think that you can immediately see what that test tests. Basically nothing (or well, you could argue that it tests the test library, but again, thats not your code).

While I wish that I could not just tell you what is wrong with your code but also tell you how you should do it right I do agree with some people in the comments section that it seems like you do not currently have a basic grasp of the concept of unit testing and/or mocking (sorry, I know it sounds harsh when people say that but it wont do you any good if I lie to you about this). I would highly recommend that you read and/or some basic tutorials on the topic of unit testing to try to get a bit of a better grasp of what you are actually doing.

Id actually recommend you to drop the mocking first and just focus on feeling comfortable writing unit tests without mocks for simple pieces of code. Then when you feel comfortable doing that you can start adding the mocks. Theres nothing wrong iwth learning to walk before trying to run a marathon :)

Answer 2

The problem here is that repositories are all about database access, and unit tests specifically don't test external dependencies like databases.

You normally use a mock to remove the database interaction from unit tests, but in this case you would be removing the code you're trying to test - which is why you're confused.

UserRepository.CreateNewUser probably looks a bit like this:

public User CreateNewUser(params){
    dataset = CallNewUserStoredProcedure(params);
    return new User(dataset);
}

The first line calls the database, it's out of scope of unit tests. The second just converts the data returned into a user. You could unit test that, if you can mock the call to the database - but there's very little point. You've probably already tested the User constructor code, and while there's a chance that you've accidently written:

return new Fish(dataset);

the compiler would have caught it.

The most likely bugs, and thus the ones you want to test for, are in the stored procedure, and your interface to it. This calls for integration testing, not unit testing. You need a test database, that you can clean at the start of the test. Then call the SP, check you get back what you expect, then check that the database contains the data you expect.

These tests are harder to use, and slower to run than unit tests, but IME they're much more important than unit testing relatively simple functions because there are lots of places for bugs to hide in database code. The good news is that you don't have to run them very often, because this code doesn't change very often. Also, it's often quicker to write integration tests than to manually test the same interface.

Answer 3

Good answers have been given, but I wanted to add another way of understanding unit tests.

My car won't start. It clearly doesn't work. But what do I need to fix? Are the cylinders getting fuel and air but not igniting? Is the fuel tank letting fuel into the engine? Is the exhaust blocked?

It's hard to figure out what's wrong when the only test you have is starting the whole car (this is the equivalent of an integration test). It would be much better if you could test each part separately to see if it performs its duty.

To do this, we test the following: the part gets all the input it needs, and we observe the output it generates. If the output matches our expectations, then the part works.

This follows the same pattern as basic functions in programming:

Input > Processing > Output

Take the example of a calculator, specifically the Add(int,int) function. If we supply it with specific input (1,1) and observe if it's output is what we expect (2), then we can conclude that the Add() method (i.e. the "processing" part) is working correctly.

For the car example, you'd do the same thing in real life.

• If you want to see if the fuel tank works, you take a tank (processing), put fuel in it (input), and see if fuel flows out the other end (observe output).
• If you want to see if the fuel tank can hold the expected amount of fuel, you take a tank (processing), put fuel in it (input), and ensure nothing comes out (observe output), and see if the amount of fuel you've been able to put in is (at least) the expected amount of fuel the tank should be able to hold.
• You can perform some other tests, such as checking if fuel doesn't exist from other places, but the "happy path" testing of the fuel tank is done by checking the intended place for fuel to come out.

When you understand the basic principle behind unit test, i.e. testing the processing based on given input and observed output, it should be more clear how you should structure your unit tests.

---

So what are you testing? If we strip away the parts that don't contribute to the assertions:

var user = new User();
Assert.IsAssignableFrom<User>(user);

That's not a meaningful test, as you've rigorously defined the type of the object whose type you're now checking. It can never fail. You're really only testing your ability to write a test. it's essentially the same as doing:

var sum = 1 + 1;
Assert.AreEqual(sum, 1 + 1);

But I think you intend to test something completely different, when I look at the other part of the test:

var userRepository = new Mock<IUserRepository>();
userRepository.Setup(s => s.CreateNewUser(user)).Returns(Task.FromResult(1));

If your test aims to test the repository, you shouldn't be mocking the repository. You mock the things that aren't being tested right now. You mock the dependencies of the thing you're trying to test.

For example, this mocking would be useful if your were testing e.g. a UserService who has a dependency on an IUserRepository. Assuming the following service:

public class UserService
{
    private readonly IUserRepository _userRepo;

    public UserService(IUserRepository userRepo)
    {
        _userRepo = userRepo;
    }

    public async Task<string> CreateUser(string name)
    {
        var user = new User(name);
        var userId = await _userRepo.CreateNewUser(user);

        return $"Created user with ID {userId}";
    }
}

Your unit test is mocking the right dependency to test if the correct output message is given:

[Fact]
public void Return_message_contains_new_user_ID()
{
    // Mocked dependency

    int newUserID = 123456;

    var userRepository = new Mock<IUserRepository>();
    userRepository.Setup(s => s.CreateNewUser(It.IsAny<User>())).Returns(Task.FromResult(newUserID));

    // Real object to test

    var userService = new UserService(userRepository); // with the mocked dependency

    // Perform the test with controlled input

    var result = userService.CreateUser("John Doe");

    Assert.AreEqual(result, "Created user with ID 123456");
}

Testing return messages isn't the best test (or software design), but this is just a simple example to prove that the assertions are executed on the output of the thing that we're testing.

The same pattern emerges here: we provide the input (John Doe), we mock the dependencies (we force the fake repository to tell the service user 123456 was created) we observe the output (the message), and we test if the message is what we expect it to be.