-
Notifications
You must be signed in to change notification settings - Fork 45
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
Correção de bug em calculo de fretes #184
base: master
Are you sure you want to change the base?
Conversation
O cálculo do frete (calculate_freights) estava enviando o type de pacote da classe e não o tipo de pacote de envio.
Olá, obrigado pela contribuição. Por favor, você pode acrescentar teste dessa modificação? em tempo, me aprece que com essa modificação alguns testes como esse Line 439 in 419e24b
|
Olá, realizei os testes, os únicos quebrados não tem relação alguma com a classe de testes client. Como poderia exportar os testes para vocês? É o meu primeiro pull request. |
Olá @nichmorgan, tudo bem? |
Corrijam-me se eu estiver errado, principalmente por ser novato em python, Mas essa PR passa em todos os testes já existentes (acabei de rodar), então ela deveria ser aprovada, nao? Além disso, a modificação sugeriada é para corrigir um possível bug, que inclusive está acontecendo comigo. @nichmorgan Achou outra saida para o erro que encontrou, ou apenas aplicou a modificação localmente? Ou ainda nem era erro, e achou o meio certo de utilizar? Abraços. |
Exatamente @parannoide, eu mesmo estou usando meu fork, pq não sei mais como aprovar aqui, talvez inexperiência minha. |
@nichmorgan e @parannoide gostaria de dar meus 2 cents, mesmo não sendo o mantenedor do repositório. Pelo que checkei na doc dos correios, na página 4/16 essa parte do código realmente está errada e faz sentido a alteração. Sobre os testes. Pelo que pude olhar aqui, encontrei alguns problemas:
A partir disso, acredito que seja interessante adicionar uma maior cobertura de testes para o primeiro ponto e atualizar o teste com vcr para que realize uma nova chamada aos correios. Como eu sei que essa parte de testes com Correios não é algo trivial e intuitivo eu tomei a liberdade de subir um diff da atualização que fiz caso queira seguir como exemplo. Acabei colocando usuário e senha do ambiente de teste público dos Corrreios sem mascarar. Atualmente no projeto é mascarado com XXX ou outros número, eu não vejo problema nisso em deixar público e facilitar futuras alterações, porém, só os mantenedores poderão opiniar melhor. |
@nichmorgan De qualquer forma, sua sugestão era a correção que eu precisava no momento. Muito obrigado. @erikhenrique Obrigado pela constribuição, dependendo da minha disponibilidade nos próximos dias tentarei seguir seu exemplo. |
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.
Aguardando fix relacionados aos testes
O cálculo do frete (calculate_freights) estava enviando o type de pacote da classe e não o tipo de pacote de envio.