¿Está bien construir objetos con parámetros nulos en pruebas unitarias?

37

Comencé a escribir algunas pruebas unitarias para mi proyecto actual. Aunque realmente no tengo experiencia con eso. Primero quiero "obtenerlo" por completo, por lo que actualmente no estoy usando mi marco de trabajo IoC ni una biblioteca de imitación.

Me preguntaba si hay algo malo en proporcionar argumentos nulos a los constructores de objetos en las pruebas unitarias. Permítanme proporcionar un código de ejemplo:

public class CarRadio
{...}


public class Motor
{
    public void SetSpeed(float speed){...}
}


public class Car
{
    public Car(CarRadio carRadio, Motor motor){...}
}


public class SpeedLimit
{
    public bool IsViolatedBy(Car car){...}
}

Otro ejemplo de código de automóvil (TM), reducido a solo las partes importantes para la pregunta. Ahora escribí una prueba algo como esto:

public class SpeedLimitTest
{
    public void TestSpeedLimit()
    {
        Motor motor = new Motor();
        motor.SetSpeed(10f);
        Car car = new Car(null, motor);

        SpeedLimit speedLimit = new SpeedLimit();
        Assert.IsTrue(speedLimit.IsViolatedBy(car));
    }
}

La prueba funciona bien. SpeedLimitnecesita un Carcon un Motorpara hacer lo suyo. No está interesado en CarRadioabsoluto, así que proporcioné nulo para eso.

Me pregunto si un objeto que proporciona la funcionalidad correcta sin estar completamente construido es una violación de SRP o un olor a código. Tengo la sensación persistente de que sí, pero speedLimit.IsViolatedBy(motor)tampoco me parece bien: un automóvil, no un motor, viola un límite de velocidad. Tal vez solo necesito una perspectiva diferente para las pruebas unitarias en comparación con el código de trabajo, porque toda la intención es probar solo una parte del todo.

¿Construir objetos con nulos en pruebas unitarias huele a código?

R. Schmitz
fuente
24
Un poco fuera de tema, pero Motorprobablemente no debería tener ninguno speed. Debe tener throttleay calcular a en torquefunción de la corriente rpmy throttle. Es el trabajo del automóvil usar un Transmissionpara integrar eso en una velocidad actual, y convertirlo en una rpmseñal de retorno a Motor... Pero supongo que de todos modos no estabas en el realismo, ¿verdad?
cmaster
17
El olor del código es que su constructor toma nulo sin quejarse en primer lugar. Si crees que esto es aceptable (puedes tener tus razones para esto): ¡adelante, pruébalo!
Tommy
44
Considere el patrón de objeto nulo . A menudo simplifica el código, a la vez que lo hace seguro para NPE.
Bakuriu
17
Felicitaciones : ha probado que con una nullradio, el límite de velocidad se calcula correctamente. Ahora es posible que desee crear una prueba para validar el límite de velocidad con una radio; por si el comportamiento difiere ...
Matthieu M.
3
No estoy seguro acerca de este ejemplo, pero a veces el hecho de que solo desee probar la mitad de una clase es una pista de que algo debería romperse
Owen

Respuestas:

70

En el caso del ejemplo anterior, es razonable que a Carpueda existir sin a CarRadio. En cuyo caso, diría que no solo es aceptable pasar un valor nulo a CarRadioveces, sino que es obligatorio. Sus pruebas deben garantizar que la implementación de la Carclase sea robusta y no arroje excepciones de puntero nulo cuando no CarRadiohaya ninguna presente.

Sin embargo, tomemos un ejemplo diferente: consideremos a SteeringWheel. Supongamos que a Cartiene que tener un SteeringWheel, pero a la prueba de velocidad realmente no le importa. En este caso, no pasaría un nulo SteeringWheelya que esto está empujando a la Carclase a lugares donde no está diseñada para ir. En este caso, sería mejor crear algún tipo de DefaultSteeringWheel(para continuar con la metáfora) bloqueado en línea recta.

