¿Espiar en clase probada es una mala práctica?

13

Estoy trabajando en un proyecto donde las llamadas internas de clase son habituales, pero los resultados son muchas veces valores simples. Ejemplo ( código no real ):

public boolean findError(Set<Thing1> set1, Set<Thing2> set2) {
  if (!checkFirstCondition(set1, set2)) {
    return false;
  }
  if (!checkSecondCondition(set1, set2)) {
    return false;
  }
  return true;
}

Escribir pruebas unitarias para este tipo de código es realmente difícil, ya que solo quiero probar el sistema de condiciones y no la implementación de las condiciones reales. (Lo hago en pruebas separadas). De hecho, sería mejor si aprobara las funciones que implementan las condiciones y en las pruebas simplemente proporciono alguna simulación. El problema con este enfoque es el ruido: utilizamos mucho los genéricos .

Una solución de trabajo; sin embargo, es hacer que el objeto probado sea un espía y simular las llamadas a las funciones internas.

systemUnderTest = Mockito.spy(systemUnderTest);
doReturn(true).when(systemUnderTest).checkFirstCondition(....);

La preocupación aquí es que la implementación del SUT se cambia efectivamente y puede ser problemático mantener las pruebas sincronizadas con la implementación. ¿Es esto cierto? ¿Existe la mejor práctica para evitar este caos de las llamadas a métodos internos?

Tenga en cuenta que estamos hablando de partes de un algoritmo, por lo que dividirlo en varias clases puede no ser una decisión deseada.

java unit-testing allprog
fuente

Respuestas:

15

Las pruebas unitarias deben tratar las clases que prueban como cajas negras. Lo único que importa es que sus métodos públicos se comporten de la manera esperada. Cómo la clase logra esto a través de métodos internos estatales y privados no importa.

Cuando sienta que es imposible crear pruebas significativas de esta manera, es una señal de que sus clases son demasiado poderosas y hacen demasiado. Debe considerar mover algunas de sus funciones a clases separadas que se pueden probar por separado.

Philipp
fuente
1
Comprendí la idea de las pruebas unitarias hace mucho tiempo y he escrito un montón de ellas con éxito. Es simplemente engañoso que algo parezca simple en el papel, se vea peor en el código, y finalmente me enfrento a algo que tiene una interfaz realmente simple pero requiere que me burle de la mitad del mundo alrededor de las entradas.
allprog
@allprog Cuando necesitas hacer muchas burlas, parece que tienes demasiadas dependencias entre tus clases. ¿Intentaste reducir el acoplamiento entre ellos?
Philipp
@allprog si estás en esa situación, el diseño de la clase es el culpable.
itsbruce
Es el modelo de datos que causa el dolor de cabeza. Tiene que cumplir con las reglas de ORM y muchos otros requisitos. Con lógica comercial pura y código sin estado, es mucho más fácil hacer las pruebas unitarias correctamente.
allprog
3
Las pruebas unitarias no necesariamente necesitan manejar el SUT como caja de conexiones. Es por eso que se llaman pruebas unitarias. Al burlarme de las dependencias, puedo influir en el entorno y para saber de qué me tengo que burlar, también tengo que conocer algunos de los aspectos internos. Pero, por supuesto, esto no significa que el SUT deba cambiarse de ninguna manera. El espionaje, sin embargo, permite hacer algunos cambios.
allprog
4

Si ambos findError()y checkFirstCondition()etc. son métodos públicos de su clase, entonces findError()es efectivamente una fachada para la funcionalidad que ya está disponible desde la misma API. No hay nada de malo en eso, pero significa que tiene que escribir pruebas que sean muy similares a las pruebas ya existentes. Esta duplicación simplemente refleja la duplicación en su interfaz pública. Esa no es razón para tratar este método de manera diferente a los demás.

Kilian Foth
fuente
Los métodos internos se hacen públicos solo porque necesitan ser comprobables y no quiero subclasificar el SUT o incluir las pruebas unitarias en la clase SUT como una clase interna estática. Pero entiendo tu punto. Sin embargo, no pude encontrar buenas pautas para evitar este tipo de situaciones. Los tutoriales siempre se quedan en el nivel básico que no tiene nada que ver con el software real. De lo contrario, la razón para espiar es exactamente para evitar la duplicación del código de prueba y hacer que la unidad de prueba tenga un alcance.
allprog
3
No estoy de acuerdo con que los métodos auxiliares deban ser públicos para realizar pruebas unitarias adecuadas. Si el contrato de un método establece que verifica varias condiciones, entonces no hay nada de malo en escribir varias pruebas contra el mismo método público, una para cada "subcontrato". El objetivo de las pruebas unitarias es lograr la cobertura de todo el código, no lograr una cobertura superficial de los métodos públicos a través de una correspondencia de prueba de método 1: 1.
Kilian Foth
Usar solo la API pública para probar es muchas veces significativamente más complejo que probar las piezas internas una por una. No discuto, entiendo que este enfoque no es el mejor y tiene mi retrospectiva que mi pregunta muestra. El mayor problema es que las funciones no son componibles en Java y las soluciones son extremadamente concisas. Pero parece que no hay otra solución para las pruebas unitarias reales.
allprog
4

