5

I hope I can make a clear question... I've simplified my code example for this question, I hope it still makes sense.

public class PersonComponent
{
    public bool IsQualified(Person person)
    {
        if (person.PropertyA){ return true; }
        if (person.PropertyB && !person.PropertyC) { return true; }
        if (VeryDifficultFunction(person.Certificates))
        {
            return true;
        }
        return false;
    }

    private bool VeryDifficultFunction(List<Certificate> certificates)
    {}
}

public class Person
{
    public bool PropertyA { get; set; }
    public bool PropertyB { get; set; }
    public bool PropertyC { get; set; }
    public List<Certificate> Certificates { get; set; }
}

public class Certificate
{ }

I have a personcomponent which determines if a given person is qualified or not.

Besides a few simple validations on properties A, B and C there is a complicated decision tree on the list of Certificates of the Person.

Now when I implement this complicated decision in the PersonComponent I would create all unit tests on the PersonComponent (lets say the number of tests is 12). Then, I decide that this knowledge (the complicated decision tree) is too much knowledge for the personcomponent so I refactor it to a new class, the personCertificatescomponent.

Let's say this class now looks like this:

public class PersonCertificatesComponent
{
    public bool GetVeryDifficultFunctionResult(List<Certificate> certificates)
    {}
}

Now, what do I do with my (12) unit tests for this specific code?

Do I MOVE them to operate on the personCertificatescomponent?

PROS:

That seems like a good idea because the personCertificatescomponent is then separately unit tested (which I think is nice if someone wants to reuse this component), AND (whic is a big plus imo) I don't have to setup the unit tests for the personcomponent every time with the right values for PropertyA/B/C so it reaches the VeryDifficultFunction part.

I separated the unit tests for the 2 classes so it makes my code more 'clear' (Somehow I like 2 classes with 8 unit tests better than 1 class with 16 unit tests, but maybe that is something I should unlearn)

CONS:

But now I don't have the tests on the personcomponent anymore. An idea what came up was to unit test if the personCertificatescomponent was used (Mock test) but then I'm tied with my unit test to the implementation of the personcomponent

Or do I COPY them, so the personCertificatescomponent AND the personcomponent are both unit tested? (that seems nice, but copying the tests seems not DRY)

Or do I leave the tests where they are? (pro's and cons inverse of moving the unit tests).

I can't stand it that I can't get a good answer for myself.....

  • it is quite simple : adapt unit tests to new requirements – BЈовић Aug 31 '15 at 9:13
  • 1
    @BЈовић: that misses the point - in the OPs situation, there are no new requirements, and the API of PersonComponent is still the same. – Doc Brown Aug 31 '15 at 9:14
7

When you refactor a class without changing it's API, you want your unit tests to make sure you do not break anything during the refactoring. So you obviously don't want to change your existing unit tests on PersonComponent before the refactoring is complete.

The interesting question is: is it a good idea to change the tests afterwards, to make it possible to test PersonCertificatesComponent in isolation? IMHO this depends on the following questions:

  • how good is your code & branch coverage for PersonCertificatesComponent with the existing tests through PersonComponent? If it is fine, do nothing. If not, consider to add additional tests afterwards for this class, but do not remove the existing tests.

  • are the existing unit tests not running fast enough, or are they overly complicated, and could this be improved by refactoring to simpler, faster tests for PersonCertificatesComponent? If the answer is yes, consider refactoring of the tests, otherwise do nothing.

When you decide to refactor some of your tests because of the second criterion, you should indeed avoid to repeat yourself and do not create code with essentially the same logic twice - this would lead straightforward to maintenance problems. So either verify the code & branch coverage of PersonComponent will not decrease when moving tests from there to PersonCertificatesComponent, or, if you are going to use an existing test as a "template" for a new one, make sure you reuse the same code in a "duplicated" test.

  • I agree with "how good is your code & branch coverage for PersonCertificatesComponent with the existing tests through PersonComponent? If it is fine, do nothing". The only thing is that the tests are more complicated than they COULD be (They would be less complex if I test the PersonCertificatesComponent only), because I have to consider the rest of the PersonComponent flow also. I asked myself if I consider that a 'problem' – Michel Aug 31 '15 at 13:00
  • The third step of the TDD cycle is Refactor; that is cleaning up whatever mess you made while getting things to work. Once your tests are working fine, you should move them to wherever they fit best. – Martin Epsz Aug 31 '15 at 14:20
  • @Michel: if you think the tests become really simpler when moving them from PersonComponent to PersonCertificatesComponent, then do so, but make sure you have still enough coverage for the code in PersonComponent - just as I already wrote above. If you think they become only slightly simpler, then it is probably a matter of taste, experience and available time. – Doc Brown Aug 31 '15 at 14:27
-1

In TDD fashion, you refactor your tests along with your code. First you are supposed to change your test, then your code. But try to ensure you have only 1 or few tests failing at a time. To accomplish, you might end up in an intermediate state, that won't exit in final code.

  • 2
    While I agree, this doesn't realy answer my question – Michel Aug 31 '15 at 12:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.