Pete
fuente
44
Y no olvides escribir una prueba unitaria para comprobar que Carno se puede construir un sin un SteeringWheel...
cmaster
13
Puede que me falte algo, pero si es posible e incluso común crear un Carsin a CarRadio, ¿no tendría sentido crear un constructor que no requiera que se pase uno y no tenga que preocuparse por un nulo explícito en absoluto? ?
una tijereta
16
Los autos sin conductor pueden no tener volante. Solo digo. ☺
BlackJack
1
@James_Parsons Olvidé agregar que se trataba de una clase que no tenía verificaciones nulas porque solo se usaba internamente y siempre recibía parámetros no nulos. Otros métodos de Carhubieran arrojado un NRE con un valor nulo CarRadio, pero el código relevante para SpeedLimitno lo haría. En el caso de la pregunta publicada ahora, estaría en lo correcto y, de hecho, lo haría.
R. Schmitz
2
@mtraceur La forma en que todos asumieron que la clase que permite la construcción con un argumento nulo significa que nulo es una opción válida me hizo darme cuenta de que probablemente debería tener controles nulos allí. El problema real que tendría entonces está cubierto por muchas respuestas buenas, interesantes y bien escritas aquí que no quiero arruinar y que me ayudaron. También acepto esta respuesta ahora porque no me gusta dejar las cosas sin terminar y es la más votada (aunque todas fueron útiles).
R. Schmitz
23

¿Construir objetos con nulos en pruebas unitarias huele a código?

Sí. Tenga en cuenta la definición C2 del olor a código : "un CodeSmell es una pista de que algo podría estar mal, no una certeza".

La prueba funciona bien. SpeedLimit necesita un automóvil con motor para hacer lo suyo. No está interesado en un CarRadio en absoluto, por lo que proporcioné nulo para eso.

Si está tratando de demostrar que eso speedLimit.IsViolatedBy(car)no depende del CarRadioargumento pasado al constructor, entonces probablemente sería más claro para el futuro mantenedor hacer explícita esa restricción al introducir un doble de prueba que falla la prueba si se invoca.

Si, como es más probable, solo está tratando de ahorrarse el trabajo de crear uno CarRadioque sabe que no se usa en este caso, debe notar que

  • esto es una violación de la encapsulación (está permitiendo que el hecho de que CarRadiono se use se filtre en la prueba)
  • ha descubierto al menos un caso en el que desea crear un Carsin especificar unCarRadio

Sospecho fuertemente que el código está tratando de decirte que quieres un constructor que se vea como

public Car(IMotor motor) {...}

y luego ese constructor puede decidir qué hacer con el hecho de que no hayCarRadio

public Car(IMotor motor) {
    this(null, motor);
}

o

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

o

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

etc.

Se trata de pasar nulo a otra cosa mientras se prueba SpeedLimit. Puede ver que la prueba se llama "SpeedLimitTest" y el único Assert está comprobando un método de SpeedLimit.

Ese es un segundo olor interesante: para probar SpeedLimit, debe construir un automóvil completo alrededor de una radio, aunque lo único que probablemente le interese a la comprobación de SpeedLimit es la velocidad .

Esta podría ser una pista que SpeedLimit.isViolatedBydebería estar aceptando una interfaz de rol que implementa el automóvil , en lugar de requerir un automóvil completo.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeedes un nombre realmente pésimo; Esperemos que su idioma de dominio tenga una mejora.

