A construção de objetos com parâmetros nulos nos testes de unidade está OK?

37

Comecei a escrever alguns testes de unidade para o meu projeto atual. Eu realmente não tenho experiência com isso, no entanto. Primeiro, quero "obtê-lo" completamente, portanto, atualmente não estou usando nem minha estrutura de IoC nem uma biblioteca de simulação.

Fiquei me perguntando se há algo errado em fornecer argumentos nulos para 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){...}
}

Outro exemplo de código de carro (TM), reduzido a apenas as partes importantes para a questão. Agora eu escrevi um teste mais ou menos 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 corre bem. SpeedLimitprecisa de um Carcom um Motorpara fazer sua coisa. Ele não está interessado em CarRadionada, por isso forneci nulo para isso.

Gostaria de saber se um objeto que fornece a funcionalidade correta sem ser totalmente construído é uma violação do SRP ou um cheiro de código. Tenho essa sensação incômoda, mas speedLimit.IsViolatedBy(motor)também não parece certa - um limite de velocidade é violado por um carro, não por um motor. Talvez eu só precise de uma perspectiva diferente para testes de unidade e código de trabalho, porque toda a intenção é testar apenas uma parte do todo.

A construção de objetos com null em testes de unidade é um cheiro de código?

R. Schmitz
fonte
24
Um pouco fora de tópico, mas Motorprovavelmente não deveria ter um speed. Ele deve ter um throttlee calcular um com torquebase no atual rpme throttle. O trabalho do carro é usar um Transmissionpara integrar isso à velocidade atual e transformá-lo em um rpmsinal de volta para o Motor... Mas eu acho que você não estava interessado no realismo, estava?
cmaster
17
O cheiro do código é que seu construtor aceita nulo sem reclamar em primeiro lugar. Se você acha que isso é aceitável (você pode ter suas razões para isso): vá em frente, teste!
21418 Tommy
4
Considere o padrão de objeto nulo . Muitas vezes, simplifica o código, além de torná-lo seguro para NPE.
Bakuriu 20/0318
17
Parabéns : você testou que, com um nullrádio, o limite de velocidade é calculado corretamente. Agora você pode criar um teste para validar o limite de velocidade com um rádio; apenas no caso de o comportamento variar ...
Matthieu M. 21/03
3
Não tenho certeza sobre este exemplo, mas às vezes o fato de que você só quer testar meio classe é um indício de que algo deve ser quebrado
Owen

Respostas:

70

No caso do exemplo acima, é razoável que a Carpossa existir sem a CarRadio. Nesse caso, eu diria que não apenas é aceitável passar um nulo CarRadioàs vezes, como também é obrigatório. Seus testes precisam garantir que a implementação da Carclasse seja robusta e não lança exceções de ponteiro nulo quando não CarRadiohouver nenhuma .

No entanto, vamos dar um exemplo diferente - vamos considerar a SteeringWheel. Vamos supor que a Cartenha que ter uma SteeringWheel, mas o teste de velocidade não se importa com isso. Nesse caso, eu não passaria um nulo, SteeringWheelpois isso levaria a Carclasse a lugares onde ela não foi projetada para ir. Nesse caso, seria melhor criar algum tipo de DefaultSteeringWheel(para continuar a metáfora) bloqueado em uma linha reta.

Pete
fonte
44
E não se esqueça de escrever um teste de unidade para verificar se um Carnão pode ser construído sem uma SteeringWheel...
cmaster
13
Talvez esteja faltando alguma coisa, mas se é possível e até comum criar um Carsem um CarRadio, não faria sentido criar um construtor que não exija que um seja passado e que não precise se preocupar com um nulo explícito ?
uma brincadeira
16
Os carros autônomos podem não ter um volante. Apenas dizendo. ☺
BlackJack
11
@James_Parsons Esqueci de acrescentar que tratava-se de uma classe que não tinha verificações nulas porque era usada apenas internamente e sempre recebia parâmetros não nulos. Outros métodos de Carlançariam um NRE com um nulo CarRadio, mas o código relevante para SpeedLimitnão. No caso da pergunta postada agora, você estaria correto e eu realmente faria isso.
R. Schmitz
2
@mtraceur A maneira como todos assumiram que a classe que permite a construção com um argumento nulo significa que nulo é uma opção válida, me fez perceber que eu provavelmente deveria ter verificações nulas lá. O problema real que eu teria então é coberto por muitas respostas boas, interessantes e bem escritas aqui, que não quero estragar e que me ajudaram. Também aceito esta resposta agora porque não gosto de deixar as coisas inacabadas e são as mais votadas (embora todas tenham sido úteis).
R. Schmitz
23

