From 23f6f6d6b2bbd0b1a5e2bf5cf3d25ab388c8907a Mon Sep 17 00:00:00 2001 From: Julio Biason Date: Wed, 15 Jan 2020 12:21:52 -0300 Subject: [PATCH] Complete the translation --- content/thoughts/reveries-about-testing.md | 236 ++++++++++++++++++ content/thoughts/reveries-about-testing.pt.md | 28 +-- 2 files changed, 249 insertions(+), 15 deletions(-) diff --git a/content/thoughts/reveries-about-testing.md b/content/thoughts/reveries-about-testing.md index 47ca54e..e453b01 100644 --- a/content/thoughts/reveries-about-testing.md +++ b/content/thoughts/reveries-about-testing.md @@ -190,6 +190,242 @@ said tests in suites to be run that way). ## Coverage +Against what most people claim, I do believe that you can reach 100% coverage +pretty easily: You just need to delete code. + +The idea is quite simple, actually: If your tests check the system behavior, +and I'm proving that all tests pass, everything that doesn't have coverage +point to code that isn't necessary and, thus, can be deleted. + +It is not every code that can be removed. In the alarm manager example, even +when our "unit tests" cover all functionalities of each module, we got a block +in the "integration tests" that didn't have any coverage. This block was +responsible for checking the input of a module (for example, it won't allow +sending a SNMP message without a text). But, when we checked the code, we +realized that the base module (the one calling the others) was already doing +that validation and that this check was unnecessary. This lead into the +discussion of which test (and code block) should be removed. But we did have a +piece of "dead code" that was being marked as "alive" because we had unit +tests for both blocks. + +A more practical example. Imagine there is a class that keeps customer data in +a web shop[^3]: + +```python +class Client: + def __init__(self, name): + self.name = name +``` + +After awhile, comes a new requirement: A certain "Dewey" keeps creating +accounts non-stop, without doing any purchases, just to put trash in the +database; we need to block any new customers that make their name as just one +name. + +Then, thinking about SOLID[^4], the developer changes teh code to this: + +```python +def _multiple_names(name): + split_names = name.split(' ') + return len(split_names) > 1 + +def _validate_name(name): + if not _multiple_names(name): + raise Exception("Invalid name") + return name + +class Client: + def __init__(self, name): + self.name = _validate_name(name) +``` + +Now, when there is any service trying to create a new customer, the name is +validated against a certain rules and one of those is that the name must have +multiple names[^5]. + +New funcionality, new tests, right? + +```python +import pytest + +def test_single_name(): + """'Cher' não tem multiplos nomes.""" + assert not _multiple_names('Cher') + +def test_multiple_name(): + """'Julio Biason' tem múltiplos nomes.""" + assert _multiple_names('Julio Biason') + +def test_valid_name(): + """'Julio Biason' é um nome válido.""" + _validate_name('Julio Biason') + +def test_invalid_name(): + """'Cher' não é um nome válido e por isso levanta uma exceção.""" + with pytest.raises(Exception): + _validate_name('Cher') + +def test_client_error(): + """Se tentar criar uma conta com 'Cher', deve dar erro.""" + with pytest.raises(Exception): + Client(name='Cher') + +def test_client(): + """Uma conta com nome 'Julio Biason' deve funcionar.""" + Client(name='Julio Biason') +``` + +Running the tests: + +``` +$ pytest --cov=client client.py +==== test session starts ==== +plugins: cov-2.4.0 +collected 6 items + +client.py ...... + +---- coverage: platform linux, python 3.4.3-final-0 ---- +Name Stmts Miss Cover +------------------------------- +client.py 25 0 100% + +==== 6 passed in 0.11 seconds ==== +``` + +100% coverage and new functionality done! The developer give themselves some +pats in the back and go home. + +But, in the middle of the night, one of the managers who is also a friend of +Björk, gets a call from her telling that she tried to buy something and just +got an error message. The developer gets in the office next morning, sees the +manager email complaining about their famous friend being blocked and goes +into fixing the code: + +```python +class Client: + def __init__(self, name): + self.name = name +``` + +There, no more validation[^6] e now Björk can buy whatever she wants. But +running the tests: + +``` +==== FAILURES ==== +____ test_client_error ____ + + def test_client_error(): + with pytest.raises(Exception): +> Client(name='Cher') +E Failed: DID NOT RAISE + +client.py:37: Failed +==== 1 failed, 5 passed in 0.63 seconds ==== +``` + +Oh, sure! Cher is now a valid name and that behavior being tested is invalid. +We need to change the test to accept Cher: + +```python +def test_client_error(): + """Se tentar criar uma conta com 'Cher', deve funcionar.""" + Client(name='Cher') +``` + +And running the tests once again: + +``` +$ pytest --cov=client client.py +==== test session starts ==== +rootdir: /home/jbiason/unitt, inifile: +plugins: cov-2.4.0 +collected 6 items + +client.py ...... + +---- coverage: platform linux, python 3.4.3-final-0 ---- +Name Stmts Miss Cover +------------------------------- +client.py 24 0 100% + +==== 6 passed in 0.12 seconds ==== +``` + +Wonderful! Everything is working with the expected behaviors and we still have +100% coverage. + +But can you spot the problem in the code? + +The problem is now that `_multiple_names` is not being used anywhere, but it +is shown as "alive" 'cause there is a lost test that keeps saying that the +code is being used. If we had started with just the behavior tests -- +using just the system inputs and outputs -- right out of the bat we would see +that the function is not used anymore -- and if we need it in the future... +well, that's what version control systems are for. + +Although this looks like a silly example, there are some cases in which the +processing flow can be changed by the environment itself. For example, in +Django, you can add "middleware" classes, which are capable of intercepting +Requests or Responses and change their result. The most common example of +middleware is the Authentication, which adds the logged user information in +the Request; but those changes can be more deep, like changing some form +information. In those cases, the input (or the output, or both) is changed +and writing tests that ignore the middleware will not be a correct +representation of the input (or output, or both) of the system. And there we +can ask if the test is valid 'cause it is using a state that should not exist +in the normal use of the system. + +## Mocks + +Some time ago, I used to say that mocks should be used for things external to +the system. But I realized that definition is not quite correct -- there are +external things that shouldn't be mocked -- and that a better definition for +what should be mocked is "anything that you have no control". + +For example, if you're writing a system that uses IP geolocation using an +external service, you probably will mock the call for that service, as it is +out of your control. But a call to a database, when you're using a +system/framework that abstracts all the calls for the database (like Django +does), then the database, even being an external resource, is still under your +control and, thus, shouldn't be mocked -- but since the system/framework +offers a database abstraction, using any database shouldn't affect the +results. + +Another example are microservices. Even microservices inside the same company +or steam are external and out of control of the project and, thus, should be +mocked. "But they are from the same team!" Yes, but they are not part of the +same project and a) the idea behind microservices is to uncouple those +services and/or b) are in different directory trees. One of the advantages of +microservices from the same team is that the expected contract from one is +know by the team and that could be easily mocked (the team knows that, calling +a service with X inputs, it should receive an Y response -- or error). + +# Conclusion + +Again, the idea is not to rewrite every test that you have 'cause "the right +way is my way". On the other hand, I see a lot of tests being written in any +way, just using the context of "one test for each function/class" and, in some +cases, that doesn't make any sense and should get a little more thinking. By +exposing those "impure thoughts" about tests, I hope that would make people +rethink the way they are writing their tests + --- [^1]: Unless you want to use roman numerals, but anyway... + +[^2]: The reason for being changed to a string can be anything: due some + security plugin, 'cause we are using a database that doesn't work properly + with integers, 'cause we are plugging this system with a legacy one... + +[^3]: A class that keeps customer information should be way more complex that + this, but let's keep it simple just for this example. + +[^4]: No, that's not really SOLID, but that's keep it simple again for this + example. + +[^5]: Someone will send me the "Fallacies Developers Believe About User Names" + links for this, right? + +[^6]: Sure, I should change just `_validate_name`, but this way it makes it + even more clear what the problem is. diff --git a/content/thoughts/reveries-about-testing.pt.md b/content/thoughts/reveries-about-testing.pt.md index 6dbea7c..82c6caa 100644 --- a/content/thoughts/reveries-about-testing.pt.md +++ b/content/thoughts/reveries-about-testing.pt.md @@ -226,7 +226,7 @@ class Client: self.name = name ``` -Entretanto, depois de um tempo, surge um novo requisito: Um tal de "Zézinho" +Depois de um tempo, surge um novo requisito: Um tal de "Zézinho" está criando usuários sem parar, sem fazer compras, só pra sujar o banco; devemos bloquear todos os cadastros que tenham como nome do usuário apenas um nome. @@ -318,7 +318,6 @@ class Client: Pronto, não tem mais validação[^6] e agora a Xuxa pode comprar. Mas ao rodar os testes: - ``` ==== FAILURES ==== ____ test_client_error ____ @@ -343,7 +342,6 @@ def test_client_error(): E rodando os testes de novo: - ``` $ pytest --cov=client client.py ==== test session starts ==== @@ -379,28 +377,28 @@ exemplo, no Django, é possível ter classes "middleware", que são capazes de interceptar Requisições ou Respostas e alterar o resultado o mesmo. O exemplo mais comum é o middleware de Autenticação, que adiciona informações do usuário logado na Requisição; mas essas alterações podem ser mais profundas, como -alterar os dados do formulário, por exemplo. Nesses casos, a entrada (ou a -saída, ou ambos) é afetada e, portanto, qualquer coisa que ignore os -middlewares não vai representar a entrada (ou saída, ou ambos) do sistema -corretamente. E aí podemos perguntar se o teste é válido por gerar estados que -não devem existir naturalmente no sistema. +alterar os dados do formulário. Nesses casos, a entrada (ou a +saída, ou ambos) é afetada e, portanto, escrever testes que ignorem os +middlewares não vão representar a entrada (ou saída, ou ambos) do sistema +corretamente. E aí podemos perguntar se o teste é válido por usar estados que +não existem naturalmente no sistema. ## Mocks Há um tempo, eu indicava que "mocks" deveriam ser usados para coisas externas ao sistema. Entretanto, eu percebi que essa definição não é bem precisa -- -existem coisas externas que não devem ser mockadas -- mas que uma definição +existem coisas externas que não devem ser mockadas -- e que uma definição melhor para o que deve ser mockado é "qualquer coisa que esteja fora do seu controle". Por exemplo, se você está escrevendo um sistema que faz geolocalização de IPs através de um serviço externo, você provavelmente irá mockar a chamada para o serviço, já que ele está fora do seu controle. Mas uma chamada para o banco de -dados, quando você já utiliza um sistema de abstrai toda a parte de banco de -dados (por exemplo, Django), então o banco não é mais uma entidade externa, e -sim interna do sistema e que, portanto, não deveria ser mockada -- mas como o -sistema oferece uma abstração do banco, então usar qualquer banco não deve -afetar o resultado. +dados, quando você já utiliza um sistema/framework de abstrai toda a parte de +banco de dados (por exemplo, Django), então o banco, apesar de ser uma entidade +externa, ainda está sob seu controle e, portanto, não deveria ser mockada -- +mas como o sistema/framework oferece uma abstração do banco, então usar +qualquer banco não deve afetar o resultado. Outro exemplo são microserviços. Mesmo microserviços dentro da mesma empresa ou mesmo grupo são externos e fora do controle do projeto e, portanto, @@ -413,7 +411,7 @@ com dados X, haverá a resposta Y -- ou erro). # Conclusão -De novo, a ideia não é reescrever todos os casos de testes que você tem para +De novo, a ideia não é reescrever todos os testes que você tem porque "o jeito certo, que é o meu jeito". Entretanto, eu realmente vejo muitos testes sendo escritos "a revelia", considerando a simples métrica de "um teste por função/classe" e, em muitos casos, isso não faz sentido e que precisariam