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

[FIX] payment_cielo tests #1064

Merged
merged 1 commit into from
Jan 6, 2021
Merged

Conversation

DiegoParadeda
Copy link
Contributor

This PR fixes Travis test errors in module payment_cielo

@DiegoParadeda DiegoParadeda force-pushed the fix/payment_cielo_tests branch from a183ca0 to 31aa76a Compare January 6, 2021 13:26
@rvalyi
Copy link
Member

rvalyi commented Jan 6, 2021

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1064-by-rvalyi-bump-nobump, awaiting test results.

@rvalyi
Copy link
Member

rvalyi commented Jan 6, 2021

valeu @DiegoParadeda

pessoal, esse tipo de problema mostra bem as limitaçoes do VCRPy: testa de forma bem completa mas fica chato parta manter. Nisso minha opiniao é de reservar isso a modulos super comun que todos PSCs dominam no repo e pros outros modulos tentar pegar mais leve com a biblioteca mock.

Eu por examplo ja expressei minha opiniao sobre a questao de testar as NFSe com VCRPy, sendo que nem todo PSC vai ter chave para testar e atualizar os dados do VCRPy e eu imagino que esse problema com o payment_cielo é so um gostinho das dores de cabeça que teremos com os modulos de NFSe testados com VCRPy.
No caso dos testes da NFe, acho aceitavel na medida que qualquer PSC do projeto vai ter que dominar a transmissao da NFe de qualquer forma e vai poder ajudar a manter.

cc @mileo @renatonlima @marcelsavegnago

@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 0269dfc. Thanks a lot for contributing to OCA. ❤️

@OCA-git-bot OCA-git-bot merged commit 0269dfc into OCA:12.0 Jan 6, 2021
@rvalyi
Copy link
Member

rvalyi commented Jan 6, 2021

brincadeira né, a merda do PR #820 que a gente tinha tirado veio junto nesse PR...
Galera vamos ter que tirar de novo, vai dar rewrite, desculpa para quem der pull nessa hora. Vamos almoçarf e tiramos a lambança.

@rvalyi
Copy link
Member

rvalyi commented Jan 6, 2021

pessoal, esse PR so tinha um commit que parecia certo. Porem a coisa louca foi que o ocabot parece que jogou depois os commits do PR#820 que nao apromavos e tiramos por achar que nao tava no razoavel ainda.
Dai podemos tirar de novo, mas o que garante que os commits do #820 nao vao voltar na sequencia? Alguem entende o que aconteceu?

Screenshot from 2021-01-06 13-57-59

@marcelsavegnago
Copy link
Member

Dai podemos tirar de novo, mas o que garante que os commits do #820 nao vao voltar na sequencia? Alguem entende o que aconteceu?

Se puder tirar eu já faço o rebase aqui e vejo se os commits continuam nas PRs de ajuste de NFSe que tenho em aberto

@rvalyi
Copy link
Member

rvalyi commented Jan 6, 2021

sim @marcelsavegnago . Ficamos enrolado num go live com outra regressão que te pergunto quem que sera que fez. Mas vamos tirar de novo daqui pouco. Da ultima vez eh provavel que tiramos os commits da branch sem detonar os commits e que por algum motivo isso voltou, eh o que suspeitamos pelo menos.

@mileo mileo deleted the fix/payment_cielo_tests branch February 28, 2021 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants