What is the correct way here to modify/refactor product code to support integration testing?

Question

Given below is my product code in essence and in a simplified representation. This one is like an algorithm for completing a workflow. Each of the steps(private methods) result in changes to a record in the database. Even the external tool invocation results in updating the database via another callback feedback mechanism.

void IWorkFlow.PerformBusinessWorkflowX(Input input) 
{

    PreparePreRequisiteData();

    DoTask_A();

    TriggerExternalToolTo_DoTask_B();   /* external tool invocation */
}

A colleague of mine wanted to write an integration test case for this but without wanting to include the external tool invocation. His goal was to test the rest if the BusinessWorkFlow logic at an integrated level but excluding the external tool. Not a unit test using mocks but an integrated test, where in he validates state of the database pre and post test. In order to achieve the goal he modified the code like this.

void IWorkFlow.PerformBusinessWorkflowX(Input input, bool isTriggerExternalTool) 
{

    PreparePreRequisiteData();

    DoTask_A();

    if(isTriggerExternalTool) 
    {
        TriggerExternalToolTo_DoTask_B();   /* external tool invocation */
    }
}

But, I am somehow not pleased with this style of refactoring. As the invocation of the external tool is an unseparable/integral part of the business workflow, I would rather not have the product code modified to have a boolean flag indicating that the tool invocation is optional.

I thought a better solution(yet ugly(?) because of the nature of it) would be to stick with original method without needing the boolean flag dependency passed in. And instead, have the decision of invoking the tool to support tests embedded inside the TriggerExternalToolTo_DoTask_B() method. Or even by calling it TryTriggerExternalToolTo_DoTask_B().

Something like,

private void TryTriggerExternalToolTo_DoTask_B()
{
    //will be false for integration tests. Any other value then invoke. 
    if(ConfigurationManager.AppSettings["InvokeTool"] != "false") 
    {

    }
}

I am somehow also not in favor of breaking down the IWorkFlow.PerformBusinessWorkflowX(Input input) method into 2 parts(one part doing first 2 steps and the second part doing just the tool invocation exposed via different interface methods), just for the sake of having flexibility in product code and to support test code. And also because since all the steps belong to the single workflow(algorithm).

Questions

1) Am I wrong in saying that introducing a boolean flag into product code just to support tests may not be the best practice? (some have told me that this is what is design for testability. I hope this is not what design for testability actually stands for)

2) Is the seemigly ugly solution of pushing down logic of invocation into the TryTriggerExternalToolTo_DoTask_B() and relying on the appsetting to come to the rescue a better solution?

3) Seeing that I do not want to break down the method(extend the interface) just for the sake of having flexibility, are there any better solutions to the above problem?

PS - Please correct me if I am ignorant and kindly provide open technical suggestions.

Answer 1

No, I would not modify the prototype just for that. And ugh, I hate it when testers monkey with my elegant class design.

The appropriate way to deal with this it to inject a class which executes the external function. If the injected class is null, just skip it. This way the tester can configure his test using AutoFac or whatever DI toolkit you are using; he can turn it on, turn it off, or even substitute his own IExternalToolInvoker class which merely inspects the inputs ( a, b, and c in my example) and ensures that they are correct.

class WorkflowExecutor : IWorkflow
{
    IExternalToolInvoker _invoker;

    public WorkflowExecutor(IExternalToolInvoker invoker)
    {
        _invoker = invoker;
    }

    public void Execute(IInput input)
    {
        PreparePrerequisiteData();
        DoTask_A();

        if (_invoker != null) invoker.TryTriggerExternalTool(a, b, c);
    }
}

Answer 2

I also like @JohnWu s answer. One additional thing to consider: The testers modification/request may be an indication that your class is doing too much. The dependency injection solution would be a way to break the functionality apart.