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

77

Tengo un debate con un colega programador sobre si es una buena o mala práctica modificar un código de trabajo solo para que sea comprobable (por ejemplo, mediante pruebas unitarias).

Mi opinión es que está bien, dentro de los límites de mantener buenas prácticas de ingeniería de software y orientadas a objetos, por supuesto (no "hacer que todo sea público", etc.).

La opinión de mi colega es que modificar el código (que funciona) solo con fines de prueba es incorrecto.

Solo un ejemplo simple, piense en este fragmento de código que es utilizado por algún componente (escrito en C #):

public void DoSomethingOnAllTypes()
{
    var types = Assembly.GetExecutingAssembly().GetTypes();

    foreach (var currentType in types)
    {
        // do something with this type (e.g: read it's attributes, process, etc).
    }
}

He sugerido que este código se puede modificar para llamar a otro método que hará el trabajo real:

public void DoSomething(Assembly asm)
{
    // not relying on Assembly.GetExecutingAssembly() anymore...
}

Este método toma un objeto de ensamblaje para trabajar, lo que permite pasar su propio ensamblaje para realizar la prueba. Mi colega no pensó que esta sea una buena práctica.

¿Qué se considera una práctica buena y común?

liortal
fuente
55
Su método modificado debe tomar un Typecomo parámetro, no un Assembly.
Robert Harvey
Tienes razón - Tipo o de la Asamblea, el punto era que el código debe permitir el suministro que como parámetro, aunque pueda parecer que esta funcionalidad está allí sólo para ser utilizado para la prueba ...
liortal
55
Su automóvil tiene un puerto ODBI para "pruebas": si las cosas fueran perfectas, no serían necesarias. Supongo que tu auto es más confiable que su software.
mattnz
24
¿Qué seguridad tiene él de que el código "funciona"?
Craige
3
Por supuesto, hazlo. El código probado es estable en el tiempo. Pero tendrá un tiempo mucho más fácil para explicar los errores recientemente introducidos si vinieron con alguna mejora de características en lugar de una solución para la capacidad de prueba
Petter Nordlander

Respuestas:

135

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

  • Es más fácil de mantener.
  • Es más fácil razonar sobre
  • Está más flojamente acoplado y
  • Tiene un mejor diseño general, arquitectónicamente.
Robert Harvey
fuente
3
No mencioné estas cosas en mi respuesta, pero estoy totalmente de acuerdo. Volver a trabajar su código para que sea más comprobable tiende a tener varios efectos secundarios felices.
Jason Swett
2
+1: generalmente estoy de acuerdo. Ciertamente, hay casos claros en los que hacer que el código sea comprobable daña estos otros objetivos beneficiosos, y en esos casos la capacidad de prueba es la prioridad más baja. Pero, en general, si su código no es lo suficientemente flexible / extensible para probar, no será lo suficientemente flexible / extensible para usar o envejecer con gracia.
Telastyn
66
El código comprobable es mejor código. Sin embargo, siempre existe el riesgo inherente de modificar el código que no tiene pruebas a su alrededor, y la forma más fácil y económica de mitigar ese riesgo en el muy corto plazo es dejarlo en paz, lo cual está bien ... hasta que realmente lo necesite para cambiar el código Lo que debe hacer es vender los beneficios de las pruebas unitarias a su colega; Si todos están de acuerdo con las pruebas unitarias, entonces no puede haber argumento de que el código debe ser comprobable. Si no todos están a bordo con las pruebas unitarias, no tiene sentido.
guysherman
3
Exactamente. Recuerdo que estaba escribiendo pruebas unitarias para aproximadamente 10k líneas de origen de mi propio código. Sabía que el código funcionaba perfectamente, pero las pruebas me obligaron a repensar algunas situaciones. Tuve que preguntarme "¿Qué está haciendo exactamente este método?". Encontré varios errores en el código de trabajo solo al mirarlo desde un nuevo punto de vista.
Sulthan
2
Sigo haciendo clic en el botón arriba, pero solo puedo darte un punto. Mi empleador actual es demasiado miope para permitirnos implementar realmente pruebas unitarias, y todavía me estoy beneficiando de escribir conscientemente mi código como si estuviera trabajando en una prueba de manejo.
AmericanUmlaut
59

Hay (aparentemente) fuerzas opuestas en juego.

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

Los proponentes de mantener privados todos los 'detalles de implementación' generalmente están 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 que no está disponible fuera el objetivo final, el único código encapsulado verdadero sería este:

static void Main(string[] args)

¿Su colega propone que este sea el único punto de acceso en su código? ¿Deberían todos los demás códigos ser inaccesibles por personas que llaman externas?

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

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

No me gustaría exponer un método privado que no protege sus invariantes, pero a menudo se puede modificar para que no protegen sus invariantes, y luego exponerla al público (por supuesto, con TDD, lo haces de la otro camino alrededor).

Abrir una API para la comprobabilidad es algo bueno , porque lo que realmente está haciendo es aplicar el Principio de Abierto / Cerrado .

Si solo tiene una única llamada de su API, no sabe qué tan flexible es realmente su API. Lo más probable es que sea bastante inflexible. Las pruebas actúan como un segundo cliente, brindándole comentarios valiosos sobre la flexibilidad de su API .

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

Mark Seemann
fuente
3
+1 Te sientes seguro al exponer un método público cuando protege adecuadamente a sus invariantes.
Shambulator
1
¡Me encanta tu respuesta! :) Cuántas veces escucho: "La encapsulación es el proceso para hacer privados algunos métodos y propiedades". Es como decir que cuando programa en objetos orientados, esto se debe a que programa con objetos. :( No me sorprende que una respuesta tan clara provenga de EL maestro de la inyección de dependencia. Seguramente leeré un par de veces su respuesta para hacerme sonreír sobre mi pobre y antiguo código heredado.
Samuel
Agregué algunos ajustes a tu respuesta. Como comentario aparte, leí su publicación de Encapsulación de propiedades , y la única razón convincente que he escuchado para usar Propiedades automáticas es que cambiar una variable pública a una propiedad pública rompe la compatibilidad binaria; exponerlo como una propiedad automática desde el principio permite la validación u otra funcionalidad interna que se agregará más tarde, sin romper el código del cliente.
Robert Harvey
21

Parece que estás hablando de inyección de dependencia . Es realmente común, y en mi opinión, bastante necesario para la capacidad de prueba.

Para abordar la cuestión más amplia de si es una buena idea modificar el código solo para que sea comprobable, piense de esta manera: el código tiene múltiples responsabilidades, incluyendo a) ser ejecutado, b) ser leído por humanos yc) 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ícate!

