From 3933dac0c7ff154e24f3068e8a1a55dd65f3b720 Mon Sep 17 00:00:00 2001 From: "Michael (mcm)" Date: Thu, 11 May 2023 14:38:34 +0000 Subject: [PATCH 1/4] [REF] web: remove top level await This commit removes two top level await in test files and reorganises a bit the tests setup as the module loading sequence has changed. The commit also converts the modules that contained these top level await to esm. The goal of removing async modules is to simplify the module loader in the future. task id: 3265979 Part-of: odoo/odoo#121185 --- addons/web/static/tests/helpers/legacy.js | 12 --- addons/web/static/tests/helpers/session.js | 2 +- .../static/tests/legacy/helpers/test_utils.js | 39 +++---- addons/web/static/tests/setup.js | 100 +++++++++--------- addons/web/static/tests/webclient/helpers.js | 4 - 5 files changed, 66 insertions(+), 91 deletions(-) delete mode 100644 addons/web/static/tests/helpers/legacy.js diff --git a/addons/web/static/tests/helpers/legacy.js b/addons/web/static/tests/helpers/legacy.js deleted file mode 100644 index ce5150644e2..00000000000 --- a/addons/web/static/tests/helpers/legacy.js +++ /dev/null @@ -1,12 +0,0 @@ -odoo.define( - "web.test_legacy", - ["web.session", "web.SessionOverrideForTests", "web.test_utils"], - async (require) => { - require("web.SessionOverrideForTests"); - require("web.test_utils"); - const session = require("web.session"); - await session.is_bound; // await for templates from server - - return { legacyProm: session.is_bound }; - } -); diff --git a/addons/web/static/tests/helpers/session.js b/addons/web/static/tests/helpers/session.js index 984a7b77026..b861f07e076 100644 --- a/addons/web/static/tests/helpers/session.js +++ b/addons/web/static/tests/helpers/session.js @@ -1,4 +1,4 @@ -/** @odoo-module alias=web.SessionOverrideForTests **/ +/** @odoo-module **/ // Override the Session.session_reload function // The wowl test infrastructure does set a correct odoo global value before each test diff --git a/addons/web/static/tests/legacy/helpers/test_utils.js b/addons/web/static/tests/legacy/helpers/test_utils.js index 5ae95570792..130615fe345 100644 --- a/addons/web/static/tests/legacy/helpers/test_utils.js +++ b/addons/web/static/tests/legacy/helpers/test_utils.js @@ -1,5 +1,4 @@ -odoo.define('web.test_utils', ["web.session", "web.test_utils_create", "web.test_utils_control_panel", "web.test_utils_dom", "web.test_utils_fields", "web.test_utils_file", "web.test_utils_form", "web.test_utils_graph", "web.test_utils_kanban", "web.test_utils_mock", "web.test_utils_modal", "web.test_utils_pivot", "web.tools"], async function (require) { - "use strict"; +/** @odoo-module alias=web.test_utils **/ /** * Test Utils @@ -10,19 +9,18 @@ odoo.define('web.test_utils', ["web.session", "web.test_utils_create", "web.test * instance of a view, appended in the dom, ready to be tested. */ - const session = require('web.session'); - const testUtilsCreate = require('web.test_utils_create'); - const testUtilsControlPanel = require('web.test_utils_control_panel'); - const testUtilsDom = require('web.test_utils_dom'); - const testUtilsFields = require('web.test_utils_fields'); - const testUtilsFile = require('web.test_utils_file'); - const testUtilsForm = require('web.test_utils_form'); - const testUtilsGraph = require('web.test_utils_graph'); - const testUtilsKanban = require('web.test_utils_kanban'); - const testUtilsMock = require('web.test_utils_mock'); - const testUtilsModal = require('web.test_utils_modal'); - const testUtilsPivot = require('web.test_utils_pivot'); - const tools = require('web.tools'); + import testUtilsCreate from "web.test_utils_create"; + import testUtilsControlPanel from "web.test_utils_control_panel"; + import testUtilsDom from "web.test_utils_dom"; + import testUtilsFields from "web.test_utils_fields"; + import testUtilsFile from "web.test_utils_file"; + import testUtilsForm from "web.test_utils_form"; + import testUtilsGraph from "web.test_utils_graph"; + import testUtilsKanban from "web.test_utils_kanban"; + import testUtilsMock from "web.test_utils_mock"; + import testUtilsModal from "web.test_utils_modal"; + import testUtilsPivot from "web.test_utils_pivot"; + import tools from "web.tools"; function deprecated(fn, type) { @@ -113,15 +111,7 @@ odoo.define('web.test_utils', ["web.session", "web.test_utils_create", "web.test return testUtilsDom.returnAfterNextAnimationFrame(); } - // Loading static files cannot be properly simulated when their real content is - // really needed. This is the case for static XML files so we load them here, - // before starting the qunit test suite. - // (session.js is in charge of loading the static xml bundle and we also have - // to load xml files that are normally lazy loaded by specific widgets). - // Assets can also contain static xml files. They are loaded when the session - // is launched. - await session.is_bound; - return { + export default { mock: { addMockEnvironment: testUtilsMock.addMockEnvironment, addMockEnvironmentOwl: testUtilsMock.addMockEnvironmentOwl, @@ -266,4 +256,3 @@ odoo.define('web.test_utils', ["web.session", "web.test_utils_create", "web.test triggerPositionalMouseEvent: deprecated(testUtilsDom.triggerPositionalMouseEvent, 'dom'), unpatch: deprecated(testUtilsMock.unpatch, 'mock'), }; -}); diff --git a/addons/web/static/tests/setup.js b/addons/web/static/tests/setup.js index bf7e7ef81b3..716b9f77755 100644 --- a/addons/web/static/tests/setup.js +++ b/addons/web/static/tests/setup.js @@ -7,7 +7,6 @@ import { assets, templates } from "@web/core/assets"; import { browser, makeRAMLocalStorage } from "@web/core/browser/browser"; import { nextTick, patchTimeZone, patchWithCleanup } from "@web/../tests/helpers/utils"; import { memoize } from "@web/core/utils/functions"; -import { legacyProm } from "web.test_legacy"; import { registerCleanup } from "./helpers/cleanup"; import { utils } from "./helpers/mock_env"; import { session as sessionInfo } from "@web/session"; @@ -19,6 +18,7 @@ transitionConfig.disabled = true; import { patch } from "@web/core/utils/patch"; import { App, whenReady } from "@odoo/owl"; import { currencies } from "@web/core/currency"; +import "./helpers/session"; const { prepareRegistriesWithCleanup } = utils; @@ -300,59 +300,52 @@ function removeUnwantedAttrsFromTemplates(attrs) { } } -// alt attribute causes issues with scroll tests. Indeed, alt is -// displayed between the time we scroll programatically and the time -// we assert for the scroll position. The src attribute is removed -// as well to make sure images won't trigger a GET request on the -// server. - -// Clean up templates that have already been added. -removeUnwantedAttrsFromTemplates(["alt", "src"]); - -const { loadXML, getBundle, loadJS, loadCSS } = assets; -patch(assets, "TestAssetsLoadXML", { - loadXML: function (templates) { - console.log("%c[assets] fetch XML ressource", "color: #66e; font-weight: bold;"); - // Clean up new templates that might be added later. - loadXML(templates); - removeUnwantedAttrsFromTemplates(["alt", "src"]); - }, - getBundle: memoize(async function (xmlID) { - console.log( - "%c[assets] fetch libs from xmlID: " + xmlID, - "color: #66e; font-weight: bold;" - ); - return getBundle(xmlID); - }), - loadJS: memoize(async function (ressource) { - if (ressource.match(/\/static(\/\S+\/|\/)libs?/)) { +function patchAssets() { + const { loadXML, getBundle, loadJS, loadCSS } = assets; + patch(assets, "TestAssetsLoadXML", { + loadXML: function (templates) { + console.log("%c[assets] fetch XML ressource", "color: #66e; font-weight: bold;"); + // Clean up new templates that might be added later. + loadXML(templates); + removeUnwantedAttrsFromTemplates(["alt", "src"]); + }, + getBundle: memoize(async function (xmlID) { console.log( - "%c[assets] fetch (mock) JS ressource: " + ressource, + "%c[assets] fetch libs from xmlID: " + xmlID, "color: #66e; font-weight: bold;" ); - return nextTick(); - } - console.log( - "%c[assets] fetch JS ressource: " + ressource, - "color: #66e; font-weight: bold;" - ); - return loadJS(ressource); - }), - loadCSS: memoize(async function (ressource) { - if (ressource.match(/\/static(\/\S+\/|\/)libs?/)) { + return getBundle(xmlID); + }), + loadJS: memoize(async function (ressource) { + if (ressource.match(/\/static(\/\S+\/|\/)libs?/)) { + console.log( + "%c[assets] fetch (mock) JS ressource: " + ressource, + "color: #66e; font-weight: bold;" + ); + return nextTick(); + } console.log( - "%c[assets] fetch (mock) CSS ressource: " + ressource, + "%c[assets] fetch JS ressource: " + ressource, "color: #66e; font-weight: bold;" ); - return nextTick(); - } - console.log( - "%c[assets] fetch CSS ressource: " + ressource, - "color: #66e; font-weight: bold;" - ); - return loadCSS(ressource); - }), -}); + return loadJS(ressource); + }), + loadCSS: memoize(async function (ressource) { + if (ressource.match(/\/static(\/\S+\/|\/)libs?/)) { + console.log( + "%c[assets] fetch (mock) CSS ressource: " + ressource, + "color: #66e; font-weight: bold;" + ); + return nextTick(); + } + console.log( + "%c[assets] fetch CSS ressource: " + ressource, + "color: #66e; font-weight: bold;" + ); + return loadCSS(ressource); + }), + }); +} export async function setupTests() { // uncomment to debug memory leaks in qunit suite @@ -389,7 +382,16 @@ export async function setupTests() { patchOwlApp(); }); - await Promise.all([whenReady(), legacyProm]); + await Promise.all([whenReady(), session.is_bound]); + + // alt attribute causes issues with scroll tests. Indeed, alt is + // displayed between the time we scroll programatically and the time + // we assert for the scroll position. The src attribute is removed + // as well to make sure images won't trigger a GET request on the + // server. + // Clean up templates that have already been added. + removeUnwantedAttrsFromTemplates(["alt", "src"]); + patchAssets(); // make sure images do not trigger a GET on the server new MutationObserver((mutations) => { diff --git a/addons/web/static/tests/webclient/helpers.js b/addons/web/static/tests/webclient/helpers.js index 36139719f6f..6095b6d13fd 100644 --- a/addons/web/static/tests/webclient/helpers.js +++ b/addons/web/static/tests/webclient/helpers.js @@ -19,10 +19,6 @@ import { effectService } from "@web/core/effects/effect_service"; import { hotkeyService } from "@web/core/hotkeys/hotkey_service"; import { menuService } from "@web/webclient/menus/menu_service"; import { WebClient } from "@web/webclient/webclient"; -// This import is needed because of it's sideeffects, for exemple : -// web.test_utils easyload xml templates at line : 124:130. -// Also it set the autocomplete delay time for the field Many2One at 0 for the tests at line : 132:137 -import "web.test_legacy"; import AbstractService from "web.AbstractService"; import ActionMenus from "web.ActionMenus"; import basicFields from "web.basic_fields"; From 03ad36676f9a8e8fd2c62899a19f84ae18a25219 Mon Sep 17 00:00:00 2001 From: "Michael (mcm)" Date: Fri, 26 May 2023 12:42:32 +0000 Subject: [PATCH 2/4] [FIX] web: reset uniqueId to 0 in tests The commit [1] removed two async module and caused a reorganisation on the module loading. Since that commit, `uniqueId` did not begin with the id 1 in its test and so the test failed. Now, the variable containing the last id of `uniqueId` is set on the function itself. This allows us to reset the id in the test. [1]: 96687f29496ab4cf16ae2ab1584ef1a6a3fb4a11 Part-of: odoo/odoo#121185 --- addons/web/static/src/core/utils/functions.js | 5 +++-- addons/web/static/tests/core/utils/functions_tests.js | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/addons/web/static/src/core/utils/functions.js b/addons/web/static/src/core/utils/functions.js index dfc8f24b36f..d80bca8348a 100644 --- a/addons/web/static/src/core/utils/functions.js +++ b/addons/web/static/src/core/utils/functions.js @@ -21,7 +21,6 @@ export function memoize(func) { }[funcName]; } -let idCounter = 0; /** * Generate a unique integer id (unique within the entire client session). * Useful for temporary DOM ids. @@ -30,5 +29,7 @@ let idCounter = 0; * @returns {string} */ export function uniqueId(prefix = "") { - return `${prefix}${++idCounter}`; + return `${prefix}${++uniqueId.nextId}`; } +// set nextId on the function itself to be able to patch then +uniqueId.nextId = 0; diff --git a/addons/web/static/tests/core/utils/functions_tests.js b/addons/web/static/tests/core/utils/functions_tests.js index b3c9466e8f7..3ab07da0150 100644 --- a/addons/web/static/tests/core/utils/functions_tests.js +++ b/addons/web/static/tests/core/utils/functions_tests.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import { memoize, uniqueId } from "@web/core/utils/functions"; +import { patchWithCleanup } from "../../helpers/utils"; QUnit.module("utils", () => { QUnit.module("Functions"); @@ -70,6 +71,7 @@ QUnit.module("utils", () => { }); QUnit.test("uniqueId", (assert) => { + patchWithCleanup(uniqueId, { nextId: 0 }); assert.strictEqual(uniqueId("test_"), "test_1"); assert.strictEqual(uniqueId("bla"), "bla2"); assert.strictEqual(uniqueId("test_"), "test_3"); From 69afcfcc1ad78e04622c77ff396f84d40d51898c Mon Sep 17 00:00:00 2001 From: "Michael (mcm)" Date: Fri, 26 May 2023 12:49:37 +0000 Subject: [PATCH 3/4] [FIX] web: prevent false test fail The commit [1] removed two async module and caused a reorganisation on the module loading. Since that commit, two tests that throw an error failed because the error message was written in the console and the server considered this as a test fail. Now, the error message is not written in the console anymore. [1]: 96687f29496ab4cf16ae2ab1584ef1a6a3fb4a11 Part-of: odoo/odoo#121185 --- .../web/static/tests/views/form/form_view_tests.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/addons/web/static/tests/views/form/form_view_tests.js b/addons/web/static/tests/views/form/form_view_tests.js index 07643c5340f..d9c00a6d652 100644 --- a/addons/web/static/tests/views/form/form_view_tests.js +++ b/addons/web/static/tests/views/form/form_view_tests.js @@ -13041,6 +13041,13 @@ QUnit.module("Views", (hooks) => { QUnit.test( "coming to an action with an error from a form view with a dirty x2m", async function (assert) { + const handler = (ev) => { + // need to preventDefault to remove error from console (so python test pass) + ev.preventDefault(); + }; + window.addEventListener("unhandledrejection", handler); + registerCleanup(() => window.removeEventListener("unhandledrejection", handler)); + class TestClientAction extends Component { setup() { throw new Error("Something went wrong"); @@ -13124,6 +13131,13 @@ QUnit.module("Views", (hooks) => { QUnit.test( "coming to an action with an error from a form view with a record in creation", async function (assert) { + const handler = (ev) => { + // need to preventDefault to remove error from console (so python test pass) + ev.preventDefault(); + }; + window.addEventListener("unhandledrejection", handler); + registerCleanup(() => window.removeEventListener("unhandledrejection", handler)); + class TestClientAction extends Component { setup() { throw new Error("Something went wrong"); From ac153558a698ae4d2de8e1275be259283b7e3eca Mon Sep 17 00:00:00 2001 From: "Michael (mcm)" Date: Fri, 26 May 2023 12:51:16 +0000 Subject: [PATCH 4/4] [FIX] web, mail: indirectly call a patched function The commit [1] removed two async module and caused a reorganisation on the module loading. Since that commit, the patches could be applied in a different order and be applied after a save of one these patches. This commit fixes a case where we save the reference of a patched function that we call it later with missing patches. [1]: 96687f29496ab4cf16ae2ab1584ef1a6a3fb4a11 Part-of: odoo/odoo#121185 --- .../mail/static/tests/helpers/test_utils.js | 3 +- .../tests/core/debug/debug_manager_tests.js | 3 +- addons/web/static/tests/helpers/mock_env.js | 52 ++++++++++--------- addons/web/static/tests/setup.js | 4 +- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/addons/mail/static/tests/helpers/test_utils.js b/addons/mail/static/tests/helpers/test_utils.js index faa9dbcb21f..162772a4fb3 100644 --- a/addons/mail/static/tests/helpers/test_utils.js +++ b/addons/mail/static/tests/helpers/test_utils.js @@ -16,12 +16,11 @@ import { registerCleanup } from "@web/../tests/helpers/cleanup"; import { clearRegistryWithCleanup, registryNamesToCloneWithCleanup, - utils, + prepareRegistriesWithCleanup, } from "@web/../tests/helpers/mock_env"; import { getFixture, makeDeferred, patchWithCleanup } from "@web/../tests/helpers/utils"; import { doAction, getActionManagerServerData } from "@web/../tests/webclient/helpers"; -const { prepareRegistriesWithCleanup } = utils; const { afterNextRender } = App; // load emoji data once, when the test suite starts. diff --git a/addons/web/static/tests/core/debug/debug_manager_tests.js b/addons/web/static/tests/core/debug/debug_manager_tests.js index 0a0760530e1..70e7eef664b 100644 --- a/addons/web/static/tests/core/debug/debug_manager_tests.js +++ b/addons/web/static/tests/core/debug/debug_manager_tests.js @@ -10,7 +10,7 @@ import { uiService } from "@web/core/ui/ui_service"; import { useSetupView } from "@web/views/view_hook"; import { ActionDialog } from "@web/webclient/actions/action_dialog"; import { hotkeyService } from "@web/core/hotkeys/hotkey_service"; -import { makeTestEnv, utils } from "../../helpers/mock_env"; +import { makeTestEnv, prepareRegistriesWithCleanup } from "../../helpers/mock_env"; import { fakeCompanyService, fakeCommandService, @@ -33,7 +33,6 @@ import { editSearchView, editView, setDefaults, viewMetadata } from "@web/views/ import { fieldService } from "@web/core/field_service"; import { Component, xml } from "@odoo/owl"; -const { prepareRegistriesWithCleanup } = utils; export class DebugMenuParent extends Component { setup() { diff --git a/addons/web/static/tests/helpers/mock_env.js b/addons/web/static/tests/helpers/mock_env.js index d7e3828f722..7b31a463907 100644 --- a/addons/web/static/tests/helpers/mock_env.js +++ b/addons/web/static/tests/helpers/mock_env.js @@ -60,36 +60,38 @@ export const registryNamesToCloneWithCleanup = [ "wowlToLegacyServiceMappers", ]; -function prepareRegistriesWithCleanup() { - // Clone registries - registryNamesToCloneWithCleanup.forEach((registryName) => - cloneRegistryWithCleanup(registry.category(registryName)) - ); +export const utils = { + prepareRegistriesWithCleanup() { + // Clone registries + registryNamesToCloneWithCleanup.forEach((registryName) => + cloneRegistryWithCleanup(registry.category(registryName)) + ); - // Clear registries - clearRegistryWithCleanup(registry.category("command_categories")); - clearRegistryWithCleanup(registry.category("debug")); - clearRegistryWithCleanup(registry.category("error_dialogs")); - clearRegistryWithCleanup(registry.category("favoriteMenu")); - clearRegistryWithCleanup(registry.category("ir.actions.report handlers")); - clearRegistryWithCleanup(registry.category("main_components")); - clearRegistryWithCleanup(registry.category("wowlToLegacyServiceMappers")); + // Clear registries + clearRegistryWithCleanup(registry.category("command_categories")); + clearRegistryWithCleanup(registry.category("debug")); + clearRegistryWithCleanup(registry.category("error_dialogs")); + clearRegistryWithCleanup(registry.category("favoriteMenu")); + clearRegistryWithCleanup(registry.category("ir.actions.report handlers")); + clearRegistryWithCleanup(registry.category("main_components")); + clearRegistryWithCleanup(registry.category("wowlToLegacyServiceMappers")); - clearRegistryWithCleanup(registry.category("services")); - clearServicesMetadataWithCleanup(); + clearRegistryWithCleanup(registry.category("services")); + clearServicesMetadataWithCleanup(); - clearRegistryWithCleanup(registry.category("systray")); - clearRegistryWithCleanup(registry.category("user_menuitems")); - clearRegistryWithCleanup(registry.category("kanban_examples")); - clearRegistryWithCleanup(registry.category("__processed_archs__")); - // fun fact: at least one registry is missing... this shows that we need a - // better design for the way we clear these registries... -} + clearRegistryWithCleanup(registry.category("systray")); + clearRegistryWithCleanup(registry.category("user_menuitems")); + clearRegistryWithCleanup(registry.category("kanban_examples")); + clearRegistryWithCleanup(registry.category("__processed_archs__")); + // fun fact: at least one registry is missing... this shows that we need a + // better design for the way we clear these registries... + }, +}; // This is exported in a utils object to allow for patching -export const utils = { - prepareRegistriesWithCleanup, -}; +export function prepareRegistriesWithCleanup() { + return utils.prepareRegistriesWithCleanup(); +} /** * @typedef {import("@web/env").OdooEnv} OdooEnv diff --git a/addons/web/static/tests/setup.js b/addons/web/static/tests/setup.js index 716b9f77755..5c5dc7084d8 100644 --- a/addons/web/static/tests/setup.js +++ b/addons/web/static/tests/setup.js @@ -8,7 +8,7 @@ import { browser, makeRAMLocalStorage } from "@web/core/browser/browser"; import { nextTick, patchTimeZone, patchWithCleanup } from "@web/../tests/helpers/utils"; import { memoize } from "@web/core/utils/functions"; import { registerCleanup } from "./helpers/cleanup"; -import { utils } from "./helpers/mock_env"; +import { prepareRegistriesWithCleanup } from "./helpers/mock_env"; import { session as sessionInfo } from "@web/session"; import { config as transitionConfig } from "@web/core/transition"; import { loadLanguages } from "@web/core/l10n/translation"; @@ -20,8 +20,6 @@ import { App, whenReady } from "@odoo/owl"; import { currencies } from "@web/core/currency"; import "./helpers/session"; -const { prepareRegistriesWithCleanup } = utils; - function stringifyObjectValues(obj, properties) { let res = ""; for (const dotted of properties) {