Refactorizando tests unitarios hacia tests sin dependencias

En el post anterior sobre tests unitarios y dependencias exponía distintos tipos de escenarios que podemos encontrar al escribir tests unitarios en base a las dependencias del método que necesitamos testear.

Llegábamos a los siguientes tipos:

  1. Métodos sin dependencias: son funciones puras en la que el valor de salida depende únicamente de los valores de la entrada.
  2. Métodos que dependen del estado interno del objeto: son métodos cuyo comportamiento depende del estado en que se encuentre el objeto al que pertenecen, o que modifican el estado interno del objeto al que pertenecen, pero no dependen de nada externo.
  3. Métodos que dependen del resultado de invocar un método en otro objeto, pero cuyo resultado es observable (directa o indirectamente).
  4. Métodos que se limitan a coordinar otros objetos, sin producir ningún resultado observable, ni directa ni indirectamente.

Terminaba diciendo que mi estrategia es concentrarme en testear los dos primeros tipos de métodos y tratando de minimizar (o directamente obviar) los tests para lo otros tipos. Los métodos que sólo verifican interacciones suelo no testearlos con test unitarios y cubrirlos únicamente con tests de integración, pero los métodos del tercer tipo son fácilmente refactorizables a métodos más simples que podamos testear más comódamente.

La técnica es sencilla: separar la lógica de la interacción con otros objetos.

Un caso típico

Vamos a verlo con un ejemplo sencillo, supongamos un método que calcula estadísticas sobre un conjunto de pedidos (sí, esto se puede hacer directamente con LINQ o, mejor aún, en una consulta a la base de datos, pero lo importante es la idea):

public class OrderStatsCalculator
{
  private readonly IOrderRepository repository;

  public OrderStatsCalculator(IOrderRepository repository)
  {
    this.repository = repository;
  }

  public decimal GetAverageAmount()
  {
    var orders = repository.GetOrders();
    var total = 0;
    foreach(var order in orders)
      total += order.Total;
    return total / orders.Length;
  }
}

Para testear este método, la forma “tradicional” que se ve muchas veces es aplicar inyección de dependencias por el constructor para pasar un mock:

[Test]
public void ItReturnsTheAverageAmountOfAllOrders()
{
  var repository = new Mock<Order>();
  var service = new OrderStatsService(repository);
  
  var order1 = Build.Order().AddLine("Fanta", 4).AddLine("Guisantes", 4).Build();
  var order2 = Build.Order().AddLine("Leche", 2).Build();

  repository.Stub(x => x.GetOrders()).Return(new[] { order1, order2 });

  var avg = service.GetAverageAmount();

  Assert.AreEquals(5, avg);
}

Es el típico esquema Arrange/Act/Assert, construyendo primero los objetos (usando un builder para no depender del API de Order en el test), actuándo sobre ellos, y verificando el resultado.

No es que sea un test horrible, pero lo que realmente me interesa testear, que es la fórmula para calcular la media, queda un poco perdido ente la creación del mock y la preparación del valor de retorno de GetOrders.

Una alternativa mejor

Para mejorar esto, vamos a separar la parte de lógica (el cálculo de la media) de la interacción (la obtención de los pedidos). Esto es tan sencillo como aplicar un extract method:

public class OrderStatsCalculator
{
  private readonly IOrderRepository repository;

  public OrderStatsCalculator(IOrderRepository repository)
  {
    this.repository = repository;
  }

  public decimal GetAverageAmount()
  {
    var orders = repository.GetOrders();
    return GetAverageAmount(orders);
  }

  public static decimal GetAverageAmount(Order[] orders)
  {
    var total = 0;
    foreach(var order in orders)
      total += order.Total;
    return total / orders.Length;
  }
}

Con esta pequeña refactorización, podemos pasar a testear la lógica de una manera más sencilla:

[Test]
public void ItReturnsTheAverageAmountOfAllOrders()
{
  var order1 = Build.Order().AddLine("Fanta", 4).AddLine("Guisantes", 4).Build();
  var order2 = Build.Order().AddLine("Leche", 2).Build();

  var avg = OrderStatsService.GetAverageAmount(new[] { order1, order2 });

  Assert.AreEquals(5, avg);
}

Hemos extraído de nuestro método con dependencias sobre otros objetos una función pura que podemos incluso declarar como static, por lo que no nos hace falta ni siquiera crear las dependencias que tiene la clase para poder testear la lógica.

Al hacer este cambio surgen algunas preguntas razonables:

Y el test del método original, ¿no sigue siendo igual que antes?

En general, yo dejaría de testear el método original. No tiene una lógica complicada, son dos líneas de código y lo único que hace es cargar datos para pasarlos a otro método.

¿No indica esto un problema de demasiadas responsabilidades en la clase OrderStatsCalculator?

En este ejemplo concreto, podría ser. Podríamos considerar que esa clase tiene dos responsabilidades: cargar los pedidos y calcular la media. En cualquier caso, aunque separásemos en dos clases, habríamos llegado al mismo punto: un método que contiene lógica y otro que coordina, y mi postura sería la misma: testear el método que tiene lógica y dejar el método que coordina tranquilo y, en todo caso, cubierto con tests de interacción.

Conclusión

A veces nos complicamos demasiado la vida en aras de seguir estrictamente lo que se supone que son “buenas prácticas”, como conseguir un 100% de cobertura de código en los tests.

Una solución simple como ésta permite concentrar la lógica real, la que aporta valor a la aplicación, en métodos fáciles de testear, huyendo de la necesidad de usar herramientas complejas como librerías de mocks y haciendo que cualquiera pueda añadir nuevos casos de tests cómodamente.


2 comentarios en “Refactorizando tests unitarios hacia tests sin dependencias

  1. Creo que la solución propuesta no es adecuada. Tratando de resolver un problema que sólo afecta a los tests (reducir la verbosidad de la creación del mock del repositorio), se cae en otro que afecta directamente a la API pública (abrir algo de la clase que debería ser privado, única y exclusivamente para poder ser testeado) y, por tanto, reduce las posibilidades de refactorización interna (sin afectar a terceros). No sé si es por el caso expuesto, que puede ser demasiado “de juguete”, pero no veo la ganancia y sí la pérdida. Apenas se ganan dos líneas de código que bien se podrían haber ahorrado, por ejemplo, utilizando algún framework como NBuilder o similar o, simplemente, con un método privado que ayude a hacer más legible el test. Sin embargo, como digo, la solución ha causado la apertura de la implementación de la clase hacia afuera. Ahora la superficie de interacción con la clase es mayor, innecesariamente. En su diseño original esa superficie era menor, y lo era por algo. Si un día se decide que el método estático no es suficiente y que se requiere de algún tipo de campo del objeto, se está atado de manos y pies si la dependencia con el método público estático se ha extendido fuera de los límites de la API. Más si, por ejemplo, esa clase que se está desarrollando es parte de una API pública que consume un tercero que no está bajo control. En general, creo que la solución pasa realmente por, o bien simplificar la dependencia desde un IOrderRepository a un simple IEnumerable o bien utilizar alguna técnica o framework que mermita la creación del mock con menos complicaciones.

  2. Hola Abel,

    Muchas gracias por tu comentario, es interesante lo que dices y coincide con las dos preocupaciones más habituales que me han llegado a través de twitter y email :-)

    En lo referente a refactorizar el setup del test, es verdad que usando un extract method puedes dejarlo más legible, pero sigues teniendo el problema de que la lógica de cálculo está acoplada a la obtención de los datos para ese cálculo, por lo que si cambias la forma de cargar datos (por ejemplo porque cambia el API de IOrderRepository), tienes que cambiar los tests sobre el cálculo de la media. Además, sigues teniendo que meter mocks, que no deja de ser una cosa más a manejar.

    En cuanto a cambiar el API pública, es verdad que depende mucho del contexto. La verdad es que siempre estaba pensando en un servicio interno a la aplicación y que, en general, suele estar detrás de un header interface, por lo que los clientes no verían nunca el nuevo método.

    De todas formas, si te preocupa mucho, siempre puedes separar en dos clases los métodos, al estilo de lo que decía al final del post (y en la línea de lo que comentas sobre el IEnumerable). Incluso en este caso concreto podrías hacerlo con un extension method (es la ventaja de refactorizar hacia funciones sin dependencias, te da mucha libertad para moverlas donde quieras :-):

    public static class OrderExtensions
    {
      public static decimal GetAverageAmount(this IEnumerable<Order> orders)
      {
        return ....;
      }
    }
    
    public class OrderStatsCalculator
    {
      //... 
      public decimal GetAverageAmount()
      {
        var orders = repository.GetOrders();
        return orders.GetAverageAmount();
      }
    

    Un saludo,

    Juanma.

Deja un comentario

Tu dirección de correo electrónico no será publicada. Los campos necesarios están marcados *

*

Puedes usar las siguientes etiquetas y atributos HTML: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>