Jason Swett
fuente
DI no es la pregunta principal (solo estaba tratando de dar un ejemplo), el punto era si estaría bien proporcionar otro método que originalmente no estaba destinado a ser creado, solo por el bien de la prueba.
liortal
44
Lo sé. Pensé que abordé su punto principal en mi segundo párrafo, con un "sí".
Jason Swett
13

Es un problema de huevo y gallina.

Una de las principales razones por las que es bueno tener una buena cobertura de prueba de su código es que le permite refactorizar sin miedo. ¡Pero se encuentra 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 y lo refactoriza, por cualquier razón, existe el riesgo de que lo rompa.

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

Entonces, diría que, a menos que esta base de código en particular sea bastante estática y no se espere que se haga un trabajo significativo en el futuro, lo que desea hacer es una buena práctica técnica.

Por supuesto, si se trata de una buena práctica comercial , es una gran variedad de gusanos.

Carson63000
fuente
Una preocupación importante Por lo general, hago refactorizaciones compatibles con IDE libremente ya que la posibilidad de romper cualquier cosa allí es muy baja. Si la refactorización está más involucrada, lo haría solo cuando necesite cambiar el código de todos modos. Para cambiar cosas, necesita pruebas para reducir los riesgos, por lo que incluso obtiene valor comercial.
Hans-Peter Störr
El punto es: ¿queremos conservar el código antiguo porque queremos darle la responsabilidad al objeto de consultar el ensamblaje actual o porque no queremos agregar algunas propiedades públicas o cambiar la firma del método? Creo que el código viola el SRP y, por esta razón, debe ser refactorizado sin importar cuán asustados estén los colegas. Claro, si esta es una API pública utilizada por muchos usuarios, tendrá que pensar en una estrategia como implementar una fachada o cualquier cosa que lo ayude a otorgar al código antiguo una interfaz que evite demasiados cambios.
Samuel
7

Esto puede ser solo una diferencia en é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 no es un fin en sí misma. Como tal, aplazaría cualquier refactorización de este tipo hasta que pueda predecir que este código necesitará mantenimiento para promover algún fin comercial.

En el punto que determina que este código se requiere un poco de mantenimiento, que sería un buen momento para refactorizar para la capacidad de prueba. Dependiendo de su caso de negocios, puede ser una suposición válida que todo el código eventualmente requerirá algún mantenimiento, en cuyo caso la distinción que hago con las otras respuestas aquí ( por ejemplo, la respuesta de Jason Swett ) desaparece.

En resumen: 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 al permitir el mantenimiento de una base de código, pero es un requisito comercial modificar la función de su código que debería impulsar su refactorización. Si no existe tal requisito comercial, probablemente sería mejor trabajar en algo que a sus clientes les interese.

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

Aidan Cully
fuente
2

Creo que tu colega está equivocado.

Otros han mencionado las razones por las cuales esto es algo bueno, pero siempre y cuando se te dé permiso para hacerlo, deberías estar bien.

La razón de esta advertencia es que realizar cualquier cambio en el código tiene el costo de tener que volver a probar el código. Dependiendo de lo que haga, este trabajo de prueba en realidad 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.

ozz
fuente
2

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

Cuando estudio los caminos descubiertos y hago un esfuerzo por algunos de ellos, es cuando descubro errores como casos de error que "nunca sucederán". Entonces, sí, modificar el código y verificar la cobertura de la prueba hace que el código sea mejor.

Sam Gamoran
fuente
2

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 si bien 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 la prueba unitaria utilizando las herramientas de prueba de unidad y burla que requieren estos cambios de código. No creo que sean algo saludable, no cuando ves un código que es bastante sencillo y simple se convierte en un desorden de pesadilla de interacciones complejas impulsadas por la necesidad de hacer que cada método de clase se desacople totalmente de todo lo demás. . Y para agregar insulto a la lesión, tenemos muchos argumentos sobre si los métodos privados deben ser probados o no. (por supuesto que deberían, qué '

El problema, por supuesto, está en la naturaleza de las herramientas de prueba.

Hay mejores herramientas disponibles ahora que podrían llevar estos cambios de diseño a la cama para siempre. Microsoft tiene Fakes (nee Moles) que le permiten tropezar 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 utilizó Fakes, reemplazaría la llamada GetTypes con la suya que devolvió datos de prueba válidos e inválidos, lo cual es bastante importante, su cambio sugerido no lo tiene en cuenta.

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 detalladas).

Martin Fowler tiene una discusión sobre esta área en su artículo Los simulacros no son trozos

gbjbaanb
fuente
1

Una buena práctica común es utilizar pruebas de Unidad y registros de depuración . Las pruebas unitarias aseguran que si realiza más cambios en el programa, su antigua funcionalidad no se rompe. 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 fines de prueba. No es inusual cambiar el código para esto. Pero se debe tener cuidado para que el código de producción no se vea afectado por esto. En C ++ y C esto se logra usando el MACRO , que es una 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 dicha disposición está en C #.
Además, cuando agregue código de prueba en su programa, debe estar claramente visibleque esta porción de código se agrega para algún propósito de prueba. O de lo contrario, el desarrollador que intenta comprender el código simplemente va a sudar por esa parte del código.

Manoj R
fuente
1

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

La razón por la que señalo esto es que muchos pasos de inyección de dependencia solo para pruebas fuera de los límites del objeto original. Sé que dijiste " no 'hacer que todo sea 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 destinados.

Ross Patterson
fuente
0

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

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

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

Pero he visto código que no está bien así:

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 cuándo :

si tiene una implementación funcional y por el simple hecho de "implementar capacidades de prueba" realizó los cambios, entonces debe volver a probar su aplicación porque puede haber roto su DoSomething()método.

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

k3b
fuente