what to unit test after refactoring common code into separate class

tags:

unit-testing

views:

answers:

what to unit test after refactoring common code into separate class

Let's say I've got two classes, each fully tested. There's duplication though, so I refactor the common code into a new class. Should I unit test that new class? If so, how?

Here's the options I can see:

don't unit test the new class (it's already fully tested by the original tests)
copy+paste the original tests to the new test class
move the original tests to the new test class and replace original tests with mocks
leave original tests alone, but write more fine-grained tests in the new test class

+1 A:

Hi there.

If it was me then I'd give the new class it's own set of unit tests. Those tests would be a copy + paste of the previous tests which ran against the same code.

Although you're duplicating work, in the long term you need to think about how this new class might change and having those tests in a new unit test class/fixture will be cleaner for you.

Cheers. Jas.

Jason Evans 2010-07-15 14:54:46

but if future refactoring breaks the tests I've double the amount of tests I need to fix

Brad Cupit 2010-07-15 15:03:40

Ahh, sorry I thought you were going to delete the refactored code from the other classes, leaving just that code inside the new class as @Bill mentioned below. I would strongly recommend doing that, rather then leaving duplicate code.

Jason Evans 2010-07-15 15:20:35

Oh I did delete the refactored code from the old classes.What I was trying to say was: if I copy the test methods from the old test class to the new one, and some future refactoring to the new class-under-test breaks things, both the new test class and the old test class will require fixing.

Brad Cupit 2010-07-15 15:28:39

+1 A:

If your refactored class is covered by existing tests, you are probably ok.

Looking at your options, i would also do number 4. If you did some refactoring, you probably made something more generic than it was before. In that case, you could test the generic functionality in a generic manner. So I would do 4 if your refactored solution is more generic. If it is just moving code around to be DRY, i would probably do 1.

hvgotcodes 2010-07-15 14:55:53

wrt #1, I worry a junior engineer may think "sometimes it's ok to skip tests" and follow that pattern, not realizing that this case is unique since it's already tested

Brad Cupit 2010-07-15 15:04:55

a valid concern. But that junior engineer would have to be made to understand that the reason this is ok is because the refactored code is covered. whenever i do something like this I test the refactor separately, fwiw. That way if the refactored code grows i already have tests in place to keep what i have locked down, and a place to put the new tests.

hvgotcodes 2010-07-15 15:21:01

ok cool, so you kind of do #2 and #4?

Brad Cupit 2010-07-15 15:35:33

I try to never copy and paste.

hvgotcodes 2010-07-15 18:31:46

+2 A:

There's duplication though, so I refactor the common code into a new class.

I'm taking that to mean that the two old classes now inherit the common behavior from the new class. If that's the case, then the old test cases should already be testing the common behavior and there's probably no need to write separate tests.

If that's not the case (like if you're creating a utility class whose methods are called by the two original classes), then I'd probably move the tests to their own unit test class so they only need to be in one place.

Bill the Lizard 2010-07-15 14:57:55

It is a utility class. The original classes are practically a one-liner now, and delegate all work to the utility class. So you'd not test the original (one-liner) classes and just focus on the utility class?

Brad Cupit 2010-07-15 15:43:18

@Brad: Yes, if you're just using it as a utility I'd test it once directly.

Bill the Lizard 2010-07-15 16:22:58

@Bill the Lizard: I think I would have agreed with you a few weeks ago, but I've started to realize that lower-level tests (the new test class) are much more fragile in the face of refactoring, but higher-level tests (the old test classes) don't need updates after refactoring the lower-level code, helping you know your refactoring didn't break anything. On the other hand, I hate the alternative of duplicating tests in both the original and the utility class.

Brad Cupit 2010-07-15 16:41:17

@Brad: In the case of inheritance I agree with that. I always try to test the leaf nodes of an inheritance graph. If you write tests for a base class, then any inheriting class has the opportunity to change how something behaves and needs to be tested again. If your original classes are one-liners, then all the logic should be in your utility class now. That should really only need to be tested once if it's called in exactly the same way in both of the original classes. Only their unique logic should need to be tested separately.

Bill the Lizard 2010-07-15 17:24:58

@Bill: I see now what you're saying, and it's a great point. Maybe what I've done is unusual. The old classes each did something different, but did similar lookups. I could have refactored those lookups into a utility class and kept the rest of the logic in the original classes, in which case I would 100% agree with you. Instead, I encapsulated everything in this new utility class, so it completely does the work, and the original classes don't care how. So this utility class does a few different things (each a public method), but shares lots of common code and encapsulates how it's done.

Brad Cupit 2010-07-15 17:48:21

ansaurus

tags:

views:

answers:

what to unit test after refactoring common code into separate class

related questions