A construção de objetos com null em testes de unidade é um cheiro de código?

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

O teste corre bem. O SpeedLimit precisa de um carro com um motor para fazer o seu trabalho. Não está interessado em um CarRadio, por isso forneci nulo para isso.

Se você está tentando demonstrar que speedLimit.IsViolatedBy(car)não depende do CarRadioargumento transmitido ao 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ê estiver apenas tentando salvar o trabalho de criar um CarRadioque você sabe que não é usado nesse caso, observe que

  • isso é uma violação do encapsulamento (você está deixando o fato de que CarRadionão é usado vazar para o teste)
  • você descobriu pelo menos um caso em que deseja criar um Carsem especificar umCarRadio

Eu suspeito fortemente que o código esteja tentando dizer que você deseja um construtor que se pareça com

public Car(IMotor motor) {...}

e então esse construtor pode decidir o que fazer com o fato de que não há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.

É sobre se passar nulo para outra coisa durante o teste do SpeedLimit. Você pode ver que o teste é chamado "SpeedLimitTest" e o único Assert está verificando um método do 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 que a verificação do SpeedLimit provavelmente se preocupa seja com a velocidade .

Essa pode ser uma dica que SpeedLimit.isViolatedBydeve estar aceitando uma interface de função que o carro implementa , em vez de exigir um carro inteiro.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeedé um nome realmente ruim; espero que seu idioma de domínio melhore.

