Skip to content

Conversation

@Dj0ulo
Copy link

@Dj0ulo Dj0ulo commented Dec 17, 2025

Behavior before commit:
When mountWithCleanup is called, the config to create a new instance
of App contains dev: false and test: true.
In the constructor of TemplateSet, this.dev is set to config.dev
but it's then it's overwritten in the constructor of App if
config.test is set.

This has the annoying side-effect of calling wrapError during Hoot
tests, which adds quite a big performance and memory overhead.

Fix:
Make sure that this.dev is set to config.dev even if config.test
is true.

task-5269391

__Behavior before commit:__
When [`mountWithCleanup`] is called, the config to create a new instance
of `App` contains `dev: false` and `test: true`.
In the constructor of [`TemplateSet`], `this.dev` is set to `config.dev`
but it's then it's overwritten in the constructor of `App` if
`config.test` is set.

This has the annoying side-effect of calling `wrapError` during Hoot
tests, which adds quite a big performance and memory overhead.

__Fix:__
Make sure that `this.dev` is set to `config.dev` even if `config.test`
is `true`.

[`mountWithCleanup`]: https://github.com/odoo/odoo/blob/7fd5f90fe8fce4de49ec10c683f92e7a3557981a/addons/web/static/tests/_framework/component_test_helpers.js#L138
[`TemplateSet`]: https://github.com/odoo/owl/blob/4e834369527f5d747de8f13236504dc20731c1c8/src/runtime/template_set.ts#L38

task-5269391
@Dj0ulo Dj0ulo requested a review from ged-odoo December 17, 2025 13:14
@Dj0ulo
Copy link
Author

Dj0ulo commented Jan 6, 2026

@ged-odoo can you check this PR when you have time 😉 ?

@kebeclibre
Copy link
Contributor

@Dj0ulo
mountWithCleanup in odoo's hoot tests works as expected: every test is supposed to run in dev mode (with the overhead you describe).
For the purpose of tests we want:

  • Check props mandatorily for every case
  • ensure that in a relatively easily written tests, loops are correctly written in the templates
  • .... more easily made, easily detected errors

For the purpose of test development we want:

  • Provide to the dev more verbose output to be able to fix without plunging into owl's sometimes obscure errors.
  • some more logging

In odoo's tests, disabling the dev mode mostly defeats the purpose of tests: ensure boot, API etc...
The performance overhead is significant, I guess our position on the subject is that that overhead is worth the benefit of detecting basic errors -- mind that if we did that in hoot it was because we fixed countless use cases where props were not given or respected etc....
Maybe owl should have have more than one level of debugging level though.....
For the future I know stuff are in the works to improve IDE integration, type checking etc..... to cover some of this stuff on the developer's side.

I do not close this as the issue is relevant and probably needs work, however I don't think the fix is right.

Note that if you are bothered in your tests by performance, you may test your components with the mountWithCleanUp ; then the business use cases with some other mounting function......

@Dj0ulo
Copy link
Author

Dj0ulo commented Jan 6, 2026

@kebeclibre Thanks for the detailed explanation! I'll let this PR as it is until a better solution is found then.

@Dj0ulo
Copy link
Author

Dj0ulo commented Jan 6, 2026

@kebeclibre
But if dev mode should be set in tests, I don't get why dev is set to false here though:

https://github.com/odoo/odoo/blob/7fd5f90fe8fce4de49ec10c683f92e7a3557981a/addons/web/static/tests/_framework/component_test_helpers.js#L137

Shouldn't this line be removed to be coherent?

Edit: my bad it's for the props validation part 🤔

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.

2 participants