As developers we tend to write complicated unit tests that are hard to read and maintain. Maybe it comes from the feeling that test code is not a proper code ? There is some magic in writing 'proper' unit tests. Using word proper might not be even suitable here because how do you define 'proper'. It is the same problem as with the definition of 'unit, everyone has his own definition that depends on the context.

Code below feels like not 'proper'

[Test]
public void IfUserIsAuthenticated_and_Admin_return_View()
{
  AuthServiceMoq.Setup(x => x.CheckAccess(It.IsAny<HttpContextBase>())).Returns(true);
  var sut = new Controller(service.Object, service2.Object, service3.Object, service4.Object, service5.Object, service6.Object, service7.Object)
  var result = sut.Edit(var1, var2, var3);
  Assert.That(result, Is.InstanceOf<ViewResult>());
}

This one was found on production (slight changes applied). I don't really like the function name to start with.

IfUserIsAuthenticated_and_Admin_return_View()
It looks ok but what does it really say ? It does something for scenario with authenticated user, probably an admin user and it returns some View.

Code

  • mocks some kind of authentication service that uses HttpContext
  • sut is a controller with a nice list of dependencies
  • we do execute sut with edit and a list of variables
  • and some view is returned in the process

Does this test looks good ? Does it reads really well ? I don't think so. The mocking logic, as usual, is making it complicated. Creation of the sut is also full of surprises with its number of dependencies.

Improvement

public void EditView_is_accessible_to_authorized_user_of_admin_type()
{
   var sut = CreateSutWithAuth(isAuthorized: true, UserType.Admin);
   var result = sut.Edit(_, _, _);  
   Assert.True(HasAccess(result));
}

EditView_is_accessible_to_authorized_user_of_admin_type()
Not perfect but better, clear message on what is happening here.

Code

  • Sut creation is hidden behind the helper class with an 'interface' exposing only important bits. I need to control if the user is authorized and what kind of type he is to perform this test
  • Edit call is very simple with variables that are not affecting test are hidden behind variable '_'. This is usefull technique to hide non important parts of the code
  • Check for the access is hidden behind custom assertion. We don't need to expose details here.

Simple changes yet making a huge difference.

Share