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

[16.0][FIX] mis_builder: ensure ci #662

Merged

Conversation

chienandalu
Copy link
Member

@chienandalu chienandalu commented Dec 13, 2024

Some modules could insert data to the default company that could make the tests checks fail. With this patch we ensure, at least for this test cases to be performed with a brand new company.

An example of test failing on this (there were several on this test suite):

2024-12-12 14:02:43,190 46 ERROR prod odoo.addons.mis_builder.tests.test_data_sources: FAIL: TestMisReportInstanceDataSources.test_actuals
Traceback (most recent call last):
  File "/opt/odoo/auto/addons/mis_builder/tests/test_data_sources.py", line 204, in test_actuals
    assert_matrix(matrix, [[11, 13], [11, 30], [11, 13], [AccountingNone, 17]])
  File "/opt/odoo/auto/addons/mis_builder/tests/common.py", line 29, in assert_matrix
    assert (
AssertionError:  != 11 in row 2 col 0

There was intrusive data (I couldn't identify which module was adding it) that would appear in the report. In the debugging, I got these results. As it can be seen, there are three unexpected rows.

>>> kpi matrix
[[11.0, 13.0], [11.0, 30.0], [AccountingNone, AccountingNone], [AccountingNone, AccountingNone], [AccountingNone, AccountingNone], [11.0, 13.0], [AccountingNone, 17.0]]
>>> expected
[[11, 13], [11, 30], [11, 13], [AccountingNone, 17]]

Anyway, we should worry about searching for ghosts every time. Just testing in a new company does the job in a reliable way

  • Local CIs running ok 👍

cc @Tecnativa

please review @pedrobaeza @carlosdauden

fyi @CarlosRoca13

Some modules could insert data to the default company that could make
the tests checks fail. With this patch we ensure, at least for this test
cases to be performed with a brand new company.
@OCA-git-bot
Copy link
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@pedrobaeza
Copy link
Member

Putting a fragment of the errors got previously helps for understanding the integration issue.

@chienandalu
Copy link
Member Author

Done and checked in local CI

@sbidoul
Copy link
Member

sbidoul commented Dec 13, 2024

Looks good.

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-662-by-sbidoul-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a7e850d into OCA:16.0 Dec 13, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 7b35105. 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