Con esa interfaz en su lugar, su prueba SpeedLimitpodría ser mucho más simple

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
VoiceOfUnreason
fuente
En su último ejemplo, supongo que tendría que crear una clase simulada que implemente la IHaveSpeedinterfaz ya que (al menos en c #) no podría crear una instancia de una interfaz.
Ryan Searle
1
Así es: la ortografía efectiva dependerá del sabor de Blub que esté utilizando para sus pruebas.
VoiceOfUnreason
18

¿Construir objetos con nulos en pruebas unitarias huele a código?

No

De ningún modo. Por el contrario, si NULLes un valor que está disponible como argumento (algunos lenguajes pueden tener construcciones que no permiten el paso de un puntero nulo), debe probarlo.

Otra pregunta es qué sucede cuando pasas NULL. Pero eso es exactamente de lo que se trata una prueba unitaria: asegurarse de que un resultado definido suceda cuando para cada entrada posible. Y NULL es una entrada potencial, por lo que debe tener un resultado definido y una prueba para eso.

Entonces, si se supone que su automóvil es construible sin una radio, debe escribir una prueba para eso. Si no es así y se supone que debe lanzar una excepción, debe escribir una prueba para eso.

De cualquier manera, sí, debes escribir una prueba NULL. Sería un olor a código si lo dejaras fuera. Eso significaría que tiene una rama de código no probada que acaba de asumir que mágicamente no tendrá errores.


Tenga en cuenta que estoy de acuerdo con las otras respuestas de que su código bajo prueba podría mejorarse. No obstante, si el código mejorado tiene parámetros que son punteros, pasar NULLincluso por el código mejorado es una buena prueba. Quisiera una prueba que muestre lo que sucede cuando paso NULLcomo motor. Eso no es un olor a código, es lo opuesto a un olor a código. Está probando.

nvoigt
fuente
Parece que estás escribiendo sobre pruebas Caraquí. Si bien no veo nada, consideraría una declaración incorrecta, la pregunta es sobre las pruebas SpeedLimit.
R. Schmitz
@ R.Schmitz No estoy seguro de lo que quieres decir. He citado la pregunta, se trata de pasar NULLal constructor de Car.
nvoigt
1
Se trata de pasar nulo a otra cosa durante la pruebaSpeedLimit . Puede ver que la prueba se llama "SpeedLimitTest" y la única Assertes verificar un método de SpeedLimit. Las pruebas Car, por ejemplo, "si se supone que su automóvil es construible sin una radio, debe escribir una prueba para eso", ocurriría en una prueba diferente.
R. Schmitz
7

Yo personalmente dudaría en inyectar nulos. De hecho, cada vez que inyecto dependencias en un constructor, una de las primeras cosas que hago es generar una excepción ArgumentNullException si esas inyecciones son nulas. De esa manera puedo usarlos con seguridad dentro de mi clase, sin docenas de cheques nulos repartidos. Aquí hay un patrón muy común. Tenga en cuenta el uso de readonly para garantizar que las dependencias establecidas en el constructor no puedan establecerse como nulas (o cualquier otra cosa) después:

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

Con lo anterior, tal vez podría hacer una prueba unitaria o dos, donde inyecto nulos, solo para asegurarme de que mis cheques nulos funcionen como se esperaba.

Dicho esto, esto se convierte en un problema, una vez que haces dos cosas:

  1. Programa para interfaces en lugar de clases.
  2. Use un marco de imitación (como Moq para C #) para inyectar dependencias simuladas en lugar de nulos.

Entonces, para su ejemplo, tendría:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

Puede encontrar más detalles sobre el uso de Moq aquí .

Por supuesto, si quiere comprenderlo sin burlarse primero, aún puede hacerlo, siempre que esté utilizando interfaces. Simplemente cree un CarRadio y un motor ficticios de la siguiente manera e inyéctelos:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
Eterno21
fuente
3
Esta es la respuesta que habría escrito
Liath
1
Incluso iría tan lejos como para decir que los objetos ficticios también tienen que cumplir su contrato. Si IMotortuviera un getSpeed()método, también DummyMotortendría que devolver una velocidad modificada después de un éxito setSpeed.
ComFreek
1

No solo está bien, es una técnica conocida de ruptura de dependencia para obtener código heredado en un arnés de prueba. (Ver "Trabajando efectivamente con código heredado" por Michael Feathers).

¿Huele? Bien...

La prueba no huele. La prueba está haciendo su trabajo. Está declarando que "este objeto puede ser nulo y el código bajo prueba aún funciona correctamente".

El olor está en la implementación. Al declarar un argumento de constructor, estás declarando que "esta clase necesita esto para funcionar correctamente". Obviamente, eso no es cierto. Cualquier cosa falsa es un poco maloliente.

RubberDuck
fuente
1

Retrocedamos a los primeros principios.

Una prueba unitaria prueba algún aspecto de una sola clase.

Sin embargo, habrá constructores o métodos que toman otras clases como parámetros. La forma en que maneje esto variará de un caso a otro, pero la configuración debe ser mínima ya que son tangenciales para el objetivo. Puede haber escenarios en los que pasar un parámetro NULL es perfectamente válido, como un automóvil que no tiene radio.

Asumo aquí, que solo estamos probando la línea de carrera para comenzar (para continuar con el tema del automóvil), pero también es posible que desee pasar NULL a otros parámetros para verificar que cualquier código defensivo y mecanismos de manejo de excepciones funcionen como necesario.

Hasta aquí todo bien. Sin embargo, ¿qué pasa si NULL no es un valor válido? Bueno, aquí estás en el mundo turbio de simulacros, trozos, muñecos y falsificaciones .

Si todo esto parece un poco difícil de asumir, en realidad ya está utilizando un ficticio al pasar NULL en el constructor de automóviles, ya que es un valor que potencialmente no se utilizará.

Lo que debería quedar claro con bastante rapidez, es que potencialmente está disparando su código con una gran cantidad de manejo NULL. Si reemplaza los parámetros de clase con interfaces, también allana el camino para burlarse y DI, lo que hace que estos sean mucho más fáciles de manejar.

Robbie Dee
fuente
1
"Las pruebas unitarias son para probar el código de una sola clase". Suspiro . Eso no es lo que son las pruebas unitarias y seguir esa directriz te llevará a clases infladas o pruebas contraproducentes que obstaculizan.
Ant P
3
Por desgracia, tratar la "unidad" como un eufemismo para la "clase" es una forma muy común de pensar, pero en realidad todo lo que hace es (a) alentar las pruebas de "entrada y salida de basura", lo que puede socavar totalmente la validez de toda la prueba. suites y (b) imponen límites que deberían ser libres de cambiar, lo que puede dañar la productividad Deseo que más personas escuchen su dolor y desafíen esta preconcepción.
Ant P
3
No hay tanta confusión. Pruebe unidades de comportamiento, no unidades de código. No hay nada sobre el concepto de una prueba de unidad, excepto en la mente generalizada de las pruebas de software, que implica que una unidad de prueba está acoplada al concepto distintivo de OOP de una clase. Y una idea errónea muy dañina que es, también.
Ant P
1
No estoy seguro exactamente a qué te refieres: estoy argumentando exactamente lo contrario de lo que estás insinuando. Aplique / simule límites duros (si es absolutamente necesario) y pruebe una unidad de comportamiento . A través de la API pública. Lo que esa API es depende de lo que está construyendo, pero la huella debe ser mínima. Agregar abstracción, polimorfismo o código de reestructuración no debería hacer que las pruebas fallen - "unidad por clase" contradice esto y socava completamente el valor de las pruebas en primer lugar.
Ant P
1
RobbieDee: estoy hablando exactamente de lo contrario. Considere que usted puede ser el que alberga ideas erróneas comunes, vuelva a visitar lo que considera una "unidad" y tal vez profundice un poco más en lo que realmente estaba hablando Kent Beck cuando hablaba de TDD. Lo que la mayoría de la gente llama TDD ahora es una monstruosidad de culto de carga. youtube.com/watch?v=EZ05e7EMOLM
Ant P
1

Echando un vistazo a su diseño, sugeriría que a Carse compone de un conjunto de Features. Una característica puede ser una CarRadio, o EntertainmentSystemque puede consistir en cosas como una CarRadio, DvdPlayeretc.

Entonces, como mínimo, tendrías que construir un Carcon un conjunto de Features. EntertainmentSystemy CarRadioserían implementadores de la interfaz (Java, C #, etc.) del public [I|i]nterface Featurey esto sería una instancia del patrón de diseño Compuesto.

Por lo tanto, siempre construiría un Carcon Featuresy una prueba unitaria nunca crearía una instancia Carsin ninguno Features. Aunque podría considerar burlarse de un BasicTestCarFeatureSetque tiene un conjunto mínimo de funcionalidades requeridas para su prueba más básica.

Solo algunos pensamientos.

John

johnd27
fuente