如何测试/更改未经测试和不可测试的代码？

Question

最近，我不得不更改旧系统上的一些代码，其中并非所有代码都有单元测试。
在进行更改之前，我想编写测试，但是每个类都创建了很多依赖项和其他反模式，这使得测试变得非常困难。
显然，我想重构代码以使其更容易测试，编写测试然后更改它。
你会这样做吗？ 或者您会花费大量时间编写难以编写的测试，而这些测试在重构完成后大部分都会被删除吗？

Answer 1

首先，这是一篇很棒的文章，其中包含有关单元测试的技巧。 其次，我发现避免对旧代码进行大量更改的一个好方法是稍微重构它，直到可以测试它为止。 一种简单的方法是使私有成员受到保护，然后覆盖受保护的字段。

例如，假设您有一个类在构造函数期间从数据库加载一些内容。 在这种情况下，您不能只重写受保护的方法，而是可以将数据库逻辑提取到受保护的字段，然后在测试中重写它。

public class MyClass {
    public MyClass() {
        // undesirable DB logic
    }
}

然后

public class MyClass {
    public MyClass() {
        loadFromDB();
    }

    protected void loadFromDB() {
        // undesirable DB logic
    }
}

你的测试看起来像这样：

public class MyClassTest {
    public void testSomething() {
        MyClass myClass = new MyClassWrapper();
        // test it
    }

    private static class MyClassWrapper extends MyClass {
        @Override
        protected void loadFromDB() {
            // some mock logic
        }
    }
}

这是一个不好的例子，因为在这种情况下你可以使用 DBUnit，但我实际上最近在类似的情况下这样做了，因为我想测试一些与数据完全无关的功能已加载，因此非常有效。 我还发现这种成员公开在其他类似情况下很有用，在这些情况下我需要摆脱类中长期存在的某些依赖关系。

如果您正在编写一个框架，我会建议您不要使用此解决方案，除非您真的不介意将成员暴露给框架的用户。

这有点像黑客，但我发现它非常有用。

Answer 2

所以我完全尊重公认的迈克-斯通答案。
因为有总比没有好。

不过，我会提供另一种选择。

我不使用迈克-斯通答案的原因是……（恕我直言）……“创造一个〜更多的技术债务……来处理一个大的技术债务项目”。

这是我的方法。 它是一块垫脚石。

见下文。 我什至还包含了一些过去几年的 PTSD 代码片段。 （带有字符串数组的黑客存储过程调用程序）。 就像，我真的想模拟一些用管道胶带粘在一起的代码。

/* BEFORE */

public interface IEmployeeManager
{ 
    void addEmployee(string lastname, string firstname, string ssn, DateTime dob);
}

public class EmployeeManager : (implements) IEmployeeManager
{

    /* no dependencies injected, just "new it up" or "use static stuff" */

    public void addEmployee(string lastname, string firstname, string ssn, DateTime dob)
    {
            if(dob.Subtract(DateTime.Now).Months < 16)
            {
                throw new ArgumentOutOfRangeException("Too young");
            }


           /* STATIC CALL, :( */  
    
    
   DatabaseHelper.runStoredProcedureWrapper("dbo.uspEmployeeAdd", new String[] {lastname, firstname, ssn, dob.ToString()};


            /* "new it up". :( */

            new EmailSender.sendEmail("[email protected]", "new employee alert subject", string.format("New Employee Added. (LastName='{0}', FirstName='{1}')", lastname, firstname));

    }

}

/* AFTER */

public interface IEmployeeDataHelperWrapper
{ 
    void addEmployeeToDatabase(string lastname, string firstname, string ssn, DateTime dob);
}

public class EmployeeDataHelperWrapper : (implements) IEmployeeDataHelperWrapper
{
    public void addEmployeeToDatabase(string lastname, string firstname, string ssn, DateTime dob)
    {
        DatabaseHelper.runStoredProcedureWrapper("dbo.uspEmployeeAdd", new String[] {lastname, firstname, ssn, dob.ToString()};
    }
}

and

public interface IEmailSenderWrapper
{ 
    void sendEmail(string to, string subject, string body);
}

public class EmailSenderWrapper : (implements) IEmailSenderWrapper
{
    public void sendEmail(string to, string subject, string body);
    {
        new EmailSender.sendEmail(to, subject, body);
    }
}

and and

public interface IEmployeeManager (NO CHANGE)

and （重构）

public class EmployeeManager : (implements) IEmployeeManager
{

