Está construindo objetos com parâmetros nulos em testes unitários OK?

37

Comecei a escrever alguns testes de unidade para o meu projeto atual. Eu realmente não tenho experiência com isso. Primeiro eu quero "entender" completamente, então atualmente não estou usando nem meu framework IoC nem uma biblioteca de zombaria.

Eu queria saber se há algo errado em fornecer argumentos nulos para os construtores de objetos em testes de unidade. Deixe-me fornecer um código de exemplo:

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){...}
}

Ainda outro exemplo de código de carro (TM), reduzido a apenas as partes importantes para a pergunta. Eu agora escrevi um teste assim:

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));
    }
}

O teste é executado corretamente. SpeedLimit precisa de um Car com um Motor para fazer sua parte. Ele não está interessado em um CarRadio , então eu forneci null para isso.

Eu estou querendo saber se um objeto fornecendo funcionalidade correta sem ser totalmente construído é uma violação do SRP ou um cheiro de código. Eu tenho essa sensação incômoda de que isso acontece, mas speedLimit.IsViolatedBy(motor) também não parece certo - um limite de velocidade é violado por um carro, não por um motor. Talvez eu só precise de uma perspectiva diferente para testes unitários versus código de trabalho, porque toda a intenção é testar apenas uma parte do todo.

A construção de objetos com valor nulo na unidade testa um cheiro de código?

    
por R. Schmitz 20.03.2018 / 11:49
fonte

7 respostas

70

No caso do exemplo acima, é razoável que um Car possa existir sem um CarRadio . Nesse caso, eu diria que não só é aceitável passar um CarRadio nulo, às vezes, eu diria que é obrigatório. Seus testes precisam garantir que a implementação da classe Car seja robusta e não lança exceções de ponteiro nulo quando nenhum CarRadio estiver presente.

No entanto, vamos dar um exemplo diferente - vamos considerar um SteeringWheel . Vamos supor que um Car tenha um SteeringWheel , mas o teste de velocidade não se importa com isso. Nesse caso, eu não passaria um SteeringWheel nulo, já que isso está empurrando a classe Car para lugares para os quais ela não foi projetada. Nesse caso, seria melhor criar algum tipo de DefaultSteeringWheel , o qual (para continuar a metáfora) é bloqueado em uma linha reta.

    
por 20.03.2018 / 12:28
fonte
23

Is constructing objects with null in unit tests a code smell?

Sim. Observe a definição C2 de cheiro de código : "um CodeSmell é uma pista de que algo pode estar errado, não uma certeza."

The test runs fine. SpeedLimit needs a Car with a Motor in order to do its thing. It is not interested in a CarRadio at all, so I provided null for that.

Se você estiver tentando demonstrar que speedLimit.IsViolatedBy(car) não depende do argumento CarRadio passado para o construtor, provavelmente seria mais claro para o futuro mantenedor tornar essa restrição explícita introduzindo um teste duplo que falha no teste se for invocado.

Se, como é mais provável, você está tentando salvar o trabalho de criar um CarRadio que você sabe que não é usado nesse caso, observe que

  • isso é uma violação do encapsulamento (você está deixando que o fato de CarRadio não ser usado vaze para o teste)
  • você descobriu pelo menos um caso em que deseja criar um Car sem especificar um CarRadio

Suspeito strongmente que o código está tentando dizer que você deseja um construtor que pareça

public Car(IMotor motor) {...}

e, em seguida, esse construtor pode decidir o que fazer com o fato de não haver CarRadio

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

ou

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

ou

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

etc.

Its about if passing null to something else while testing SpeedLimit. You can see that the test is called "SpeedLimitTest" and the only Assert is checking a method of SpeedLimit.

Esse é um segundo cheiro interessante - para testar o SpeedLimit, você precisa construir um carro inteiro em torno de um rádio, mesmo que a única coisa com que o SpeedLimit provavelmente se preocupe seja a velocidade .

Isso pode ser uma indicação de que SpeedLimit.isViolatedBy deveria aceitar uma interface de função que o carro implementa

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeed é um nome muito ruim; espero que a sua linguagem de domínio tenha uma melhoria.

Com essa interface, seu teste para SpeedLimit pode ser muito mais simples

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

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
    
por 20.03.2018 / 14:53
fonte
18

Is constructing objects with null in unit tests a code smell?

Não

Nada disso. Ao contrário, se NULL for um valor disponível como argumento (algumas linguagens podem ter construções que não permitem a passagem de um ponteiro nulo), você deve testá-lo.

Outra questão é o que acontece quando você passa NULL . Mas isso é exatamente o que é um teste unitário: garantir que um resultado definido esteja acontecendo quando para cada entrada possível. E NULL é uma entrada em potencial, então você deve ter um resultado definido e você deve ter um teste para isso.

Então se o seu carro é suposto para ser construível sem um rádio, você deve escrever um teste para isso. Se não é e é suposto lançar uma exceção, você deve escrever um teste para isso.

