Need help identifying what unit tests to write

I have the following method and am looking to write effective unit tests that also give me good coverage of the code paths:

public TheResponse DoSomething(TheRequest request)
{
    if (request == null)
        throw new ArgumentNullException("request");

    BeginRequest(request);

    try
    {
        var result = Service.DoTheWork(request.Data);

        var response = Mapper.Map<TheResult, TheResponse>(result);

        return response;
    }
    catch (Exception ex)
    {
        Logger.LogError("This method failed.", ex);

        throw;
    }
    finally
    {
        EndRequest();
    }
}

The Service and Logger objects used by the method are injected into the class constructor (not shown). BeginRequest and EndRequest are implemented in a base class (not shown). And Mapper is the AutoMapper class used for ob开发者_开发知识库ject-to-object mapping.

My question is what are good, effective ways to write unit tests for a method such as this that also provides complete (or what makes sense) code coverage?

I am a believer in the one-test-one-assertion principal and use Moq for a mocking framework in VS-Test (although I'm not hung up on that part for this discusion). While some of the tests (like making sure passing null results in an exception) are obvious, I find myself wondering whether others that come to mind make sense or not; especially when they are exercising the same code in different ways.

Judging from your post/comments you seem to know what tests you should write already, and it pretty much matches what I'd test after first glance at your code. Few obvious things to begin with:

• Null argument exception check
• Mock Service and Logger to check whether they're called with proper data
• Stub Mapper (and potentially Service) to check whether correct results are actually returned

Now, the difficult part. Depending whether your base class is something you have access to (eg. can change it without too much trouble), you can try approach called Extract & Override:

1. Mark BeginRequest/EndRequest as virtual in base class
2. Do nothing with them in derived class
3. Introduce new, testable class deriving from class you want to test; override methods from base class (BeginRequest/EndRequest), making them eg. change some internal value which you can later easily verify

Code could look more or less like this:

Base 
{
    protected virtual void BeginRequest(TheRequest request) { ... }
    protected virtual void EndRequest() { ... }
}

Derived : Base // class you want to test
{
    // your regular implementation goes here
    // virtual methods remain the same
}

TestableDerived : Derived // class you'll actually test
{
    // here you could for example expose some properties 
    // determining whether Begin/EndRequest were actually called,
    // calls were made in correct order and so on - whatever makes
    // it easier to verify later

    protected override void BeginRequest(TheRequest request) { ... }
    protected override void EndRequest() { ... }  
}

You can find more about this technique in Art of Unit Testing book, as well as Working Effectively with Legacy Code. Even tho I believe there are probably more elegant solutions, this one should enable you to test flows and verify general correctness of interactions within DoSomething method.

Problems of course appear when you don't have access to base class and you can't modify its code. Unfortunatelly I don't have "out of book" solution to such situation, but perhaps you could create virtual wrapper around Begin/EndRequest in Derived, and still use extract & override TestableDerived.

The question is tagged TDD, but the question itself is very test-after, which is part of the problem of why you are having a hard time reconciling the two. If you did TDD, a different, more testable design may emerge. Why do the BeginRequest and EndRequest have to be part of inheritance. Can they be part of a TransactionManager that gets injected?

Anyway, if there is a strict need for BeginRequest and EndRequest to be part of the class, you could perhaps make them virtual and test with a sublcass that captures those method calls.

You could also inject some kind of alternative TransactionManager (assuming that that is what makes sense) only under test, that delegates, and in the real code just calls itself.

But when I get to the point of needing those things, I wonder if what the test is really telling me is that those methods need to be decoupled from this code.

I would add assertions that BeginRequest and EndRequest get called, since they are probably pretty important. Other than that, there's probably not anything else too important here. You don't want to test stuff that isn't really that important, like perhaps logging the error. Most your tests would probably focus on what the service is doing.

This is just a small part of a larger paper that I wrote a year or two ago. My company uses this in part as a standard for unit testing. Hope it helps.

Unit Test Golden Rule

“For every public method in the application’s business layer there should be at least 1 unit test. For every class in the application there should be at least 1 test class.”

Items to Unit Test

In general there are four types of methods that are associated with classes;

• Modifiers: Changes one or more
  values (or objects) associated with
  attributes of the object
• Accessors: Returns a value (or object) that depends on the state of the object
• Constructors: Called once when the object is created
• Destructors: Called when the object is destroyed.

When writing unit tests, the developer’s primary concern should be any and all public methods that are employed with in their class at the business layer itself (the methods you will be using in your web layer). By doing so, the developer will also be testing any private methods, and any public methods of the classes within the DAL that are utilized by the class that is being unit tested.

Due to the inherent nature of a class, and the natural processes of a unit test, Constructors and Destructors are inherently built into every unit test. In some cases though, depending on the architecture and the preferences of the developer these two classes can contain custom code, and can be overloaded, in these cases, all possible overload scenarios would also need to be unit tested as well. In a situation where multiple overloads exist for the constructor of the class, each public method needs to be tested once using each of the overloaded constructors.

How to configure/utilize Unit Test Projects

In each solution there should be a Unit Test project that can be saved and shared within whatever architecture is being used to share code. By doing this, and having each developer working on a particular project update and save their unit tests, it will allow other developers (current, and future) to utilize the same unit tests. By thoroughly documenting each unit test as to how it is supposed to function, and under what circumstances, it creates a document that allows developers that are new to the application to become familiar with the structure and intended use of each object. When creating unit tests (visual studio) will automatically create a test class within the test project for each of the methods that the user chooses. This keeps the unit tests organized and segregated from other unit tests from other classes.  

How to Successfully Unit Test

Not including the Constructors and Destructors of a class, the other two method types, Modifiers and Accessors can be further split into seven sub types.

• Modifiers

o Inserts

o Updates

o Deletes

o Polymorphs

• Accessors

o Singular Retrieval

o Group Retrieval

o Mass Retrieval

Inserts, Updates, Deletes, and Polymorphs

Inserts Inserts consist of any situation where data is being added. This is not limited to an SQL database, but may also be applied to other data sources including XML, Arrays and Arraylists, in-memory relational data sources, and even external text documents.

Updates Updates are similar to Inserts except that instead of adding new data, they modify existing data. Like inserts they are not limited to SQL.

Deletes Deletes are the removal or disablement of data. They can also target the same data types as inserts and updates.

Polymorphs Simply put, Polymorphism is the characteristic of being able to assign a different meaning or usage to something in different contexts - specifically, to allow an entity such as a variable, a function, or an object to have more than one form.

Singular, Group, and Mass Retrievals

Singular Retrievals Singular Retrievals involve the return of a single value or record. This can be as simple as a method that returns a user’s name based on the input of a user id.

Group Retrievals Group Retrievals are used when a sub set of a larger group are required. Group retrievals are used for things like filtering.

Mass Retrievals Mass Retrievals involve returning entire sets of data. This may be an entire table from a database, or an entire XML document, or an Array of values.

Method Subtypes and Unit Testing

When working with Modifier methods, the following five rules should be followed. Keeping in mind that each public method should be tested, it does not also mean that each public method needs to be segregated to its own test method.

1) Each Instance of an Insert, Update or Deletion should be preceded and followed by a retrieval, which type of retrieval that is required is based on the type/format of the data that is being modified.

