É uma prática ruim modificar o código estritamente para fins de teste

77

Eu tenho um debate com um colega programador sobre se é uma prática boa ou ruim modificar um trecho de código apenas para torná-lo testável (por meio de testes de unidade, por exemplo).

Minha opinião é de que está tudo bem, dentro dos limites de manter boas práticas orientadas a objetos e de engenharia de software (não "tornar tudo público", etc).

A opinião do meu colega é que modificar o código (que funciona) apenas para fins de teste está errado.

Apenas um exemplo simples, pense neste pedaço de código usado por algum componente (escrito em 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).
    }
}

Sugeri que esse código possa ser modificado para chamar outro método que fará o trabalho real:

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

Esse método utiliza um objeto Assembly para trabalhar, possibilitando passar seu próprio Assembly para fazer os testes. Meu colega não achou que isso fosse uma boa prática.

O que é considerado uma prática boa e comum?

liortal
fonte
5
Seu método modificado deve usar a Typecomo parâmetro, não como Assembly.
Robert Harvey
Você está certo - Tipo ou montagem, o ponto era que o código deveria permitir fornecer isso como um parâmetro, embora possa parecer que essa funcionalidade esteja lá apenas para ser usada para teste ...
liortal
5
Seu carro possui uma porta ODBI para "teste" - se as coisas fossem perfeitas, não seria necessário. Meu palpite é que seu carro é mais confiável que o software dele.
mattnz
24
Que garantia ele tem de que o código "funciona"?
Craige 23/05
3
Claro - faça isso. O código testado é estável ao longo do tempo. Mas você terá um tempo muito mais fácil de explicar erros recentemente introduzidas, se eles vieram com algumas melhorias recurso ao invés de uma correção para testability
Petter Nordlander

Respostas:

135

Modificar o código para torná-lo mais testável tem benefícios além da testabilidade. Em geral, código mais testável

  • É mais fácil de manter,
  • É mais fácil argumentar sobre,
  • É mais frouxamente acoplado e
  • Tem um design geral melhor, arquitetonicamente.
Robert Harvey
fonte
3
Não mencionei essas coisas na minha resposta, mas concordo totalmente. Retrabalhar seu código para ser mais testável tende a ter vários efeitos colaterais felizes.
21713 Jason Swett
2
+1: eu geralmente concordo. Certamente há casos claros em que tornar o código testável prejudica esses outros objetivos benéficos e, nesses casos, a testabilidade é a prioridade mais baixa. Mas, em geral, se seu código não for flexível / extensível o suficiente para testar, ele não será flexível / extensível o suficiente para usar ou envelhecer normalmente.
Telastyn
6
Código testável é melhor código. No entanto, sempre existe um risco inerente à modificação do código que não possui testes, e a maneira mais fácil e barata de atenuar esse risco a curto prazo é deixá-lo em paz, o que é bom ... até que você realmente precise para mudar o código. O que você precisa fazer é vender os benefícios do teste de unidade ao seu colega; se todos estiverem envolvidos no teste de unidade, não haverá argumento de que o código precise ser testável. Se nem todos estão a bordo com testes de unidade, não há sentido.
guysherman
3
Exatamente. Lembro que estava escrevendo testes de unidade para cerca de 10 mil linhas de código-fonte. Eu sabia que o código estava funcionando perfeitamente, mas os testes me forçaram a repensar algumas situações. Eu tive que me perguntar "O que exatamente esse método está fazendo?". Eu encontrei vários erros no código de trabalho apenas olhando para ele de um novo ponto de vista.
Sulthan
2
Continuo clicando no botão para cima, mas só posso lhe dar um ponto. Meu empregador atual é míope demais para nos permitir realmente implementar testes de unidade, e ainda estou me beneficiando de escrever conscientemente meu código como se estivesse trabalhando com testes.
AmericanUmlaut
59

Existem (aparentemente) forças opostas em jogo.

  • Por um lado, você deseja aplicar o encapsulamento
  • Por outro lado, você deseja poder testar o software

