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

[12.0] pre-commit after pre-commit version upgrade #1950

Merged
merged 15 commits into from
Jun 2, 2022

Conversation

renatonlima
Copy link
Member

Depois da atualização do pre-commit passou a ter alguns erros no pre-commit eu fiz esse PR com commits por módulos.

@OCA-git-bot
Copy link
Contributor

Hi @gabrielcardoso21, @rvalyi, @marcelsavegnago, @mbcosta, @mileo, @lfdivino, @hendixcosta, @luismalta,
some modules you are maintaining are being modified, check this out!

Copy link
Contributor

@mbcosta mbcosta left a comment

Choose a reason for hiding this comment

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

O erro que aconteceu no Travis é referente a um teste do modulo payment_pagseguro sem ligação com esse PR, parece ser um problema semelhante ao que aconteceu em outros módulos que fazem o teste de interface/web_tour e pelo o que lembro foi feito algo para corrigir isso( não sei dizer especificamente o que ), talvez a solução seja a mesma para esse modulo payment_pagseguro.
image

@renatonlima
Copy link
Member Author

O erro que aconteceu no Travis é referente a um teste do modulo payment_pagseguro sem ligação com esse PR, parece ser um problema semelhante ao que aconteceu em outros módulos que fazem o teste de interface/web_tour e pelo o que lembro foi feito algo para corrigir isso( não sei dizer especificamente o que ), talvez a solução seja a mesma para esse modulo payment_pagseguro. image

@hirokibastos Você pode dá uma olhada nesse teste do módulo payment_pagseguro? Eu não sei se esta relacionado ao último merge #1916 me parece que é um erro intermitente, alguns builds esses testes passam e outros gera erro, seria importante encontrar alguma forma de corrigir, ou de não gerar um falso negativo.

@hirokibastos
Copy link
Contributor

O erro que aconteceu no Travis é referente a um teste do modulo payment_pagseguro sem ligação com esse PR, parece ser um problema semelhante ao que aconteceu em outros módulos que fazem o teste de interface/web_tour e pelo o que lembro foi feito algo para corrigir isso( não sei dizer especificamente o que ), talvez a solução seja a mesma para esse modulo payment_pagseguro. image

@hirokibastos Você pode dá uma olhada nesse teste do módulo payment_pagseguro? Eu não sei se esta relacionado ao último merge #1916 me parece que é um erro intermitente, alguns builds esses testes passam e outros gera erro, seria importante encontrar alguma forma de corrigir, ou de não gerar um falso negativo.

Não estou muito por dentro do funcionamento dos testes deste módulo. Porém esse erro já foi discutido em #1886. Acredito que o @kmmelcher possa dizer melhor o que acontece.

@rvalyi
Copy link
Member

rvalyi commented Jun 2, 2022

@hirokibastos @kmmelcher sei que não é a culpa de vcs, mas realmente so quem mantem o projeto tem noção de quanta dor de cabeça nos dam esses módulos web da KMEE na v12. Vcs chegaram a olhar, eu levantei uma possibilidade de melhoria aqui #1891 mas eu não faço ideia se serve mesmo. Na Akretion ate a v12 a gente considerou que essa parte web não era bem madura e a gente não chegou a criar muitos módulos OCA web. Eu espero que os testes não vão ficar tão instáveis na 14 pelo menos.

@kmmelcher
Copy link
Contributor

@hirokibastos @kmmelcher sei que não é a culpa de vcs, mas realmente so quem mantem o projeto tem noção de quanta dor de cabeça nos dam esses módulos web da KMEE na v12. Vcs chegaram a olhar, eu levantei uma possibilidade de melhoria aqui #1891 mas eu não faço ideia se serve mesmo. Na Akretion ate a v12 a gente considerou que essa parte web não era bem madura e a gente não chegou a criar muitos módulos OCA web. Eu espero que os testes não vão ficar tão instáveis na 14 pelo menos.

Estou com uma ideia de como deixar a tour da pagseguro mais simples, vou tentar implementar para diminuir os erros

@rvalyi
Copy link
Member

rvalyi commented Jun 2, 2022

@hirokibastos @kmmelcher
Tb, agora que os testes desses modulos web ja passaram, por mim rola de tb tirar eles a instalaçao no Travis na 12 (tem umas opçoes para isso). E ai quando tiver um PR a pessoa vai ter que checar na sua maquina própria que o teste passa. Provavelmente isso seria melhor do que atrasar a vida do projeto inteiro pour causa dos mesmos 2 ou 3 mesmos módulos que pouca gente usa ou altera.

@rvalyi
Copy link
Member

rvalyi commented Jun 2, 2022

ai erro de novo, no l10n_br_website_sale dessa vez...

@rvalyi
Copy link
Member

rvalyi commented Jun 2, 2022

ja que esse merge so tem mudanças triviais eu vou fazer o merge com o botão merge. Porem nos vamos continuar com os mesmas instabilidades com os testes dos modulos web da KMEE. Caso o problema seja muito frequente eu proponho de desabilitar os 2 ou 3 modulos problematicos dos testes Travis ate que uma solução melhor seja achada.

@kmmelcher
Copy link
Contributor

@hirokibastos @kmmelcher Tb, agora que os testes desses modulos web ja passaram, por mim rola de tb tirar eles a instalaçao no Travis na 12 (tem umas opçoes para isso). E ai quando tiver um PR a pessoa vai ter que checar na sua maquina própria que o teste passa. Provavelmente isso seria melhor do que atrasar a vida do projeto inteiro pour causa dos mesmos 2 ou 3 mesmos módulos que pouca gente usa ou altera.

Onde fica a opção de remover o pagseguro dos testes do travis? eu só achei como remover ele do pre-commit

@rvalyi rvalyi merged commit 5009aa7 into OCA:12.0 Jun 2, 2022
@rvalyi
Copy link
Member

rvalyi commented Jun 2, 2022

@hirokibastos @kmmelcher Tb, agora que os testes desses modulos web ja passaram, por mim rola de tb tirar eles a instalaçao no Travis na 12 (tem umas opçoes para isso). E ai quando tiver um PR a pessoa vai ter que checar na sua maquina própria que o teste passa. Provavelmente isso seria melhor do que atrasar a vida do projeto inteiro pour causa dos mesmos 2 ou 3 mesmos módulos que pouca gente usa ou altera.

Onde fica a opção de remover o pagseguro dos testes do travis? eu só achei como remover ele do pre-commit

@kmmelcher @hirokibastos Eu acho que vc pode botar uma variavel EXCLUDE como aqui https://github.com/OCA/sale-workflow/blob/12.0/.travis.yml#L28
da para vc fazer um PR testando isso? Nesse caso do sale-workflow é simplesmente que eles definem varias sessoes de teste com esse "ENV Matrix", cada uma testando sets de módulos diferentes, mas em nossos casos a idéia seria mesmo desabilitar os testes apenas.

Por mim muito melhor ter que testar esses módulos manualmente do que eles atrasando todo mundo.

Os modulos que mais dam problemas e que teria que tirar são esses 3:

  • payment_pagseguro
  • l10n_br_website_sale_delivery
  • l10n_br_website_sale

Se botar esses 3 para correr longe do Travis, a gente já volta ao Eden original eu acho.

cc @marcelsavegnago @renatonlima

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.

7 participants