Unit Testing Legacy C# Code

Question

How to write a NUnit test for a method like this. Does this method itself warrant refactoring? What is the best approach to deal with scenarios like this in leagacy code?

         public bool DoXYZ()
            {
                ABC abc= new ABC()
                XYZ xyz = new XYZ();
                if (xyz .IsSomeCondition(Session.SessionID))
                { return false; }
                else
                { return abc.IsSomeOtherCondition(SessionID.SessionID); }
            }

This question cannot be answered without showing a bit more code. What is `Session`? What is `SessionID`? How does the containing class look like? — Darin Dimitrov, Jan 19 '11 at 17:52
You first need to know what does this class do. Knowing that you can write a Test to verify that this method is doing what it is suposed to do. Btw, I agree with the others, you should use some Dependency Injection so you can mock those ABC and XYZ. — goenning, Jan 19 '11 at 18:07
You should definately read the following book: http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052 — Steven, Jan 19 '11 at 21:01

score 4 · Answer 1 · answered Jan 19 '11 at 17:53

You will probably need to refactor it to introduce hooks for dependency injection. For example, the class that contains the DoXYZ method can get new properties for ABC and XYZ. These properties could default to instances of ABC and XYZ, but in the unit tests could be replaced by mock versions.

And if you prefer to use IoC, this approach supports that as well

score 1 · Answer 2 · answered Jan 19 '11 at 17:55

I would definitely refactor to inject the session id via a parameter - otherwise you'll have to create the session manually.

Can it be made static? Looks like it, particularly if you inject sessionid.

Also, you're implementing a (short) command dispatcher, which is generally considered an anti-pattern, compared to IoC (see Joel Martinez' answer above).

score 1 · Answer 3 · edited May 23 '17 at 11:47

1

You have two choices:

Refactor your code and use dependency injection as Joel and Chris suggested. This can involve a lot of work but it will make your code clean and easily testable.
Use advanced framework for mocking non injected dependencies as asked in this question.

The way to go in big legacy code is probably using both approaches.

You can also check this article from MSDN magazine.

edited May 23 '17 at 11:47

Community

1
1

answered Jan 19 '11 at 18:02

Ladislav Mrnka

360,892
59
660
670

Interesting article...I haven't heard of Pex before. – Chris B. Behrens Jan 19 '11 at 18:12

score 0 · Answer 4 · answered Jan 20 '11 at 04:21

Given the obfuscated code in the question, I can only offer a few pointers

I see 2 paths - so 2 unit tests needed minimum.
creating collaborators within a method is usually troublesome down the road. Pass in dependencies as ctor / method parameters. This allows you to derive a fake and manipulate branching (e.g. make xyz.IsSomeCondition return false for this test). If ABC and XYZ are simple classes that are easy to setup, then it may not be an immediate problem.. you could live without the Extract Parameter refactoring for now.
extract sessionId into a parameter to eliminate static (global) variable access, which normally retain their previous values causing dependencies between tests

.

public bool DoXYZ(ABC abc, XYZ xyz, Guid sessionId) 
{    if (xyz.IsSomeCondition(sessionId))    
        return false; 

     return abc.IsSomeOtherCondition(sessionId);  
}

Unit Testing Legacy C# Code

4 Answers4