Os proponentes de manter todos os 'detalhes da implementação' privados são geralmente motivados pelo desejo de manter o encapsulamento. No entanto, manter tudo bloqueado e indisponível é uma abordagem incompreendida do encapsulamento. Se manter tudo indisponível fosse o objetivo final, o único código encapsulado verdadeiro seria este:

static void Main(string[] args)

Seu colega está propondo tornar esse o único ponto de acesso no seu código? Todos os outros códigos devem estar inacessíveis por chamadores externos?

Dificilmente. Então, o que torna aceitável tornar alguns métodos públicos? Não é, no final, uma decisão subjetiva de design?

Não é bem assim. O que tende a orientar os programadores, mesmo em um nível inconsciente, é, novamente, o conceito de encapsulamento. Você se sente seguro em expor um método público quando ele protege adequadamente seus invariantes .

Eu não gostaria de expor um método particular que não protege seus invariantes, mas muitas vezes você pode modificá-lo para que ele não proteger seus invariantes, e , em seguida, expô-la ao público (claro, com TDD, você fazê-lo da ao contrário).

Abrir uma API para testabilidade é uma coisa boa , porque o que você realmente está fazendo é aplicar o Princípio Aberto / Fechado .

Se você tem apenas um chamador da sua API, não sabe o quão flexível ela é. As chances são de que seja bastante inflexível. Os testes agem como um segundo cliente, fornecendo um feedback valioso sobre a flexibilidade da sua API .

Portanto, se os testes sugerem que você deve abrir sua API, faça-o; mas mantenha o encapsulamento, não ocultando a complexidade, mas expondo a complexidade de maneira segura.

Mark Seemann
fonte
3
+1 Você se sente seguro em expor um método público quando ele protege adequadamente seus invariantes.
Shambulator
1
Eu amo sua resposta! :) Quantas vezes ouço: "Encapsulamento é o processo de tornar privados alguns métodos e propriedades". É como dizer que quando você programa em orientação a objetos, isso ocorre porque você programa com objetos. :( Não estou surpreso que uma resposta tão clara venha do mestre da injeção de dependência. Certamente vou ler algumas vezes sua resposta para me fazer sorrir sobre o meu pobre e antigo código legado.
Samuel
Eu adicionei alguns ajustes na sua resposta. Como um aparte, li sua postagem sobre Encapsulation of Properties , e a única razão convincente que ouvi sobre o uso de Propriedades Automáticas é que alterar uma variável pública para uma propriedade pública quebra a compatibilidade binária; expô-lo como uma propriedade automática desde o início permite que a validação ou outra funcionalidade interna seja adicionada posteriormente, sem quebrar o código do cliente.
Robert Harvey
21

Parece que você está falando sobre injeção de dependência . É realmente comum e IMO, bastante necessário para a testabilidade.

Para abordar a questão mais ampla de saber se é uma boa idéia modificar o código apenas para torná-lo testável, pense da seguinte maneira: o código tem várias responsabilidades, incluindo a) a ser executado, b) lido por humanos ec) para ser testado. Todos os três são importantes e, se o seu código não cumprir todas as três responsabilidades, eu diria que não é um código muito bom. Então modifique-o!

Jason Swett
fonte
DI não é a questão principal (eu estava apenas tentando dar um exemplo); o ponto era se seria bom fornecer outro método que originalmente não foi criado para ser criado, apenas para fins de teste.
liortal 22/05
4
Eu sei. Pensei ter abordado seu ponto principal no meu segundo parágrafo, com um "sim".
21713 Jason Swett
13

É um problema de galinha e ovo.

Uma das maiores razões pelas quais é bom ter uma boa cobertura de teste do seu código é que ele permite refatorar sem medo. Mas você está em uma situação em que precisa refatorar o código para obter uma boa cobertura de teste! E seu colega está com medo.

Eu vejo o ponto de vista do seu colega. Você tem um código que (presumivelmente) funciona, e se você o refatorar - por qualquer motivo - há um risco de que você o violará.

