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

[17.0][MIG] module_auto_update #2763

Merged
merged 19 commits into from
May 3, 2024

Conversation

fkantelberg
Copy link
Member

I had to adjust some tests because:

  • Odoo core removed the methods _patch_method and _revert_method
  • There was no i18n/en.po in general

@pedrobaeza
Copy link
Member

/ocabot migration module_auto_update

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Nov 24, 2023
@thomaspaulb
Copy link
Contributor

@fkantelberg Could you retrigger the runboat with a rebase?

OCA-git-bot and others added 17 commits February 2, 2024 12:48
Currently translated at 100.0% (4 of 4 strings)

Translation: server-tools-15.0/server-tools-15.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-module_auto_update/es_AR/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-16.0/server-tools-16.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-module_auto_update/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-16.0/server-tools-16.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-module_auto_update/
Currently translated at 100.0% (5 of 5 strings)

Translation: server-tools-16.0/server-tools-16.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-module_auto_update/es_AR/
Currently translated at 100.0% (5 of 5 strings)

Translation: server-tools-16.0/server-tools-16.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-module_auto_update/es/
Currently translated at 60.0% (3 of 5 strings)

Translation: server-tools-16.0/server-tools-16.0-module_auto_update
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-module_auto_update/it/
@thomaspaulb
Copy link
Contributor

@fkantelberg Looks like tests are failing in conjunction with database_cleanup_test

@fkantelberg
Copy link
Member Author

@thomaspaulb True and I wonder why. I suspect that the tests of database_cleanup aren't cleaning up the database afterwards. I do believe that the module_auto_update works just fine. Maybe we have to think about a tearDown function in database_cleanup to remove the test data. Why do you think?

@thomaspaulb
Copy link
Contributor

@fkantelberg After some investigation, found that database_cleanup tests are doing a commit

And I think in previous versions this was noted also and solved by running tests in isolation or setting it as a rebel module. Maybe we can try the same here on 17.0?

@fkantelberg
Copy link
Member Author

Sure lets try the rebel one :)

@thomaspaulb
Copy link
Contributor

Cool, will you cherry pick it as part of this PR?

@fkantelberg
Copy link
Member Author

Seems to work. Not sure if I would rename the tests now to make it more descriptive.

@thomaspaulb
Copy link
Contributor

Renaming which tests in which way?

@fkantelberg
Copy link
Member Author

The tests are currently named:

  • tests / test with Odoo
  • tests /test with OCB
  • tests / test with Odoo
  • tests /test with OCB

The cherry pick duplicated the existing tests BUT the first to only test database_cleanup and the other 2 all other non-rebel modules. Not really descriptive but I could live with it.

@thomaspaulb
Copy link
Contributor

Ahhhh yes. Well if they can be renamed, I would name the second one "Rebel tests with Odoo", it even sounds cool. But I wouldn't really know where to put it. If you do, and it's easy, then why not.

@fkantelberg
Copy link
Member Author

It's defined in .github/worksflows/test.yml. I renamed the one which is only testing database_cleanup Rebel test ... because the cherry picked defined that modul as rebel :)

Thanks for the support.

@thomaspaulb
Copy link
Contributor

@fkantelberg I don't see the addon hash tests running here in 17.0 either, so maybe this needs a cherry pick of #2847

@fkantelberg
Copy link
Member Author

I cherry picked the interesting parts of it and I see the tests now in the logs. Seems like Odoo runs tests differently than I do locally.

@thomaspaulb
Copy link
Contributor

What exactly did you cherry-pick though? I see the changes in the test file, but also changes in Contributors and other readme files

@fkantelberg
Copy link
Member Author

cherry picked your linked commit. pre-commit did the rest but I squashed the commits.

@dreispt
Copy link
Sponsor Member

dreispt commented May 3, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-2763-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a775396 into OCA:17.0 May 3, 2024
8 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

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