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

[14.0][FIX] sale_force_invoiced: Proper tagging tests with post_install #2774

Conversation

mt-software-de
Copy link

No description provided.

@mt-software-de
Copy link
Author

@simahawk, @rousseldenis could please check this one.
I tried to tag the unittest properly, but the unittests for the first commit failed for the module sale_triple_discount.
Normally this shouldn't have affected the test.
I added the now a second commit where i comment out the tagged.
As you can see the tests are still failing.

@pedrobaeza
Copy link
Member

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.

@mt-software-de
Copy link
Author

mt-software-de commented Nov 15, 2023

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.
It is the same as here
OCA/brand#128

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
https://github.com/odoo/odoo/blob/ad5a7263461e734e294685fb1406bec97311c4d6/addons/account/__init__.py#L71
And thats the reason why the test must be tagged as post_install otherwise it will fail with odoo -i sale_force_invoice --test-enable

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.

@pedrobaeza pedrobaeza added this to the 14.0 milestone Nov 15, 2023
@pedrobaeza
Copy link
Member

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.

@mt-software-de
Copy link
Author

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

@mt-software-de
Copy link
Author

@pedrobaeza as you can see here https://github.com/OCA/sale-workflow/actions/runs/6946367207/job/18897683508?pr=2795 - #2795
The test of sale_triple_discount failing also without my changes from this PR.
How we deal with that?

@pedrobaeza
Copy link
Member

Someone should fix that test, that can be a change upstream that provokes the problem. Ping the maintainers of the module.

@mt-software-de
Copy link
Author

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?

@pedrobaeza
Copy link
Member

We should wait for the good of the state. If not, the red state will expand...

@mt-software-de
Copy link
Author

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.

@mt-software-de mt-software-de force-pushed the 14-fix-proper-unittest-tags-sale_force_invoiced branch from be7f609 to 7282779 Compare November 28, 2023 14:50
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-2774-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4a635f3 into OCA:14.0 Feb 22, 2024
9 of 11 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

4 participants