Mas se esse é um código que deve ter manutenção e modificação contínuas, você estará correndo esse risco toda vez que fizer algum trabalho nele. E refatorar agora e obter cobertura de teste agora permitirá que você corra esse risco, sob condições controladas, e coloque o código em melhor forma para futuras modificações.

Então, eu diria que, a menos que essa base de código específica seja razoavelmente estática e não se espere que haja um trabalho significativo no futuro, o que você deseja fazer é uma boa prática técnica.

É claro que, se é uma boa prática comercial , é uma lata de minhocas completa.

Carson63000
fonte
Uma preocupação importante. Normalmente, faço o IDE suportado refatorações livremente, pois a chance de quebrar qualquer coisa é muito baixa. Se a refatoração estiver mais envolvida, eu o faria apenas quando precisar alterar o código de qualquer maneira. Para mudar as coisas, você precisa de testes para reduzir riscos, para obter valor comercial.
Hans-Peter Störr
O ponto é: queremos manter o código antigo porque queremos dar a responsabilidade ao objeto de consultar o assembly atual ou porque não queremos adicionar algumas propriedades públicas ou alterar a assinatura do método? Eu acho que o código viola o SRP e, por esse motivo, deve ser refatorado, independentemente do medo dos colegas. Certamente, se essa é uma API pública usada por muitos usuários, você terá que pensar em uma estratégia como implementar uma fachada ou qualquer coisa que ajude a conceder ao código antigo uma interface, evitando muitas alterações.
Samuel
7

Isso pode ser apenas uma diferença na ênfase das outras respostas, mas eu diria que o código não deve ser refatorado estritamente para melhorar a testabilidade. A testabilidade é altamente importante para a manutenção, mas a testabilidade não é um fim em si mesma. Como tal, eu adiaria qualquer refatoração desse tipo até que você possa prever que esse código precisará de manutenção para promover alguma finalidade comercial.

No ponto em que você determinar que este código vai exigir alguma manutenção, que seria um bom momento para refatorar para testabilidade. Dependendo do seu caso de negócios, pode ser uma suposição válida de que todo o código exigirá alguma manutenção, nesse caso a distinção que eu faço com as outras respostas aqui ( por exemplo, a resposta de Jason Swett ) desaparece.

Resumindo: a testabilidade por si só não é (IMO) uma razão suficiente para refatorar uma base de código. A testabilidade tem um papel valioso ao permitir a manutenção em uma base de código, mas é um requisito comercial modificar a função do seu código que deve impulsionar sua refatoração. Se não houver esse requisito comercial, provavelmente seria melhor trabalhar em algo que seus clientes se importam.

(É claro que o novo código está sendo mantido ativamente, portanto deve ser escrito para ser testado.)

Aidan Cully
fonte
2

Eu acho que seu colega está errado.

Outros já mencionaram as razões pelas quais isso já é uma coisa boa, mas, desde que você tenha permissão para fazer isso, deve ficar bem.

A razão para esta ressalva é que fazer qualquer alteração no código tem que custar um novo teste. Dependendo do que você faz, esse trabalho de teste pode, na verdade, ser um grande esforço por si só.

Não é necessariamente o seu lugar tomar a decisão de refatorar em vez de trabalhar em novos recursos que beneficiarão sua empresa / cliente.

ozz
fonte
2

Eu usei ferramentas de cobertura de código como parte do teste de unidade para verificar se todos os caminhos através do código são exercidos. Como um bom codificador / testador por conta própria, eu costumo cobrir 80-90% dos caminhos de código.

Quando estudo os caminhos descobertos e faço um esforço para alguns deles, é quando descubro bugs, como casos de erro que "nunca acontecerão". Então, sim, modificar o código e verificar a cobertura do teste cria um código melhor.

Sam Gamoran
fonte
2

