Skip to content
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

Resolve #91 - PR de correção de Tests - assert_console #93

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

BrenoBaiardi
Copy link

Pull request inicial para validação,

Se estiver de acordo com o esperado, darei continuidade indo mais afundo na issue e tratando outros pontos levantados

@johnazedo johnazedo changed the title Primeiro Pull erquest de correção de Tests Primeiro Pull request de correção de Tests Oct 2, 2019
Copy link
Member

@itepifanio itepifanio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrenoBaiardi pelo que vi do seu código a modificação do assert_console() está correta. Para funcionar precisa agora modificar os testes que chamam essa função. Além disso, utilizamos um estilo de código chamado PEP8, seu código está falhando em atender o estilo agora, para verificar se está tudo ok basta rodar find . -name \*.py -exec pycodestyle --ignore=E402 {} + no terminal para verificar se o seu código está estilizado.

@BrenoBaiardi BrenoBaiardi changed the title Primeiro Pull request de correção de Tests Resolve #91 - PR de correção de Tests - assert_console Oct 4, 2019
@BrenoBaiardi
Copy link
Author

Continuo trabalhando para resolver os item mencionados...

Sobre o PEP8 e o comando mencionado para verificação, não consegui fazer ele ser executado aqui, mas pelo que vi o Travis faz essa verificação, correto?

@johnazedo
Copy link
Member

Pelo o que eu vi, as falhas na verificação do Travis são só uns detalhes na codificação, como linhas em branco e/ou espaços em branco.

Screen Shot 2019-10-04 at 13 36 01

@johnazedo
Copy link
Member

Para fazer a verificação no seu repositório local, basta instalar o pycodestyle, pip install pycodestyle, e executar o script a seguir:

$ find . -name \*.py -exec pycodestyle --ignore=E402 {} +

@BrenoBaiardi
Copy link
Author

O comando que vocês usam me retorna dizendo que não foi possível encontrar os arquivos com esta sintaxe $ find "*.py".
Não sei se é questão de sistema, estou utilizando Windows. Pesquisei como utilizar o módulo e fiz as verificações arquivo por arquivo. desculpem a demora, caso existam outros problemas além destes, podem informar.

@johnazedo
Copy link
Member

johnazedo commented Oct 7, 2019

Olá @BrenoBaiardi , acredito também que seja algo do windows. Vou procurar mais sobre como rodar os scripts nessa plataforma e caso eu ache, postarei aqui.

Caso você deseje algo de rápida solução ou uma solução temporária, deixo aqui links de como utilizar o bash do UNIX no prompt de comando.

@BrenoBaiardi
Copy link
Author

O travis agora está acusando erros no unittest, vou verificar, mas não acredito que sejam nos códigos que eu alterei.

tests/utils.py Outdated
Comment on lines 20 to 24
def assert_console(fun, message: str):
"""Recebe função que printa algo na tela e realiza assert
que verifica se foi printado."""
unit = unittest.TestCase()
return unit.assertTrue(len(input_value(fun)) > 0)
return unit.assertEqual(message, input_value(fun))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essa função é utilizada em inúmeros testes do projeto, no qual, alguns não apresentam o parâmetro message, por isso os erros estão sendo apresentados no unittest pelo travis.

Sugiro fazer alguma verificação para checar o valor da mensagem.
Exemplo:

if message:
    return unit.assertEqual(message, input_value(fun))
else:
    # Coloque algo aqui
    pass

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muito obrigado, vou verificar essa condição, não me atentei à esse caso,

Eu fiz um CTRL+F de "assert_console" e pensei que tinha encontrado todos os usos.
valeu

Copy link
Member

@johnazedo johnazedo Oct 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lembre de dar um valor padrão para o parâmetro, dessa forma não vai ser apontado erro caso estejam chamando o assert_console e não estiverem passando o valor do message.

Acredito que dessa forma seja mais fácil e mais correto do que a que eu apontei acima.

Copy link
Member

@itepifanio itepifanio Oct 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lembre de dar um valor padrão para o parâmetro, dessa forma não vai ser apontado erro caso estejam chamando o assert_console e não estiverem passando o valor do message.

Acredito que dessa forma seja mais fácil e mais correto do que a que eu apontei acima.

O ideal é que cada teste tenha sua mensagem verificada mesmo. Mas tem umas mensagens gigantescas, para verificar elas daria bastante trabalho, talvez até necessitasse criar arquivos de texto com as mensagens para comparar, algo a ser discutido, por enquanto deixar um valor default como mencionado por @johnazedo é o suficiente.

@BrenoBaiardi
Copy link
Author

desculpem à demora de retorno, estou trabalhando ainda nos códigos para entrega final
Obrigado pelos direcionamentos

@BrenoBaiardi
Copy link
Author

Boa noite Srs,
As Testes que encontrei e que achei válidos para alteração foram corrigidos e alterados para receber mensagens específicas.

Quando a complexidade das mensagens de comparação for maior, o método vai validar utilizando a lógica anterior, conforme o comentado por @johnazedo . Seria interessante desenvolver uma outra maneira e/ou padronizar todas as mensagens de "não sucesso" que possam ocorrer, dessa forma uma validação mais genérica poderia ser feita utilizando asserções com as strings recebidas, ao invés de utilizar a lógica de Len().

Dada a estrutura dos testes e dos erros que aparecem, uma vez que eles poderiam retornar Exceptions, não seria melhor criar uma ou mais classes herdando Exception que pudesse fazer a validação sem depender dos prints? {mesmo que somente para o caso de problemas mais complexos, e com mais possibilidades de outputs errados)

Agradeço a oportunidade de participar, poderiam validar o build que foi enviado e verificar se o merge é possível, ou se ainda é preciso mais algum progresso

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants