From af85bb5e13b9c97d022a721bd782a37d718e4496 Mon Sep 17 00:00:00 2001 From: Aaron Bohy <aab@odoo.com> Date: Mon, 8 Jun 2020 11:50:24 +0000 Subject: [PATCH] [IMP] web,*: do not deploy services in tests *mail,point_of_sale,web_editor,website In the tests, required services are deployed for each test independently. There is no need to have, in additon, all services deployed globally. Worse, it could conflict and lead to unexpected results. This commit ensures services are no longer deployed globally in tests. It turns the module 'web.env' into a declarative module with no side-effect, by moving the service deployment to main.js, which isn't added to the tests page. Task 2287397 closes odoo/odoo#53817 Signed-off-by: Lucas Perais (lpe) <lpe@odoo.com> --- .../static/src/{env/env.js => js/main.js} | 2 +- addons/mail/views/assets.xml | 7 ++- addons/point_of_sale/static/src/js/main.js | 2 + .../static/tests/unit/helpers/test_main.js | 17 +++++++ addons/web/static/src/js/common_env.js | 37 +-------------- .../static/src/js/core/abstract_service.js | 45 +++++++++++++++++++ addons/web/static/src/js/main.js | 11 ++--- .../src/js/public/public_root_instance.js | 6 +++ addons/web/static/tests/main_tests.js | 19 +++++--- addons/web/views/webclient_templates.xml | 7 ++- .../static/tests/field_html_tests.js | 8 ++++ .../src/js/content/website_root_instance.js | 13 ++++++ 12 files changed, 124 insertions(+), 50 deletions(-) rename addons/mail/static/src/{env/env.js => js/main.js} (97%) diff --git a/addons/mail/static/src/env/env.js b/addons/mail/static/src/js/main.js similarity index 97% rename from addons/mail/static/src/env/env.js rename to addons/mail/static/src/js/main.js index ea760ba6e118..95c8c4153b1a 100644 --- a/addons/mail/static/src/env/env.js +++ b/addons/mail/static/src/js/main.js @@ -1,4 +1,4 @@ -odoo.define('mail/static/src/env/env.js', function (require) { +odoo.define('mail/static/src/js/main.js', function (require) { 'use strict'; const ModelManager = require('mail/static/src/model/model_manager.js'); diff --git a/addons/mail/views/assets.xml b/addons/mail/views/assets.xml index 1abab3c4fd8e..8817822ef48a 100644 --- a/addons/mail/views/assets.xml +++ b/addons/mail/views/assets.xml @@ -108,7 +108,6 @@ <script type="text/javascript" src="/mail/static/src/components/thread_textual_typing_status/thread_textual_typing_status.js"></script> <script type="text/javascript" src="/mail/static/src/components/thread_typing_icon/thread_typing_icon.js"></script> <script type="text/javascript" src="/mail/static/src/components/thread_viewer/thread_viewer.js"></script> - <script type="text/javascript" src="/mail/static/src/env/env.js"></script> <script type="text/javascript" src="/mail/static/src/model/model_core.js"></script> <script type="text/javascript" src="/mail/static/src/model/model_errors.js"></script> <script type="text/javascript" src="/mail/static/src/model/model_field.js"></script> @@ -213,6 +212,12 @@ </xpath> </template> + <template id="assets_backend_prod_only" name="mail prod only assets" inherit_id="web.assets_backend_prod_only"> + <xpath expr="." position="inside"> + <script type="text/javascript" src="/mail/static/src/js/main.js"></script> + </xpath> + </template> + <template id="tests_assets" name="mail_tests_assets" inherit_id="web.tests_assets"> <xpath expr="." position="inside"> <script type="text/javascript" src="/mail/static/src/env/test_env.js"></script> diff --git a/addons/point_of_sale/static/src/js/main.js b/addons/point_of_sale/static/src/js/main.js index 3258481198f3..b765e6ccd296 100644 --- a/addons/point_of_sale/static/src/js/main.js +++ b/addons/point_of_sale/static/src/js/main.js @@ -1,6 +1,7 @@ odoo.define('web.web_client', function (require) { 'use strict'; + const AbstractService = require('web.AbstractService'); const env = require('web.env'); const WebClient = require('web.AbstractWebClient'); const Chrome = require('point_of_sale.Chrome'); @@ -37,6 +38,7 @@ odoo.define('web.web_client', function (require) { configureGui({ component: chrome }); } + AbstractService.prototype.deployServices(env); const webClient = new WebClient(); startPosApp(webClient); return webClient; diff --git a/addons/point_of_sale/static/tests/unit/helpers/test_main.js b/addons/point_of_sale/static/tests/unit/helpers/test_main.js index a5473389092c..f42e01cbd6e2 100644 --- a/addons/point_of_sale/static/tests/unit/helpers/test_main.js +++ b/addons/point_of_sale/static/tests/unit/helpers/test_main.js @@ -1,6 +1,23 @@ odoo.define('web.web_client', function (require) { // this module is required by the test + const { bus } = require('web.core'); const WebClient = require('web.AbstractWebClient'); + + // listen to unhandled rejected promises, and when the rejection is not due + // to a crash, prevent the browser from displaying an 'unhandledrejection' + // error in the console, which would make tests crash on each Promise.reject() + // something similar is done by the CrashManagerService, but by default, it + // isn't deployed in tests + bus.on('crash_manager_unhandledrejection', this, function (ev) { + if (!ev.reason || !(ev.reason instanceof Error)) { + ev.stopPropagation(); + ev.stopImmediatePropagation(); + ev.preventDefault(); + } + }); + + owl.config.mode = "dev"; + const webClient = new WebClient(); return webClient; }); diff --git a/addons/web/static/src/js/common_env.js b/addons/web/static/src/js/common_env.js index 567607d50b50..792c3b966d4f 100644 --- a/addons/web/static/src/js/common_env.js +++ b/addons/web/static/src/js/common_env.js @@ -19,7 +19,7 @@ odoo.define("web.commonEnv", function (require) { const { jsonRpc } = require("web.ajax"); const { device, isDebug } = require("web.config"); - const { bus, serviceRegistry } = require("web.core"); + const { bus } = require("web.core"); const rpc = require("web.rpc"); const session = require("web.session"); const { _t } = require("web.translation"); @@ -97,40 +97,5 @@ odoo.define("web.commonEnv", function (require) { session, }; - - // Deploy services in the env (specializations of AbstractService registered - // into the serviceRegistry) - const UndeployedServices = Object.assign({}, serviceRegistry.map); - function _deployServices() { - let done = false; - while (!done) { - // find a service with no missing dependency - const serviceName = Object.keys(UndeployedServices).find(serviceName => { - const Service = UndeployedServices[serviceName]; - return Service.prototype.dependencies.every(depName => { - return env.services[depName]; - }); - }); - if (serviceName) { - const Service = UndeployedServices[serviceName]; - const service = new Service(env); - env.services[serviceName] = service; - delete UndeployedServices[serviceName]; - service.start(); - } else { - done = true; - } - } - } - serviceRegistry.onAdd((serviceName, Service) => { - if (serviceName in env.services || serviceName in UndeployedServices) { - throw new Error(`Service ${serviceName} is already loaded.`); - } - UndeployedServices[serviceName] = Service; - _deployServices(); - }); - _deployServices(); - - return env; }); diff --git a/addons/web/static/src/js/core/abstract_service.js b/addons/web/static/src/js/core/abstract_service.js index f9ad859fef2a..5157b5b55f4e 100644 --- a/addons/web/static/src/js/core/abstract_service.js +++ b/addons/web/static/src/js/core/abstract_service.js @@ -2,6 +2,7 @@ odoo.define('web.AbstractService', function (require) { "use strict"; var Class = require('web.Class'); +const { serviceRegistry } = require("web.core"); var Mixins = require('web.mixins'); var ServicesMixin = require('web.ServicesMixin'); @@ -40,6 +41,50 @@ var AbstractService = Class.extend(Mixins.EventDispatcherMixin, ServicesMixin, { this.env.bus.trigger('do-action', payload); } }, + + //-------------------------------------------------------------------------- + // Static + //-------------------------------------------------------------------------- + + /** + * Deploy services in the env (specializations of AbstractService registered + * into the serviceRegistry). + * + * @static + * @param {Object} env + */ + deployServices(env) { + const UndeployedServices = Object.assign({}, serviceRegistry.map); + function _deployServices() { + let done = false; + while (!done) { + // find a service with no missing dependency + const serviceName = Object.keys(UndeployedServices).find(serviceName => { + const Service = UndeployedServices[serviceName]; + return Service.prototype.dependencies.every(depName => { + return env.services[depName]; + }); + }); + if (serviceName) { + const Service = UndeployedServices[serviceName]; + const service = new Service(env); + env.services[serviceName] = service; + delete UndeployedServices[serviceName]; + service.start(); + } else { + done = true; + } + } + } + serviceRegistry.onAdd((serviceName, Service) => { + if (serviceName in env.services || serviceName in UndeployedServices) { + throw new Error(`Service ${serviceName} is already loaded.`); + } + UndeployedServices[serviceName] = Service; + _deployServices(); + }); + _deployServices(); + } }); return AbstractService; diff --git a/addons/web/static/src/js/main.js b/addons/web/static/src/js/main.js index 49b90228ba23..465ea0d63d46 100644 --- a/addons/web/static/src/js/main.js +++ b/addons/web/static/src/js/main.js @@ -1,18 +1,20 @@ odoo.define('web.web_client', function (require) { "use strict"; + const AbstractService = require('web.AbstractService'); const env = require('web.env'); const session = require("web.session"); const WebClient = require('web.WebClient'); + // configure the env and set it on Owl Component owl.config.mode = env.isDebug() ? "dev" : "prod"; owl.Component.env = env; - const webClient = new WebClient(); + // deploy services in the env + AbstractService.prototype.deployServices(env); - /** - * Add the owl templates to the environment and start the web client. - */ + // add the owl templates to the environment and start the web client + const webClient = new WebClient(); async function startWebClient() { await session.is_bound; env.qweb.addTemplates(session.owlTemplates); @@ -21,7 +23,6 @@ odoo.define('web.web_client', function (require) { webClient.setElement($(document.body)); webClient.start(); } - startWebClient(); return webClient; diff --git a/addons/web/static/src/js/public/public_root_instance.js b/addons/web/static/src/js/public/public_root_instance.js index e368d421fbe4..a7422a97c26a 100644 --- a/addons/web/static/src/js/public/public_root_instance.js +++ b/addons/web/static/src/js/public/public_root_instance.js @@ -1,6 +1,7 @@ odoo.define('root.widget', function (require) { 'use strict'; +const AbstractService = require('web.AbstractService'); const env = require('web.public_env'); var lazyloader = require('web.public.lazyloader'); var rootData = require('web.public.root'); @@ -11,6 +12,11 @@ var rootData = require('web.public.root'); owl.config.mode = env.isDebug() ? "dev" : "prod"; owl.Component.env = env; +/** + * Deploy services in the env + */ +AbstractService.prototype.deployServices(env); + /** * This widget is important, because the tour manager needs a root widget in * order to work. The root widget must be a service provider with the ajax diff --git a/addons/web/static/tests/main_tests.js b/addons/web/static/tests/main_tests.js index 97d9b051bbbd..30164cea61bc 100644 --- a/addons/web/static/tests/main_tests.js +++ b/addons/web/static/tests/main_tests.js @@ -3,14 +3,23 @@ odoo.define('web.web_client', async function (require) { "use strict"; const session = require("web.session"); - const WebClient = require('web.WebClient'); + const { bus } = require('web.core'); - owl.config.mode = "dev"; + // listen to unhandled rejected promises, and when the rejection is not due + // to a crash, prevent the browser from displaying an 'unhandledrejection' + // error in the console, which would make tests crash on each Promise.reject() + // something similar is done by the CrashManagerService, but by default, it + // isn't deployed in tests + bus.on('crash_manager_unhandledrejection', this, function (ev) { + if (!ev.reason || !(ev.reason instanceof Error)) { + ev.stopPropagation(); + ev.stopImmediatePropagation(); + ev.preventDefault(); + } + }); - const webClient = new WebClient(); + owl.config.mode = "dev"; await session.is_bound; session.owlTemplates = session.owlTemplates.replace(/t-transition/g, 'transition'); - - return webClient; }); diff --git a/addons/web/views/webclient_templates.xml b/addons/web/views/webclient_templates.xml index c074328ac60d..0323694b5c4c 100644 --- a/addons/web/views/webclient_templates.xml +++ b/addons/web/views/webclient_templates.xml @@ -879,6 +879,10 @@ </t> </template> + <template id="web.assets_backend_prod_only"> + <script type="text/javascript" src="/web/static/src/js/main.js"></script> + </template> + <template id="web.webclient_bootstrap"> <t t-call="web.layout"> <t t-set="head_web"> @@ -891,9 +895,8 @@ <t t-call-assets="web.assets_backend" t-js="false"/> <t t-call-assets="web.assets_common" t-css="false"/> <t t-call-assets="web.assets_backend" t-css="false"/> + <t t-call-assets="web.assets_backend_prod_only" t-css="false"/> <t t-call="web.conditional_assets_tests"/> - - <script type="text/javascript" src="/web/static/src/js/main.js"></script> </t> <t t-set="head" t-value="head_web + (head or '')"/> <t t-set="body_classname" t-value="'o_web_client'"/> diff --git a/addons/web_editor/static/tests/field_html_tests.js b/addons/web_editor/static/tests/field_html_tests.js index 52f55653c726..21c54266fae2 100644 --- a/addons/web_editor/static/tests/field_html_tests.js +++ b/addons/web_editor/static/tests/field_html_tests.js @@ -176,6 +176,13 @@ QUnit.module('web_editor', {}, function () { '</form>', res_id: 1, }); + + // Summernote needs a RootWidget to set as parent of the ColorPaletteWidget. In the + // tests, there is no RootWidget, so we set it here to the parent of the form view, which + // can act as RootWidget, as it will honor rpc requests correctly (to the MockServer). + const rootWidget = odoo.__DEBUG__.services['root.widget']; + odoo.__DEBUG__.services['root.widget'] = form.getParent(); + await testUtils.form.clickEdit(form); var $field = form.$('.oe_form_field[name="body"]'); @@ -233,6 +240,7 @@ QUnit.module('web_editor', {}, function () { '<p>t<font style="background-color: rgb(0, 255, 255);">oto t</font><font style="" class="bg-o-color-3">oto </font><font class="bg-o-color-3" style="">to</font>to</p><p>tata</p>', "should have rendered the field correctly in edit"); + odoo.__DEBUG__.services['root.widget'] = rootWidget; form.destroy(); }); diff --git a/addons/website/static/src/js/content/website_root_instance.js b/addons/website/static/src/js/content/website_root_instance.js index 657ed16784ce..fbbefb03651c 100644 --- a/addons/website/static/src/js/content/website_root_instance.js +++ b/addons/website/static/src/js/content/website_root_instance.js @@ -1,9 +1,22 @@ odoo.define('root.widget', function (require) { 'use strict'; +const AbstractService = require('web.AbstractService'); +const env = require('web.public_env'); var lazyloader = require('web.public.lazyloader'); var websiteRootData = require('website.root'); +/** + * Configure Owl with the public env + */ +owl.config.mode = env.isDebug() ? "dev" : "prod"; +owl.Component.env = env; + +/** + * Deploy services in the env + */ +AbstractService.prototype.deployServices(env); + var websiteRoot = new websiteRootData.WebsiteRoot(null); return lazyloader.allScriptsLoaded.then(function () { return websiteRoot.attachTo(document.body).then(function () { -- GitLab