De qualquer forma, sim, você deve escrever um teste para NULL . Seria um cheiro de código se você o deixasse de fora. Isso significaria que você tem um ramo de código não testado que você supostamente estaria magicamente livre de bugs.

Por favor, note que eu concordo com as outras respostas que seu código em teste poderia ser melhorado. No entanto, se o código aprimorado tiver parâmetros que são ponteiros, passar NULL mesmo para o código aprimorado é um bom teste. Eu gostaria de um teste para mostrar o que acontece quando eu passar NULL como um motor. Isso não é um cheiro de código, é o oposto de um cheiro de código. Está testando.

    
por 20.03.2018 / 16:35
fonte
7

Eu pessoalmente hesitaria em injetar nulos. Na verdade, sempre que eu insiro dependências em um construtor, uma das primeiras coisas que faço é gerar um ArgumentNullException se essas injeções forem nulas. Dessa forma, posso usá-los com segurança dentro da minha turma, sem dezenas de verificações de nulos espalhados. Aqui está um padrão muito comum. Observe o uso de readonly para garantir que as dependências definidas no construtor não possam ser definidas como null (ou qualquer outra coisa) posteriormente:

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;
   }
}

Com o acima, eu poderia talvez ter um teste de unidade ou dois, onde injetar nulos, apenas para garantir que minhas verificações de nulos funcionem conforme o esperado.

Dito isto, isso se torna um problema, uma vez que você faz duas coisas:

  1. Programa para interfaces em vez de classes.
  2. Use um framework de simulação (como o Moq for C #) para injetar dependências com mock, em vez de nulos.

Então, para o seu exemplo, você teria:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

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

Mais detalhes sobre o uso do Moq podem ser encontrados aqui .

Claro, se você quiser entendê-lo sem zombar primeiro, você ainda pode fazê-lo, contanto que esteja usando interfaces. Basta criar um CarRadio e um Motor fictícios como segue e injetá-los:

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

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
    
por 20.03.2018 / 12:56
fonte
1

Não está tudo bem, é uma técnica bem conhecida de interrupção de dependências para colocar o código legado em um equipamento de teste. (Veja "Trabalhando Efetivamente com o Código Legado", de Michael Feathers.)

Cheira? Bem ...

O teste não tem cheiro. O teste está fazendo o seu trabalho. Ele está declarando que "este objeto pode ser nulo e o código em teste ainda funciona corretamente".

O cheiro está na implementação. Ao declarar um argumento de construtor, você está declarando que "essa classe precisa dessa coisa para funcionar adequadamente". Obviamente, isso é falso. Qualquer coisa falsa é um pouco malcheiroso.

    
por 21.03.2018 / 16:43
fonte
1

Vamos dar um passo atrás para os primeiros princípios.

A unit test tests some aspect of a single class.

Haverá, no entanto, construtores ou métodos que tomam outras classes como parâmetros. O modo como você lida com isso varia de caso para caso, mas a configuração deve ser mínima, uma vez que é tangencial ao objetivo. Pode haver cenários em que a passagem de um parâmetro NULL seja perfeitamente válida - como um carro sem rádio.

Estou assumindo aqui que estamos apenas testando a linha de corrida para começar (para continuar o tema do carro), mas você pode também querer passar NULL para outros parâmetros para verificar se qualquer código defensivo e mecanismos de entrega de exceção estão funcionando conforme necessário.

Até aí tudo bem. No entanto, e se NULL não for um valor válido. Bem, aqui você está no mundo obscuro de Mocks, stubs, dummies e fakes .

Se tudo isso parece um pouco demais, você já está usando um boneco passando NULL no construtor Car, já que é um valor que potencialmente não será usado.

O que deve ficar claro rapidamente, é que você está potencialmente atirando em seu código com muito tratamento NULL. Se você substituir os parâmetros de classe por interfaces, também abrirá o caminho para o escárnio e DI, etc., o que os tornará mais simples de lidar.

    
por 20.03.2018 / 14:52
fonte
1

Ao analisar seu design, sugiro que um Car seja composto por um conjunto de Features . Um recurso talvez um CarRadio ou EntertainmentSystem , que pode consistir em coisas como CarRadio , DvdPlayer etc.

Então, no mínimo, você teria que construir um Car com um conjunto de Features . EntertainmentSystem e CarRadio seriam implementadores da interface (Java, C # etc) do public [I|i]nterface Feature e isso seria uma instância do padrão de design Composite.

Portanto, você sempre construiria um Car com Features e um teste de unidade nunca instanciaria um Car sem nenhum Features . Embora você possa considerar zombar de um BasicTestCarFeatureSet que tenha um conjunto mínimo de funcionalidade necessário para seu teste mais básico.

Apenas alguns pensamentos.

John

    
por 22.03.2018 / 19:13
fonte