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

[FIX] core: harden the legacy migrations package matching regex #136282

Closed

Conversation

KangOl
Copy link
Contributor

@KangOl KangOl commented Sep 22, 2023

Since #122569, we now try to import the migrations sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module base_maintenance, which generate a RecursionError.


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

Since odoo#122569, we now try to import the `migrations`
sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module `base_maintenance`,
which generate a RecursionError.
@KangOl
Copy link
Contributor Author

KangOl commented Sep 22, 2023

@pedrobaeza Can you test if this solves your problem?

@robodoo
Copy link
Contributor

robodoo commented Sep 22, 2023

@C3POdoo C3POdoo requested review from a team, rco-odoo and ryv-odoo and removed request for a team September 22, 2023 10:49
@C3POdoo C3POdoo added the RD research & development, internal work label Sep 22, 2023
Copy link
Collaborator

@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.

Thanks for the quick reaction, Christophe.

This patch avoids the recursion problem indeed, tested on my local. And code reviewing it, it's correct to escape the points, as we want the literal char, not a wildcard.

Copy link
Contributor

@etobella etobella left a comment

Choose a reason for hiding this comment

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

Thanks for the fast fix!

@KangOl
Copy link
Contributor Author

KangOl commented Sep 25, 2023

@robodoo r+

robodoo pushed a commit that referenced this pull request Sep 25, 2023
Since #122569, we now try to import the `migrations`
sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module `base_maintenance`,
which generate a RecursionError.

closes #136282

Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
@robodoo robodoo temporarily deployed to merge September 25, 2023 08:39 Inactive
@robodoo robodoo closed this Sep 25, 2023
erl-odoo referenced this pull request Sep 26, 2023
Upgrade (aka migration) scripts are a core part of Odoo, allowing
database manipulations for modules during version changes.

Any module, including custom ones can run upgrade scripts, even if the
`--upgrade-path` flag (and with it, the `odoo.upgrade` sub-module) is
not present. Currently only the "standard" modules benefit of easy
upgrade script testing. Any custom modules that want to run tests of
their upgrades have to import the tests in the usual `tests` folder,
which is not ideal.

Therefore, to allow TDD and programmatic testing of upgrade scripts in
custom modules, the test discovery is here modified to also parse the
module's `migrations` and `upgrades` sub-modules for tests.

closes #136130

X-original-commit: 483cc20
Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
Signed-off-by: Vranckx Florian (flvr) <flvr@odoo.com>
@fw-bot
Copy link
Contributor

fw-bot commented Sep 29, 2023

@KangOl this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot fw-bot deleted the 13.0-fix-upgradeimporthook-regex-chs branch October 9, 2023 08:46
etobella added a commit to etobella/odoo that referenced this pull request Dec 11, 2023
Since odoo#122569, we now try to import the migrations sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module base_maintenance, which generate a RecursionError.
It was partially fixed on odoo#136282 but some collateral damages where raised on tests
robodoo pushed a commit that referenced this pull request Dec 11, 2023
Since #122569, we now try to import the migrations sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module base_maintenance, which generate a RecursionError.
It was partially fixed on #136282 but some collateral damages where raised on tests

closes #145800

Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
luanjubica pushed a commit to luanjubica/odoo-code that referenced this pull request Feb 14, 2024
Since odoo#122569, we now try to import the migrations sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module base_maintenance, which generate a RecursionError.
It was partially fixed on odoo#136282 but some collateral damages where raised on tests

closes odoo#145800

Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
aaronselk pushed a commit to continuant/odoo that referenced this pull request Mar 28, 2024
Since odoo#122569, we now try to import the migrations sub-package of each module to find upgrade tests.
However, this badly written regex match the OCA module base_maintenance, which generate a RecursionError.
It was partially fixed on odoo#136282 but some collateral damages where raised on tests

closes odoo#145800

Signed-off-by: Christophe Simonis (chs) <chs@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants