¿Es una mala práctica modificar el código estrictamente para propósitos de prueba?

Modificar el código para hacerlo más comprobable tiene beneficios más allá de la capacidad de prueba. En general, el código que es más comprobable

• Es más fácil de mantener,
• Es más fácil de razonar,
• está acoplado más flojamente, y
• Tiene un mejor diseño general, arquitectónicamente.

Hay (aparentemente) fuerzas opuestas en juego.

• Por un lado, desea imponer la encapsulación
• Por otro lado, desea poder probar el software

Los defensores de mantener en privado todos los 'detalles de implementación' suelen estar motivados por el deseo de mantener la encapsulación. Sin embargo, mantener todo bloqueado y no disponible es un enfoque mal entendido para la encapsulación. Si mantener todo lo disponible no era el objetivo final, el único código encapsulado verdadero sería este:

static void Main(string[] args)

¿Está su colega proponiendo que este sea el único punto de acceso en su código? ¿Todos los demás códigos deben ser inaccesibles para los llamantes externos?

Difícilmente. Entonces, ¿qué hace que sea correcto hacer públicos algunos métodos? ¿No es, al final, una decisión de diseño subjetiva?

No del todo. Lo que tiende a guiar a los programadores, incluso en un nivel inconsciente, es, de nuevo, el concepto de encapsulación. Se siente seguro al exponer un método público cuando protege adecuadamente sus invariantes .

No quisiera exponer un método privado que no proteja sus invariantes, pero a menudo puedes modificarlo para que sí proteja a sus invariantes, y luego expóngalo al público (por supuesto, con TDD, lo hace al revés).

Abrir una API para verificabilidad es algo bueno , porque eres Realmente hacer es aplicar el Principio Abierto / Cerrado .

Si solo tiene una sola persona que llama a su API, no sabe cuán flexible es realmente su API. Es probable que sea bastante inflexible. Las pruebas actúan como un segundo cliente, brindándole valiosos comentarios sobre la flexibilidad de su API .

Por lo tanto, si las pruebas sugieren que debería abrir su API, hágalo; pero mantenga la encapsulación, no ocultando la complejidad, sino exponiendo la complejidad de una manera segura.

Parece que estás hablando de inyección de dependencia . Es muy común, e IMO, bastante necesario para la prueba.

Para abordar la cuestión más general de si es una buena idea modificar el código para hacerlo verificable, piénselo de esta manera: el código tiene múltiples responsabilidades, incluyendo a) para ser ejecutado, b) para ser leído por humanos, y c) para ser probado. Los tres son importantes, y si su código no cumple con las tres responsabilidades, entonces diría que no es un código muy bueno. ¡Así que modifíquelo!

Es un problema de huevo y gallina.

Una de las razones más importantes por las que es bueno tener una buena cobertura de prueba de su código es que le permite refactorizar sin temor. ¡Pero está en una situación en la que necesita refactorizar el código para obtener una buena cobertura de prueba! Y tu colega tiene miedo.

Veo el punto de vista de tu colega. Tiene un código que (presumiblemente) funciona, y si va a refactorizarlo, por el motivo que sea, existe el riesgo de que lo rompa.

Pero si este es el código que se espera que tenga un mantenimiento y una modificación constantes, correrá ese riesgo cada vez que realice un trabajo en él. Y refactorizar ahora y obtener cierta cobertura de prueba ahora le permitirá correr ese riesgo, bajo condiciones controladas, y obtener el código en una mejor forma para futuras modificaciones.

Por lo tanto, diría que, a menos que este código base en particular sea bastante estático y no se espere que se haga un trabajo significativo en el futuro, lo que usted quiere hacer es una buena práctica técnica.

Por supuesto, si es buena la práctica de negocios es todo un 'no puede de gusanos ..

Esto puede ser solo una diferencia en el énfasis de las otras respuestas, pero yo diría que el código no debe ser refactorizado estrictamente para mejorar la capacidad de prueba. La capacidad de prueba es muy importante para el mantenimiento, pero la capacidad de prueba es no un fin en sí mismo. Como tal, aplazaría cualquier refactorización de este tipo hasta que pueda predecir que este código necesitará mantenimiento para promover el fin de los negocios.

En el punto en que usted determina que este código requerirá un poco de mantenimiento, ese sería un buen momento para refactorizar la capacidad de prueba. Dependiendo de su caso de negocios, puede ser una suposición válida de que el código todos eventualmente requerirá algún mantenimiento, en cuyo caso la distinción que hago con las otras respuestas aquí ( por ejemplo < a href="https://softwareengineering.stackexchange.com/a/199092/14820"> la respuesta de Jason Swett ) desaparece.

Para resumir: la capacidad de prueba por sí sola no es (IMO) una razón suficiente para refactorizar una base de código. La capacidad de prueba tiene un papel valioso para permitir el mantenimiento en una base de código, pero es un requisito comercial modificar la función de su código que debe impulsar su refactorización. Si no existe tal requerimiento comercial, probablemente sería mejor trabajar en algo que les importe a sus clientes.

(El nuevo código, por supuesto, se mantiene activamente, por lo que debe escribirse para que sea verificable).

Creo que tu colega está equivocado.

Otros han mencionado las razones por las que esto ya es algo bueno, pero siempre que se te dé el visto bueno para hacerlo, deberías estar bien.

El motivo de esta advertencia es que hacer cualquier cambio en el código tiene el costo de que el código tenga que volver a probarse. Dependiendo de lo que haga, este trabajo de prueba puede ser un gran esfuerzo por sí mismo.

No es necesariamente su lugar tomar la decisión de refactorizar en lugar de trabajar en nuevas características que beneficiarán a su empresa / cliente.

He utilizado herramientas de cobertura de código como parte de las pruebas unitarias para verificar si se ejercitan todas las rutas a través del código. Como un buen programador / probador por mi cuenta, generalmente cubro el 80-90% de las rutas de código.

Cuando estudio las rutas descubiertas y hago un esfuerzo por algunas de ellas, es cuando descubro errores como los casos de error que "nunca sucederán". Así que sí, modificar el código y verificar la cobertura de las pruebas mejora el código.

Su problema aquí es que sus herramientas de prueba son basura. Debería poder burlarse de ese objeto y llamar a su método de prueba sin cambiarlo, porque aunque este simple ejemplo es realmente simple y fácil de modificar, lo que sucede cuando tiene algo mucho más complicado.

Mucha gente ha modificado su código para introducir IoC, DI y clases basadas en la interfaz simplemente para habilitar las pruebas unitarias utilizando las herramientas de simulación y pruebas unitarias que requieren estos cambios de código. No creo que sean algo saludable, no cuando ves un código bastante sencillo y simple que se convierte en una pesadilla de interacciones complejas impulsadas completamente por la necesidad de que todos los métodos de clase estén totalmente desconectados de todo lo demás. . Y para agregar insulto a la lesión, tenemos muchos argumentos sobre si los métodos privados deben probarse por unidad o no. (Por supuesto que deberían, ¿cuál es el punto de la prueba unitaria si solo prueba una parte de su sistema?), pero estos argumentos están más orientados desde el punto de vista de que es difícil probar esos métodos utilizando las herramientas existentes. Imagínese si su herramienta de prueba podría ejecute pruebas contra un método privado tan fácilmente como uno público, todos los estarían probando sin quejarse.

Por supuesto, el problema está en la naturaleza de las herramientas de prueba.

Hay mejores herramientas disponibles ahora que podrían poner estos cambios de diseño a la cama para siempre. Microsoft tiene Fakes (nee Moles) que le permite eliminar objetos concretos, incluidos los estáticos, por lo que ya no necesita cambiar su código para que se ajuste a la herramienta. En su caso, si usó Fakes, reemplazaría la llamada de GetTypes con la suya que devolvió datos de prueba válidos e inválidos, lo cual es bastante importante, su cambio sugerido no incluye eso en absoluto.

Para responder: su colega tiene razón, pero posiblemente por razones equivocadas. No cambie el código para probar, cambie su herramienta de prueba (o toda su estrategia de prueba para tener más pruebas unitarias de estilo de integración en lugar de tales pruebas de grano fino).

Martin Fowler tiene una discusión en torno a esta área en su artículo Los simulacros no son talones

Una práctica buena y común es utilizar Pruebas de unidades y registros de depuración . Las pruebas unitarias aseguran que si realiza más cambios en el programa, su antigua funcionalidad no se está rompiendo. Los registros de depuración pueden ayudarlo a rastrear el programa en tiempo de ejecución.
A veces sucede que incluso más allá de eso necesitamos tener algo solo para propósitos de prueba. No es raro cambiar el código para esto. Pero el cuidado debe ser tomado de tal manera que el código de producción no se vea afectado por esto. En C ++ y C esto se logra utilizando la MACRO , que es la entidad de tiempo de compilación. Entonces, el código de prueba no aparece en absoluto en el entorno de producción. No sé si tal disposición existe en C #.
Además, cuando agrega el código de prueba en su programa, debe ser claramente visible que esta parte del código se agrega para algún propósito de prueba. O bien, el desarrollador que intenta comprender el código simplemente va a sudar por esa parte del código.

Hay algunas diferencias serias entre tus ejemplos. En el caso de DoSomethingOnAllTypes() , hay una implicación de que do something es aplicable a los tipos en el ensamblaje actual. Pero DoSomething(Assembly asm) indica explícitamente que puede pasarle a cualquier ensamblaje .

La razón por la que señalo esto es que una gran cantidad de dependencia-inyección-para-prueba-solo se encuentra fuera de los límites del objeto original. Sé que dijiste " no 'haciendo todo público' ", pero ese es uno de los errores más grandes de ese modelo, seguido de cerca por este: abrir los métodos del objeto a usos para los que no están diseñados .

Su pregunta no dio mucho contexto en el que su colega argumentó, por lo que hay espacio para la especulación

"mala práctica" o no depende de cómo y cuando se realizan los cambios.

En mi opción, tu ejemplo para extraer un método DoSomething(types) está bien.

Pero he visto un código que no está bien como este:

public void DoSomethingOnAllTypes()
{
  var types = (!testmode) 
      ? Assembly.GetExecutingAssembly().GetTypes() 
      : getTypesFromTestgenerator();

  foreach (var currentType in types)
  {
     if (!testmode)
     {
        // do something with this type that made the unittest fail and should be skipped.
     }
     // do something with this type (e.g: read it's attributes, process, etc).
  }
}

Estos cambios hicieron que el código fuera más difícil de entender porque aumentó el número de posibles rutas de código.

Lo que quiero decir con cómo y cuando :

Si tiene una implementación operativa y por el bien de "implementar capacidades de prueba" hizo los cambios, entonces debe volver a probar su aplicación porque puede haber roto su método DoSomething() .

El if (!testmode) es más difícil de entender y probar que el método extraído.