2) Each instance of an Insert should be followed by a Deletion. This insures that any insertion of data is removed.

3) Updates should be preceded and followed by a retrieval, and should also be paired to insure that any data changes that are made are undone.

4) While retrievals may be tested during the test of modifier, they should also be tested independently.

5) While testing retrieval, it is not necessary to verify the actual data, only that data was returned.

It looks to me that testing

var result = Service.DoTheWork(request.Data);

for various incantations of request.Data is the high value part. The rest is infrastructure and I'd be looking at abstracting that to a cleaner (see below for a very rough and ready approach), more testable style to deal with the apparent value of Begin + End Request, this gets tested separately.

As you say there is no need to test that Mapper and logger do their thing.

The last thing you want are lots of brittle tests that cause you pain when you decide to re-work the internals of your objects

public TheResponse DoSomething(TheRequest request)
{
    Guard.NotNull( () => request );

    BeginRequest( (request) =>
    {
        var result = Service.DoTheWork(request.Data);

        var response = Mapper.Map<TheResult, TheResponse>(result);

       return response;
    });
}
// This is a base class method and can be tested elsewhere
void base::BeginRequest(Action<Request> execute)
{
    BeginRequest();
    try                 { execute(request);
                          return Mapper.Map<TheResult, TheResponse>(result); }
    catch(Exception ex) { Logger.Log(ex);
                          throw; }
    finally             { EndRequest(); }
}