Why and how implementing initial unit tests in legacy application code

I’m in the process of integrating unit tests in an existing legacy application. In the book “Working with legacy application” and many other books that I read, it was written that you always should write unit tests before starting the process of refactoring existing code or integrating new features, correct bugs, etc...

In the tons of samples that I read, the signature of refactoring methods is never or rarely breaks and the old unit tests still work after a lot of changes. The reason is that author code is not so legacy that the code that I view each day when I work with what I considered “legacy code”.

In the reality, when you have a legacy application, the code is so bad that you must break the signature of methods. If you try to write unit tests with the original method, after just 5 minutes of changes, you will break the entire signature and the firsts tests will be good to be send to the trash.

Just as an example, look at the code below:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace MyCompany.Accouting
{
    public class DataCreator
    {
        public static System.Data.DataSet CreateInvoice(
            System.Data.DataSet customer, 
            System.Data.DataSet order, 
            string mails, 
            ref bool isValid)
        {
            System.Data.DataSet invoice = new System.Data.DataSet();

            int taxGroupId =
                ApplicationException.ShareConnection.ExecuteScalar(
                    "SELECT Id FROM TaxGroup WHERE TaxGroup.IsDefault");

            Application.ShareConnection.ExecuteNonQuery(
                "INSERT INTO Invoice (CustomerId, EffectiveDate) VALUES(?,?)",
                 customer.Tables[0].Rows[0]["Id"], System.DateTime.Now);

            int invoiceId;

            invoiceId = Application.SharedConnection.ExecuteScalar("SELECT @@IDENTITY");

            Application.SharedConnection.ExecuteNonQuery(
                "INSERT INTO InvoiceLine (ProductId, Quantity, InvoiceId) VALUES(?,?,?)", ,
                order.Tables[0].Rows[0]["ProductId"], order.Tables[0].Rows[0]["Quantity"], invoiceId);

            foreach(string mail in mails.Split(';'))
            {
                Application.MailSender.Send(mail);
            }

            isValid = true;

            System.Data.DataRow row = invoice.Tables[0].NewRow();
            row["Id"] = invoiceId;

            invoice.Tables[0].Rows.Add(row);

            return invoice;
        }
    }
}

As you can see, there is a lot of lot of bad code here.

After the refactoring, the method will not be static, ref parameter will be removed, DataSet will be converted to POCOs object, 开发者_运维百科access to global object like “Application” will be replaced by properties injected dynamically and a lot of other changes will be made like implementing interface, review the name of class, namespace and many many other things. In fact, this code is totally a crap that should be throw away and rewritten from scratch.

If I create a unit tests for the original static method, the test will be break immediately when the static keyword will be removed to use the class in a more object oriented manner. Same for the change of DataSet to Poco, etc…

Why create a unit test if in 5 minutes, I will throw away this test? What in this test is helpful?

Which strategy will you use in this case?

Thank you very much.

---

The key item here is to pick the point that you are actually going to unit test. In your case, putting a test on the exact method you are replacing doesn't make sense. Instead a test needs to be created for every point in the application that calls your method to ensure that the specific functionality still works the same.

The reason is that once you have completed refactoring the DataCreator class you will have to go back to all of the areas that call it and change those. Putting tests on those areas prior to making changes will ensure that your functionality is the same.

See below:

public class SomeClass {
  public Boolean DoSomething() {
    OtherClass oc = new OtherClass();
    return oc.DoSomethingElse("param1", "param2") == "true";
  }
}


public class OtherClass {
  public String DoSomethingElse(String param1, String param2) {
      // horrible code here which never uses the second parameter

     return "true";
  }
}

In the above example, you might very well want to refactor DoSomethingElse to change the return type to a boolean value and eliminate the second parameter.

So you start by putting a unit test on the SomeClass.DoSomething method. Then refactor the OtherClass to your hearts content making sure the end result of DoSomething is the same.

Of course, in this situation, you want to make sure you have a unit test for every single thing that calls "DoSomethingElse".

---

Your unit tests will always have to change with signature changes. The best way to go about this is to set up unit tests that test general behavior, and do simple optimizations first.

For instance, start with optimizing the function's code itself (for instance, fixing up the data access & splitting the function up into a couple.)

Then you can move onto signature refactoring, but before you do, make sure the components that use this class have basic expected-results tests so you know if in the process of removing the out parameter, you neglected something in one of the classes that depends on this.

When doing major refactoring your tests will change quite a bit. Sometimes it's enough to have the conceptual tests laid out so you can make sure that with the refactor the usability is similar or you'll know by what tests get deprecated, what needs to be updated in many other dependent classes.

---

Write out the interface the way you want it to be, and write the unit tests against that.

Then call the legacy code from the interface until the tests pass.

Then refactor as needed.

Right?

---

The Unit Test will serve as an active/living record of what functionality was required/performed by the method before you began changing it.

Consider them like checklists for you to think about after you've refactored the method to ensure it still covers what it covered before you refactored it.