    private readonly IEmployeeDataHelperWrapper empDataHelper;

    private readonly IEmailSenderWrapper emailSenderWrapper;

    public EmployeeManager() 
    {
            /* here is the stop-gap "trick", this default constructor does a hard coded "new it up" */
            this(new EmployeeDataHelperWrapper(), new EmailSenderWrapper());
    }

    public EmployeeManager(IEmployeeDataHelperWrapper empDataHelper,
                        IEmailSenderWrapper emailSenderWrapper
    )
    {
        /* now you have this new constructor that injects INTERFACES, that can easier be mocked for unit-test code */

        this.empDataHelper = empDataHelper;
        this.emailSenderWrapper = emailSenderWrapper;
    }


    public void addEmployee(string lastname, string firstname, string ssn, DateTime dob)
    {
            if(dob.Subtract(DateTime.Now).Months < 16)
            {
                throw new ArgumentOutOfRangeException("Too young");
            }

            this.empDataHelper.addEmployeeToDatabase(lastname, firstname, ssn, dob);
            

            this.emailSenderWrapper.sendEmail("[email protected]", "new employee alert subject", string.format("New Employee Added. (LastName='{0}', FirstName='{1}')", lastname, firstname));

    }

}

本质上，我移动代码（我并没有真正“重新编码”）。

而且风险很小（恕我直言）......但是“顺序”，它的相同代码以相同的确切顺序运行。

完美吗？ 不。
但是您开始考虑“注入的依赖项”......然后它们变得可以独立重构。

Answer 3

我不知道为什么你会说一旦重构完成就会删除单元测试。 实际上，您的单元测试套件应该在主构建之后运行（您可以创建一个单独的“测试”构建，它仅在构建主产品之后运行单元测试）。 然后您将立即看到某一部分的更改是否会破坏其他子系统中的测试。 请注意，这与在构建期间运行测试有点不同（正如某些人可能主张的那样） - 一些有限的测试在构建期间很有用，但通常仅仅因为某些单元测试碰巧失败而“崩溃”构建是没有成效的。

如果您正在编写 Java（有可能），请查看 http://www.easymock.org/ -可能有助于减少测试目的的耦合。

Answer 4

我已阅读《有效处理遗留代码》，并且我同意它对于处理“不可测试”的代码非常有用。

有些技术仅适用于编译语言（我正在开发“旧”PHP 应用程序），但我想说本书的大部分内容适用于任何语言。

重构书籍有时会假设代码在重构之前处于半理想或“维护意识”状态，但我所使用的系统并不理想，并且被开发为“边做边学”应用程序，或者作为某些所使用技术的第一个应用程序（我不会为此责怪最初的开发人员，因为我是他们中的一员），因此根本没有测试，而且代码有时很混乱。 本书讨论了这种情况，而其他重构书籍通常不会（嗯，没有到这个程度）。

我应该提到的是，我没有从这本书的编辑或作者那里收到任何钱;)，但我发现它非常有趣，因为遗留代码领域缺乏资源（特别是在我的语言法语中，但那就是另一个故事）。

Answer 5

@valters

我不同意你的说法，即测试不应破坏构建。 测试应该表明应用程序没有为所测试的功能引入新的错误（发现的错误表明缺少测试）。

如果测试没有破坏构建，那么您很容易遇到新代码破坏构建的情况，并且暂时不知道它，即使测试覆盖了它。 失败的测试应该是一个危险信号，表明必须修复测试或代码。

此外，允许测试不破坏构建将导致故障率缓慢上升，直至您不再拥有一组可靠的回归测试。

如果测试经常中断，则可能表明测试的编写方式过于脆弱（依赖于可能更改的资源，例如未正确使用 DB 单元的数据库，或外部 Web 服务）这应该被嘲笑），或者这可能表明团队中有些开发人员没有给予测试适当的关注。

我坚信应该尽快修复失败的测试，就像修复无法尽快编译的代码一样。