CODEWITHSUNDEEP
c#unit-testingasp.net-mvc-4exception
user2384366
user2384366

Reputation: 1074

Exceptions and testing - how to unit test something for specific exception when I catch them all?

I'm writing MVC4 web application. Generally I try to put "try{}catch{}" block inside every controller method that returns ActionResult to the user. I do it in order to catch all Exceptions and display appropriate message, so user will never see something like:

"Reference not set to an instance of an object"

My controllers usually looks like this:

try
{

}
catch(MyFirstCustomException ex)
{
//set some message for the user and do some cleaning etc.

return ActionResult();
}
catch(MySecondCustomException ex) (and so on...)
{
//set some message for the user and do some cleaning etc.

return ActionResult();
}
catch(Exception ex)
{
//set some message for the user and do some cleaning etc.

return ActionResult();
}

However now I got the following situation: I have AccountController and a LogIn method, I want to write a unit test (using Microsoft Unit Testing Framework), that will assert that user which haven't activated his account, won't be able to log in. I have a special Exception named UserNotActivatedException that is thrown, when such attempt is detected. Problem is - since I catch all my exceptions within a controller, my test will never actually see this exception itself - thus the test will always fail. I managed to bypass the problem by creating special status enum for my model which looks like this:

public enum LoginViewModelStatus
{
NotLoggedIn = 0,
LoginSuccessfull = 1,
LoginFailed = 2,
UserNotActivatedException = 3,
UnknownErrorException = 100
}

and by setting it to a certain value when something is happening (so when I catch my special UserNotActivatedException - I set loginModelStatus to UserNotActivatedException and so on)

My questions:

  1. Are there any nicer alternatives to this?
  2. I'm thinking of using this design in other controllers as well, are there any downfalls here?
  3. Is it good design to use a lot of custom exceptions for displaying messages for users, or would it be better to use more mini if(someCondition){return false;} tests?

Upvotes: 0

Views: 1248

Answers (3)

Dude Pascalou
Dude Pascalou

Reputation: 3179

You could wrap the code inside the try part in order to be able to unit test this part. Here, the unit testable part is simply "wrapped" inside the MyUnitTestableMethod method :

try
{
    MyUnitTestableMethod();
}
catch(MyFirstCustomException ex)
{
    // ...
}
catch(MySecondCustomException ex) (and so on...)
{
    // ...
}
catch(Exception ex)
{
    // ...
}

KISS : Keep It Sanely Simple (or Keep It Simple and Stupid) :)

Upvotes: 1

Alexei Levenkov
Alexei Levenkov

Reputation: 100545

You should test that code returns expected results in all cases and more or less ignore how method does its work.

I.e. in your case Controller converts multiple exceptions into different view - test that when you feed data that causes exception scenario the Controller returns view you expect.

If lower levels of methods used by controller may throw exception - test them too, but this time for throwing particular exceptions.

It is up to you how many exceptions is enough. Good logging of exceptions is probably more important than variety. In most cases you should not show information from exception to a user anyway, but rather something like "Catastrophic error. If need assistance the error was logged with id AB455". All "expected exception" cases should be handled and presented to user as normal flow.

Note that it is ok to throw exceptions from actions as long as you have code that handles all exceptions. Action filter like HandleErrorAttribute can be used to configure exception policy for particular action/whole application.

Upvotes: 0

user1767481
user1767481

Reputation:

It seems as you have your code "too stable". That is, your logic can never generate errors. It is good from a stability point of view but not very testable.

I would in this case have a class to handle the custom logic catch all exceptions generated from that class before returning ActionResult to separate the logic.

class ActionClass
{
    public bool HandleLogin(...)
    {
        ...
    }
}

and use the class like this:

try
{
    ActionClass action = new ActionClass();
    action.HandleLogin(...)

}
// Catchblock here

This will allow you to test the logic.

Upvotes: 0

Related Questions