Adding tests for validation and login #15
Conversation
| t.Parallel() | ||
|
|
||
| t.Run("Successful body read", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
C entendeu direitinho pra que o t.Parallel serve e como ele funciona?
|
|
||
| t.Run("Handle error reading request body", func(t *testing.T) { | ||
| t.Parallel() | ||
| req, err := http.NewRequest(http.MethodGet, "/", &ErrorReader{}) |
There was a problem hiding this comment.
Como ErrorReader é usado só dentro deste mesmo pacote, não precisa exportar ele. :)
| func testLoginErrors(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| methodsToTest := []string{http.MethodDelete, http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodOptions} |
There was a problem hiding this comment.
Acho que assim ficaria mais fácil de entender. Pense no código como uma leitura, quanto mais claros os nomes das variáveis forem, menos contexto precisa ser carregado na cabeça pra entender o código.
| methodsToTest := []string{http.MethodDelete, http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodOptions} | |
| forbiddenMethods := []string{http.MethodDelete, http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodOptions} |
| }) | ||
| } | ||
|
|
||
| func assertResponseStatus(t *testing.T, req *http.Request, status int) string { |
There was a problem hiding this comment.
Quando vc define alguma coisa em um arquivo de teste, essa parada fica disponível pra todos os testes que usem o mesmo pacote. Como essa função faz assertion do status HTTP da resposta de LoginUserHandler especificamente, vale um ou outro dos abaixo:
Um: Deixar claro no nome que a assertion é para LoginUserHandler.
Outro: Aceitar como parâmetro a função. Ficaria algo assim:
func assertResponseStatus(t *testing.T, fn http.HandleFunc, req *http.Request, status int) string {
t.Helper()
rec := httptest.NewRecorder()
fn(rec, req)
require.Equal(t, status, rec.Code)
return rec.Body.String()
}
// Usado assim, por exemplo:
assertResponseStatus(t, LoginUserHandler, req, http.StatusOK)Se você optar por outro, vale criar um arquivo shared_test.go pra centralizar esses helpers de teste.
Outra dica legal é usar t.Helper() em helper functions de teste. Isso faz com que, caso o teste falhe aqui (no require.Equal, por exemplo), o report de teste do Go vai mostrar que deu erro em quem chamou a função que usa t.Helper() ao invés de dentro da função que usa t.Helper().
|
|
||
| func CheckName(name string) error { | ||
| if name == "" { |
There was a problem hiding this comment.
Tende a ser uma boa ideia sanitizar inputs string por espaços antes e depois do texto. É relativamente fácil em interfaces esquecer um espaço antes ou depois do valor de um campo e não perceber. strings.TrimSpace() tende a ser suficiente.
| if name == "" { | |
| sanitizedName := strings.TrimSpace(name) | |
| if sanitizedName == "" { |
| false, | ||
| }, | ||
| User{"", "", "john@doe.com", "Password123!"}, | ||
| false, | ||
| false, | ||
| }, | ||
| } | ||
| for _, tt := range tests { |
There was a problem hiding this comment.
Normalmente variáveis com duas letras repetidas inferem coleções em Go. Como tá sendo usada pra inferir um único teste aqui pode gerar confusão.
Uma forma de resolver seria a abaixo, levando em consideração que em testes a variável t já é usada.
| for _, tt := range tests { | |
| for _, tc := range testCases { |
| false, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| if err := Validate(tt.args.user, tt.args.isSignUp); (err != nil) != tt.wantErr { | ||
| t.Errorf("Validate() error = %v, expectedError %v", err, tt.wantErr) | ||
| if err := Validate(tt.inputUser, tt.inputIsSignUp); (err != nil) != tt.outputErr { |
There was a problem hiding this comment.
- Se o valor do erro for significativo pra quem usa
Validate, vale a pena conferí-lo aqui além de checar se veio erro ou não. - Vale usar
require.NoError(se for manter como está), erequire.ErrorIsse o valor do erro for significativo pra quem usaValidate.
| executeTestCases(t, testCases) | ||
| for _, tt := range testCases { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| resp := tt.testFunc(tt.input) |
There was a problem hiding this comment.
Como testFunc não varia, dá pra chamar CheckEmail direto aqui e economizar umas linhas de código.
| resp := tt.testFunc(tt.input) | |
| resp := CheckEmail(tt.input) |
| input T | ||
| expectedError error | ||
| testFunc func(T) error | ||
| }) { | ||
| for _, tt := range testCases { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| resp := tt.testFunc(tt.input) |
There was a problem hiding this comment.
| resp := tt.testFunc(tt.input) | |
| resp := checkPassword(tt.input) |
brunomvsouza
requested review from
brunomvsouza
and removed request for
brunomvsouza
No description provided.