-
Notifications
You must be signed in to change notification settings - Fork 294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: cover the whole session controler and service use cases #159
Conversation
BertBR
commented
May 21, 2024
- Coverage the whole session use case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@BertBR Poderia explicar a utilidade de testar esse código da controller e service dessa forma? Posso estar enganado, mas como não tem nenhuma regra de negócio sendo checada, não parece fazer sentido. Se simplesmente fizermos mais um include no prisma -> vamos ter que alterar o código de teste. Parece que esse teste ta checando se o código de hoje é exatamente igual ao código de hoje sem nenhuma liberdade pra nada. Acho que faz mais sentido fazer testes de integração pra isso ai, um teste que rode a api, insira dados no banco, chame as rotas e veja se o retorno ta correto. Se alterarmos de prisma pra sequelize, se alterarmos a ordem da invalidação das sessions, se fizermos qualquer coisa que faça o comportamento ser o mesmo e mude o código o teste ainda passa se tiver tudo certo. |
O objetivo é simples, meu caro: Garantir que o que está em produção, funcionando hoje e agregando valor continue funcionando. E sim, o objetivo de testes unitários é exatamente esse, se houver alteração, tem que mapear o comportamento novo em um teste, o que é facilmente feito em menos de 5 minutos por quem estiver implementando a nova feature, por exemplo. Em relação aos testes de integração, concordo que abrangem um escopo maior e devem sim ser priorizados em MVP ou inicio de projeto de qualquer dimensão. Esse será meu próximo passo para contribuições aqui, auto gerar testes de integração através de chamadas na api com keploy |
TLDR; Resumo: Acho que esses métodos praticamente sem regra de negócio deveriam ser testados aplicando um seed no db, chamando a api e verificando a resposta porque testar unitariamente esses métodos é só pra sobrecarregar o dev. Obs: Não estou falando que testes unitários são inúteis, mas acho que este é um uso ruim pra eles. Se tu fosse fazer um função de por exemplo: valida cpf, parse date, format date, sanitiza sei lá oq, convert sei lá oq pra sei lá oq 2... -> isso faz sentido, porque não tem essas dependências do prisma, de db, de coisas desse tipo que precisam de um mock sem nexo. |
Mano, se o teste está sensível a implementações então o problema não é o teste mas sim a arquitetura do projeto...Mas enfim, acho que essa discussão não vai levar a nenhum resultado. Seu ponto ja ficou claro pra mim, vou fechar a PR pra evitar mais ruídos |
@BertBR os testes trazem valor, sim, vou revisá-los. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Estava com logs aparecendo, subi uma alteração pra escondê-los. Bora integrar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Estava com logs aparecendo, subi uma alteração pra escondê-los. Bora integrar.