Com essa interface, seu teste SpeedLimitpode ser muito mais simples

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

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
VoiceOfUnreason
fonte
No seu último exemplo, presumo que você precise criar uma classe simulada que implemente a IHaveSpeedinterface, pois (pelo menos em c #) você não seria capaz de instanciar uma interface em si.
Ryan Searle
11
É isso mesmo - a ortografia efetiva dependerá de qual sabor do Blub você está usando para seus testes.
VoiceOfUnreason
18

A construção de objetos com null em testes de unidade é um cheiro de código?

Não

De modo nenhum. Pelo contrário, se NULLhouver um valor disponível como argumento (alguns idiomas podem ter construções que não permitem a passagem de um ponteiro nulo), você deve testá-lo.

Outra pergunta é o que acontece quando você passa NULL. Mas é exatamente disso que trata um teste de unidade: garantir que um resultado definido esteja acontecendo quando para cada entrada possível. E NULL é uma entrada potencial, portanto, você deve ter um resultado definido e um teste para isso.

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

De qualquer forma, sim, você deve escrever um teste NULL. Seria um cheiro de código se você o deixasse de fora. Isso significa que você tem um ramo de código não testado, que você supôs que magicamente estaria livre de erros.


Observe que concordo com as outras respostas de que seu código em teste pode ser aprimorado. No entanto, se o código aprimorado tiver parâmetros que são ponteiros, passar NULLmesmo para o código aprimorado é um bom teste. Eu gostaria de um teste para mostrar o que acontece quando eu passar NULLcomo motor. Não é um cheiro de código, é o oposto de um cheiro de código. Está testando.

nvoigt
fonte
Parece que você está escrevendo sobre testes Caraqui. Embora não veja nada que consideraria uma afirmação errada, a questão é sobre o teste SpeedLimit.
R. Schmitz
@ R.Schmitz Não sei o que você quer dizer. Eu citei a pergunta, é sobre passar NULLpara o construtor de Car.
Nvoigt 21/0318
11
É sobre se passar nulo para outra coisa durante o testeSpeedLimit . Você pode ver que o teste é chamado "SpeedLimitTest" e o único Assertestá verificando um método de SpeedLimit. Testes Car- por exemplo "se seu carro for construtível sem rádio, você deve escrever um teste para isso" - aconteceria em um teste diferente.
R. Schmitz
7

Eu, pessoalmente, hesitaria em injetar nulos. De fato, sempre que injeto dependências em um construtor, uma das primeiras coisas que faço é gerar uma ArgumentNullException se essas injeções forem nulas. Dessa forma, eu posso usá-los com segurança dentro da minha classe, sem dezenas de cheques 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 nulas (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 exposto, talvez eu possa fazer um teste de unidade ou dois, onde injeto nulos, apenas para garantir que minhas verificações nulas funcionem conforme o esperado.

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

  1. Programe para interfaces em vez de classes.
  2. Use uma estrutura de simulação (como Moq para C #) para injetar dependências simuladas 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 .

Obviamente, se você quiser entendê-lo sem zombar primeiro, ainda poderá fazê-lo, desde que esteja usando interfaces. Simplesmente crie um CarRadio e Motor fictício da seguinte forma e injete-os:

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

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
Eternal21
fonte
3
Esta é a resposta que eu teria escrito
Liath 20/18
11
Eu diria até que os objetos fictícios também precisam cumprir seu contrato. Se IMotortivesse um getSpeed()método, também DummyMotorseria necessário retornar uma velocidade modificada após um êxito setSpeed.
ComFreek
1

Não está tudo bem, é uma técnica bem conhecida de quebra de dependência para colocar o código legado em um equipamento de teste. (Consulte “Trabalhando efetivamente com o código legado”, de Michael Feathers.)

Cheira? Bem...

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

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

Pato de borracha
fonte
1

Vamos voltar aos primeiros princípios.

Um teste de unidade testa algum aspecto de uma única classe.

No entanto, haverá construtores ou métodos que tomam outras classes como parâmetros. A maneira como você lida com isso varia de caso para caso, mas a configuração deve ser mínima, pois é tangencial ao objetivo. Pode haver situações 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 continuar com o tema do carro), mas você também pode passar NULL para outros parâmetros para verificar se algum código defensivo e mecanismos de tratamento de exceções estão funcionando como requeridos.

Por enquanto, tudo bem. No entanto, e se NULL não for um valor válido. Bem, aqui você está no mundo sombrio de zombarias, tocos, manequins e falsificações .

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

O que deve ficar claro rapidamente, é que você está potencialmente executando o código com muita manipulação NULL. Se você substituir os parâmetros de classe por interfaces, também abrirá o caminho para zombaria e DI etc, o que os torna muito mais simples de lidar.

Robbie Dee
fonte
11
"Os testes de unidade são para testar o código de uma única classe." Suspiro . Não é isso que são os testes de unidade, e seguir essa diretriz o levará a classes inchadas ou a testes contraproducentes e dificultadores.
Ant P
3
tratar "unidade" como um eufemismo para "classe" é, infelizmente, uma maneira muito comum de pensar, mas, na realidade, tudo isso faz é (a) incentivar testes de "entrada e saída de lixo", que podem minar totalmente a validade de todo o teste suítes e (b) impor limites que devem ser livres de alterações, o que pode prejudicar a produtividade. Eu gostaria que mais pessoas ouvissem sua dor e desafiassem esse preconceito.
Ant
3
Não há essa confusão. Teste unidades de comportamento, não unidades de código. Não há nada sobre o conceito de um teste de unidade - exceto na mente generalizada dos cultos de carga dos testes de software - que implica que uma unidade de teste esteja acoplada ao conceito distintamente OOP de uma classe. E um equívoco muito prejudicial que também é.
Ant
11
Não sei exatamente o que você quer dizer - estou discutindo exatamente o oposto do que você está sugerindo. Stub / mock limites rígidos (se for absolutamente necessário) e teste uma unidade comportamental . Através da API pública. O que é essa API depende do que você está construindo, mas a área útil deve ser mínima. Adicionar abstração, polimorfismo ou código de reestruturação não deve causar falhas nos testes - a "unidade por classe" contradiz isso e mina totalmente o valor dos testes em primeiro lugar.
Ant
11
RobbieDee - Estou falando exatamente do contrário. Considere que você pode ser o único a ter conceitos errôneos comuns, revisitar o que considera uma "unidade" e talvez se aprofundar um pouco mais sobre o que Kent Beck estava realmente falando quando falou sobre TDD. O que a maioria das pessoas chama de TDD agora é uma monstruosidade de culto à carga. youtube.com/watch?v=EZ05e7EMOLM
Ant P
1

Dando uma olhada no seu design, eu sugeriria que a Caré composto por um conjunto de Features. Um recurso pode ser um CarRadio, ou EntertainmentSystemque pode consistir em coisas como um CarRadio, DvdPlayeretc.

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

Portanto, você sempre construiria um Carcom Featurese um teste de unidade nunca instanciaria um Carsem nenhum Features. Embora você possa considerar zombar de um BasicTestCarFeatureSetque tenha um conjunto mínimo de funcionalidades necessárias para o teste mais básico.

Apenas alguns pensamentos.

John

johnd27
fonte