-
-
Notifications
You must be signed in to change notification settings - Fork 308
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] [FIX] Fix problematic tests #3909
Conversation
fb1483a
to
4ad568d
Compare
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.
Grazie della PR!
@@ -145,26 +145,3 @@ def _get_report(self, report_date, report_type): | |||
report_ids = report_result['context']['active_ids'] | |||
report = self.env[report_model].browse(report_ids) | |||
return report | |||
|
|||
def test_journal_prev_year(self): |
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.
Puoi chiarire quale sarebbe il problema con questo test?
Normalmente se un test fallisce vuol dire che c'è un problema ed eliminare il test non risolve il problema.
Per aggiungere un po' di contesto, il test fu aggiunto per #3010.
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.
Puoi chiarire quale sarebbe il problema con questo test? Normalmente se un test fallisce vuol dire che c'è un problema ed eliminare il test non risolve il problema.
Sono perfettamente d'accordo.
In questo specifico caso avevo dato priorità al fatto che fosse un problema bloccante per tutte le PR attualmente aperte. Quindi l'intenzione era di procedere al merge di questa come hotfix e poi risolvere con calma il problema del test riaggiungendolo con una successiva PR.
Per aggiungere un po' di contesto, il test fu aggiunto per #3010.
Ah ecco, grazie mille mi era sfuggita!
Avevo dato un'occhiata (troppo) veloce alla PR di migrazione della 14.0 e avevo visto che il test non era presente.
Adesso mi è tutto più chiaro e dovrei aver capito qual è l'inghippo.
Aggiorno la PR.
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.
Aggiorno la PR.
Fatto.
4a422c3
to
42b9c41
Compare
42b9c41
to
d3e8ae1
Compare
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.
Molto meglio grazie!
@@ -62,7 +62,7 @@ | |||
<TipoDocumento>TD01</TipoDocumento> | |||
<Divisa>EUR</Divisa> | |||
<Data>2023-01-01</Data> | |||
<Numero>INV/2023/0001</Numero> | |||
<Numero>INV/2023/0005</Numero> |
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.
Probabilmente dovremo fare una correzione analoga l'anno prossimo, ma non vedo problemi a modificare temporaneamente così
/ocabot merge patch |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 5513b40. Thanks a lot for contributing to OCA. ❤️ |
Attualmente tutte le PR aperte che riguardano la versione 12.0 non superano
test_with_OCB
.Questo a causa di due errori sui test dovuti al passaggio all'anno nuovo.