Seu problema aqui é que suas ferramentas de teste são uma porcaria. Você deve conseguir zombar desse objeto e chamar seu método de teste sem alterá-lo - porque, embora este exemplo simples seja realmente simples e fácil de modificar, o que acontece quando você tem algo muito mais complicado.

Muitas pessoas modificaram seu código para introduzir classes de IoC, DI e interface, simplesmente para permitir o teste de unidade usando as ferramentas de simulação e teste de unidade que exigem essas alterações de código. Eu não acho que eles são uma coisa saudável, não quando você vê um código bastante direto e simples se transformar em um pesadelo de interações complexas impulsionadas inteiramente pela necessidade de tornar cada método de classe totalmente dissociado de todo o resto . E, para acrescentar insulto à lesão, temos muitos argumentos sobre se métodos privados devem ser testados em unidade ou não! (é claro que deveriam, o que '

O problema, é claro, é da natureza das ferramentas de teste.

Agora existem ferramentas melhores disponíveis que podem colocar essas alterações de design para sempre. A Microsoft possui Fakes (nee Moles) que permitem stub objetos concretos, incluindo estáticos, para que você não precise mais alterar seu código para ajustar-se à ferramenta. No seu caso, se você usasse o Fakes, substituiria a chamada GetTypes pela sua, que retornou dados de teste válidos e inválidos - o que é bastante importante, sua alteração sugerida não fornece isso.

Para responder: seu colega está certo, mas possivelmente pelos motivos errados. Não altere o código para testar, altere sua ferramenta de teste (ou toda a sua estratégia de teste para ter mais testes de unidade no estilo de integração, em vez de testes detalhados).

Martin Fowler tem uma discussão sobre essa área em seu artigo Mocks are not Stubs

gbjbaanb
fonte
1

Uma prática boa e comum é usar logs de teste e depuração de unidade . Os testes de unidade garantem que, se você fizer mais alterações no programa, sua funcionalidade antiga não será interrompida. Os logs de depuração podem ajudar a rastrear o programa em tempo de execução.
Às vezes acontece que, além disso, precisamos ter algo apenas para fins de teste. Não é incomum alterar o código para isso. Mas deve-se tomar cuidado para que o código de produção não seja afetado por causa disso. Em C ++ e C, isso é alcançado usando o MACRO , que é uma entidade de tempo de compilação. Em seguida, o código de teste não entra em cena no ambiente de produção. Não sei se essa disposição existe em C #.
Além disso, quando você adiciona código de teste ao seu programa, ele deve estar claramente visívelque esta parte do código é adicionada para algum propósito de teste. Ou então, o desenvolvedor que está tentando entender o código simplesmente suará essa parte do código.

Manoj R
fonte
1

Existem algumas diferenças sérias entre seus exemplos. No caso de DoSomethingOnAllTypes(), há uma implicação do somethingaplicável aos tipos na montagem atual. Mas DoSomething(Assembly asm)indica explicitamente que você pode passar qualquer montagem para ele.

A razão pela qual indico isso é que muitas etapas de injeção de dependência apenas para teste estão fora dos limites do objeto original. Eu sei que você disse " não 'tornando tudo público' ", mas esse é um dos maiores erros desse modelo, seguido de perto por este: abrindo os métodos do objeto até usos para os quais eles não se destinam.

Ross Patterson
fonte
0

Sua pergunta não deu muito contexto em que seu colega argumentou para que haja espaço para especulações

"má prática" ou não depende de como e quando as alterações são feitas.

Na minha opção, seu exemplo para extrair um método DoSomething(types)está ok.

Mas eu vi o código que não está bem assim:

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

Essas alterações tornaram o código mais difícil de entender porque você aumentou o número de possíveis caminhos de código.

O que quero dizer com como e quando :

se você tem uma implementação em funcionamento e, para "implementar recursos de teste", fez as alterações, você deve testar novamente seu aplicativo porque pode ter quebrado seu DoSomething()método.

A if (!testmode)é mais mais difficulilt de compreender e de teste do que o método extraída.

k3b
fonte