[IMP] web_widget_dropdown_dynamic: Remove QUnit import from tests - #3145
Conversation
03583c0 to
17cc86c
Compare
fb4588a to
6d444f5
Compare
| /* eslint-disable no-undef */ | ||
| import {editInput, getFixture} from "@web/../tests/helpers/utils"; | ||
| import {makeView, setupViewRegistries} from "@web/../tests/views/helpers"; | ||
| import {QUnit} from "@web/static/lib/qunit/qunit"; |
There was a problem hiding this comment.
Isn't it QUnit used a bit below? Could you explain the rationale behind the patch?
There was a problem hiding this comment.
It doesn't require import as it's js lib
and causing this issue in web_timeline tests
2025-04-01 07:14:55,423 270 ERROR odoo odoo.addons.web_timeline.tests.test_web_timeline.TestWebTimeline.test_timeline_arch.browser: The following modules are needed by other modules but have not been defined, they may not be present in the correct asset bundle: ['@web/static/lib/qunit/qunit']
|
This PR has the |
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
LGTM
cc @pedrobaeza
| @@ -1,8 +1,6 @@ | |||
| /** @odoo-module **/ | |||
|
|
|||
| /* eslint-disable no-undef */ | |||
There was a problem hiding this comment.
Because it's giving the below error
web/web_widget_dropdown_dynamic/static/tests/web_widget_dropdown_dynamic_tests.esm.js
7:1 error 'QUnit' is not defined no-undef
38:5 error 'QUnit' is not defined no-undef
91:5 error 'QUnit' is not defined no-undef
122:5 error 'QUnit' is not defined no-undef
149:5 error 'QUnit' is not defined no-undef
✖ 5 problems (5 errors, 0 warnings)
There was a problem hiding this comment.
Yeah, and that's because you are removing the import, so for me, you shouldn't do it. I don't see the relation of this with the error in #3136 @CarlosRoca13 can you maybe help here?
There was a problem hiding this comment.
In odoo 18.0, is not needed to import QUnit because it is declared as global https://github.com/odoo/odoo/blob/4b6770ec30bd00213674df2b075c77f33a0e65d9/addons/web/static/src/%40types/libs.d.ts#L5
So we should add here the key
Lines 10 to 21 in 514c425
There was a problem hiding this comment.
Shouldn't that be in the copier template?
There was a problem hiding this comment.
There was a problem hiding this comment.
Please rebase and remove the linter line 😄
There was a problem hiding this comment.
Done, thanks
6d444f5 to
de4c892
Compare
|
/ocabot merge patch |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at 55459fa. Thanks a lot for contributing to OCA. ❤️ |
Fix tests issue in #3136