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

[IMP] tests: only persistent table must have a primary key #159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pirols
Copy link
Contributor

@Pirols Pirols commented Oct 29, 2024

In some cases, it can be handy to define an unlogged/temporary table to memontarily store information from before the upgrade. We don't care for such non persistent tables to have a primary key.

Failure example: https://upgradeci.odoo.com/upgradeci/run/170069

In some cases, it can be handy to define an unlogged/temporary table to
memontarily store information from before the upgrade.
We don't care for such non persistent tables to have a primary key.
@Pirols Pirols requested review from a team and diagnoza October 29, 2024 14:24
@robodoo
Copy link
Contributor

robodoo commented Oct 29, 2024

Pull request status dashboard

Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

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

LGTM

@KangOl?

@KangOl
Copy link
Contributor

KangOl commented Oct 29, 2024

Actually, there is a real bug in the odoo/upgrade#6690 PR: the temporary table hasn't been cleaned up by the upgrade script (due to the early return check).

@Pirols
Copy link
Contributor Author

Pirols commented Oct 29, 2024

Actually, there is a real bug in the odoo/upgrade#6690 PR: the temporary table hasn't been cleaned up by the upgrade script (due to the early return check).

Indeed, the CI only raised due to that mistake. On the other hand, the logs still display that error:

[...]
2024-10-29 15:24:42,513 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.base.tests.test_module_state: Starting TestModuleState.test_prepare ...
2024-10-29 15:24:42,516 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.testing: Calling TestModuleState.prepare
2024-10-29 15:24:42,517 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.base.tests.test_ensure_has_pk: Starting TestTablesHavePK.test_prepare ...
2024-10-29 15:24:42,518 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.testing: Calling TestTablesHavePK.prepare
2024-10-29 15:24:42,522 23 CRITICAL pied_2050437_cp_saas~17.4 odoo.upgrade.base.tests.test_ensure_has_pk: Some tables doesn't have any primary key:
 - ___upgrade_arl_orig_sequencing
2024-10-29 15:24:42,523 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.base.tests.test_moved0: Starting TestMoved0.test_prepare ...
2024-10-29 15:24:42,524 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.testing: Calling TestMoved0.prepare
2024-10-29 15:24:42,524 23 INFO pied_2050437_cp_saas~17.4 odoo.upgrade.base.tests.test_moved0: skipped TestMoved0.test_prepare : Starting Odoo 16, no more `moved0` columns are created
[...]

Anyways, it's not blocking and I wouldn't have opened this PR for that alone, feel free to close or merge it, as you see fit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants