search

Minhacps / VotaCampinas

Esse repositório contem o código do programa que auxilia os eleitores e eleitoras de Campinas a escolherem o seu candidato ou candidata a vereadora de Campinas para as eleiçoes de 2016
http://votacampinas.org.br/
11 stars 5 forks source link

Adicionar testes para Reset Password Controller #58

Closed rodolfoprr closed 8 years ago

rodolfoprr commented 8 years ago

Adicionei testes para o controller responsável pelo "reset password". Algumas dúvidas a respeito dos testes:

Uma coisa não relacionada diretamente a este PR, é versionarmos a pasta app/test/coverage/. Talvez podemos retirar essa pasta do repositório, pois ela atrapalha na revisão do PR quando houver novos testes e não sei o quão válido é mantermos essa pasta versionada. O que acham?

@gabriel-ribeiro-ir @victorperin

victorperin commented 8 years ago

\o/ Obrigado por pensar nos testes, a gente realmente ta precisando escrever no repositório. Quanto aos testes, por enquanto não temos padrão nenhum, mas seria legal termos algum, então acho que podemos manter em inglês esse padrão mesmo.

Com relação ao comando npm test podemos voltar sim. Vou trabalhar para adiciona-lo ao travis também, assim podemos rodar os testes em todos os pull requests.

Já a pasta coverage não deve ser versionada (ao meu ver), pois ela é gerada automaticamente e podemos gera-la de novo.

rodolfoprr commented 8 years ago

@victorperin a respeito da pasta app/test/coverage/, só vou esperar a opinião do @gabriel-ribeiro-ir , e caso a gente defina em removê-la, eu abro um novo PR para isso e depois faço o rebase nesse daqui.

victorperin commented 8 years ago

tranquilo, enquanto isso, vou preparando o travis pra fazer esses testes.

victormiguez commented 8 years ago

@victorperin o travis que está rodando hoje já está preparado pra isso, só precisaria colocar o comando de testes antes de fazer deploy.

notnotgabriel commented 8 years ago

@rodolfoprr a pasta app/test/coverage/ é onde serão gerados os relatórios de cobertura? Se for isto não vejo motivo pra versionar tbm.

Sobre o padrão de testes não temos, mas eu voto em pt-br por ser um app BR e quanto mais inclusivo a gente for melhor. Se uma pessoa de outra cidade quiser forkar o repo não terá esta "dificuldade" a mais para entender o projeto.

Sobre o npm test foi eu quem tirou #vergonha huahuahua estava quebrando um teste que a gente nem tinha implementado, gerado automaticamente pelo megaboilerplate. Ele será muito bem vindo de volta 👍

rodolfoprr commented 8 years ago

@victormiguez o Travis está rodando npm test?

Sobre a escrita dos testes e o lance de ser inclusivo, bom, eu até concordo, mas a app toda está escrita em inglês, então sei lá... huahauhua. Deixo pra vocês decidirem isso. @victormiguez você vai ter que desempatar...

notnotgabriel commented 8 years ago

@rodolfoprr concordo com vc kkkkk A verdade é que não seguimos nenhum padrão.

victormiguez commented 8 years ago

Ainda não está rodando, mas já coloco no travis.yml.

Caras, na minha opinião essa pasta não deve ser versionada. Caso seja de interesse da pessoa ver o relatório de testes, ela deve rodá-los pedindo um output. Podemos manter sem a pasta de relatórios?

notnotgabriel commented 8 years ago

Lança a pasta no .gitignore \o/

rodolfoprr commented 8 years ago

Pronto, fiz o rebase com o master, agora o diff tá bala! https://github.com/Minhacps/VotaCampinas/pull/58/commits/c3fd0e0c94546d2c917011881fcb12c6007ba573

E coloquei de volta o npm test https://github.com/Minhacps/VotaCampinas/pull/58/commits/9b8b7cb7233026b8aa28e88674a44e39b04af013

Por último, descrição dos testes em inglês ou português? @victormiguez

victormiguez commented 8 years ago

Boa mano! Valeu mesmo por puxar isso!

Eu prefiro os testes em português para sermos mais inclusivos. Tem bastante código em inglês no projeto por conta do Mega Boilerplate, talvez traduzir esse código todo seja um próximo passo, mas isso é outro assunto.

rodolfoprr commented 8 years ago

Beleza, feito! :+1:?