Skip to content

Conversation

@rven
Copy link

@rven rven commented Oct 7, 2025

No description provided.

@OCA-git-bot
Copy link
Contributor

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

@rven rven changed the base branch from 18.0 to 19.0 October 7, 2025 08:49
@rven rven mentioned this pull request Oct 7, 2025
7 tasks
@rven
Copy link
Author

rven commented Oct 7, 2025

@lmignon @StefanRijnhart @pedrobaeza @sbidoul
I notice that the CI pipelines are failing but this is related to the change in v19 of Odoo.
Odoo doesn't install demo data by default anymore when running Odoo.

+ unbuffer coverage run --include './*' --branch /opt/odoo-venv/bin/odoo -d odoo -i fastapi --test-enable --http-interface=127.0.0.1 --stop-after-init

You now have to specify the --with-demo argument yourself when demo data needs to be installed.

@lmignon
Copy link
Contributor

lmignon commented Oct 7, 2025

@rven IMO we should no more rely on demo data for tests. All tests have been adapted in Odoo, and we should do the same when migrating OCA modules.

@rven
Copy link
Author

rven commented Oct 7, 2025

@lmignon The power of the demo data is that a preset of data is loaded to do the validation when testing the functionality.
Like this module a lot of demo endpoints are created, which also can be tested in the UI.
If you change this in every module, it will be harder to test the functionality with already a preset of configured data.

@pedrobaeza
Copy link
Member

Yes, I was defending since a lot even before Odoo did the move that data used in tests should be populated in the test itself instead of using demo data. Now with this Odoo move, it makes it more mandatory, and IMO we shouldn't force to load demo data. The goal of the demo data is different than the tests, but to showcase the features of the module, or to provide basics for testing manually in runboat/runbot. One possible drawback is if runboat also don't load demo data, so if it's the case, then we should rethink it.

@lmignon
Copy link
Contributor

lmignon commented Oct 7, 2025

@rven see DynAppsNV#1 😏

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you for the migration.
Code Review + Functional tests

@abdullahoday710
Copy link

When is this expected to be merged ?

@lmignon
Copy link
Contributor

lmignon commented Oct 21, 2025

When is this expected to be merged ?

@abdullahoday710 This PR depends on OCA/web-api#117. It cannot be merged until the other one is merged. A review of these pending PRs will allow them to be merged.

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.

5 participants