-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[14.0][FIX] sale_force_invoiced: Proper tagging tests with post_install #2774
[14.0][FIX] sale_force_invoiced: Proper tagging tests with post_install #2774
Conversation
@simahawk, @rousseldenis could please check this one. |
What is the reason for this tagging? I don't think it's needed. You should use the same flow as Odoo and OCA does: only tests on installing, not later once installed. |
That is why i am adding it. It must be tagged as post_install because it needs a proper account set up. When the account module is installed, the journal entries are created the post_init_hook https://github.com/odoo/odoo/blob/ad5a7263461e734e294685fb1406bec97311c4d6/addons/account/__manifest__.py#L95 It is working for the OCA test flow, because at first some base modules are installed like account, after that a second install is running for the modules of the repo. |
OK about the tags. If invoices are involved, we need to do it post-install for making sure a CoA is present. Please check CI though. |
Good. But do you know why the test currently failing? Pre-commit is clear but the test with Odoo/OCB are failing. With the last test commit, which doesn't make any sense to me |
e392b6d
to
be7f609
Compare
@pedrobaeza as you can see here https://github.com/OCA/sale-workflow/actions/runs/6946367207/job/18897683508?pr=2795 - #2795 |
Someone should fix that test, that can be a change upstream that provokes the problem. Ping the maintainers of the module. |
Is it possible to merge this PR in the meantime? Or do we have to wait for the fix? |
We should wait for the good of the state. If not, the red state will expand... |
There is already a PR for fixing OCA/account-invoicing#1605. |
be7f609
to
7282779
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.
/ocabot merge patch
On my way to merge this fine PR! |
Congratulations, your PR was merged at e326874. Thanks a lot for contributing to OCA. ❤️ |
No description provided.