Las pruebas unitarias deben probar el contrato; Es lo único importante para ellos. Probar cualquier cosa que no sea parte del contrato no es solo una pérdida de tiempo, es una posible fuente de error. Cada vez que ve a un desarrollador que cambia las pruebas cuando cambia un detalle de implementación, deben sonar las alarmas; ese desarrollador puede estar (intencionalmente o no) ocultando sus errores. Probar deliberadamente los detalles de implementación obliga a este mal hábito, lo que hace más probable que los errores se enmascaren.

Las llamadas internas son un detalle de implementación y solo deberían ser de interés para medir el rendimiento . Que no suele ser el trabajo de las pruebas unitarias.

itsbruce
fuente
Suena genial. Pero en realidad, la "cadena" que tengo que escribir y llamarlo código está en un lenguaje que sabe muy poco sobre las funciones. En teoría, puedo describir fácilmente un problema y hacer sustituciones aquí y allá para simplificarlo. En el código tengo que agregar mucho ruido sintáctico para lograr esta flexibilidad que me impide usarlo. Si el método acontiene una llamada al método ben la misma clase, las pruebas de adeben incluir pruebas de b. Y no hay forma de cambiar esto, siempre y cuando bno se pase acomo parámetro. Pero veo que no hay otra solución.
allprog
1
Si bes parte de la interfaz pública, debe probarse de todos modos. Si no es así, no necesita ser probado. Si lo hiciste público solo porque querías probarlo, lo hiciste mal.
itsbruce
Vea mi comentario a la respuesta de @ Philip. Todavía no lo he mencionado, pero el modelo de datos es la fuente del mal. El código puro y sin estado es pan comido.
allprog
2

En primer lugar, me pregunto qué es difícil de probar sobre la función de ejemplo que escribió. Hasta donde puedo ver, simplemente puede pasar varias entradas y verificar para asegurarse de que se devuelve el valor booleano correcto. ¿Qué me estoy perdiendo?

En cuanto a los espías, el tipo de prueba llamada "caja blanca" que utiliza espías y simulacros es órdenes de magnitud más trabajo para escribir, no solo porque hay mucho más código de prueba para escribir, sino cada vez que la implementación es cambiado, también debe cambiar las pruebas (incluso si la interfaz sigue siendo la misma). Y este tipo de pruebas también es menos confiable que las pruebas de recuadro negro, porque debe asegurarse de que todo ese código de prueba adicional sea correcto, y aunque puede confiar en que las pruebas unitarias de recuadro negro fallarán si no coinciden con la interfaz , no puede confiar en eso sobre el uso excesivo de códigos simulacros porque a veces la prueba ni siquiera prueba mucho código real, solo simulacros. Si los simulacros son incorrectos, lo más probable es que sus pruebas tengan éxito, pero su código aún está roto.

Cualquier persona que tenga experiencia con las pruebas de caja blanca puede decirle que es un fastidio escribir y mantener. Junto con el hecho de que son menos confiables, las pruebas de caja blanca son simplemente muy inferiores en la mayoría de los casos.

BT
fuente
Gracias por la nota La función de ejemplo es un orden de magnitud más simple que cualquier cosa que tenga que escribir en un algoritmo complejo. En realidad, la pregunta se parece más a: ¿es problemático probar algoritmos con espías en varias partes? Este no es un código con estado, todo el estado se separa en argumentos de entrada. El problema es el hecho de que quiero probar la función compleja en el ejemplo sin tener que proporcionar parámetros sanos para las subfunciones.
allprog
Con el comienzo de la programación funcional en Java 8, esto se ha vuelto un poco más elegante, pero aún así mantener la funcionalidad en una sola clase puede ser una mejor opción en el caso de algoritmos en lugar de extraer las diferentes partes (solo no útiles) para "usar una vez" clases solo por la capacidad de prueba. A este respecto, los espías hacen lo mismo que los simulacros, pero sin tener que hacer explotar visualmente el código coherente. En realidad, se usa el mismo código de configuración que con los simulacros. Me gusta mantenerme alejado de los extremos, cada tipo de prueba puede ser apropiado en lugares específicos. Probar de alguna manera es mucho mejor que de ninguna manera. :)
allprog
"Quiero probar la función compleja ... sin tener que proporcionar parámetros razonables para las funciones secundarias" - No entiendo lo que quieres decir allí. ¿Qué subfunciones? ¿Estás hablando de las funciones internas que utiliza la 'función compleja'?
BT
Para eso es útil el espionaje en mi caso. Las funciones internas son bastante complejas de controlar. No por el código sino porque implementan algo lógicamente complejo. Mover cosas en una clase diferente es una opción natural, pero esas funciones por sí solas no son útiles en absoluto. Por lo tanto, mantener la clase unida y controlarla mediante la funcionalidad de espionaje resultó ser una mejor opción. Ha trabajado sin problemas durante casi un año y podría resistir fácilmente los cambios de modelo. No he usado este patrón desde entonces, me pareció bueno mencionar que es viable en algunos casos.
allprog
@allprog "lógicamente complejo": si es complejo, necesita pruebas complejas. No hay forma de evitar eso. Los espías solo lo harán más difícil y más complejo para ti. Debería crear subfunciones comprensibles que pueda probar por sí mismas, en lugar de usar espías para probar su comportamiento especial dentro de otra función.
BT