From 683a1f7bb675bac963fb0b7f78227a9a31ae4945 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 14 Aug 2024 12:56:32 +0200 Subject: [PATCH] [FIX] runtime: log if willStart takes more than 3s Before this commit, when willStart/willUpdateProps took more than 3s, a console.warn was done. In odoo, when a warning is logged during a test, the test fails and the build is considered as "in error". There's a component that loads several resources (sequentially) in its onWillStart, which *sometimes* takes more than 3s, making builds fail non deterministically. Since a recent change (which adds another call in the problematic onWillStart), the warning gets logged quite often. A quick fix is necessary, so we change the warn into a log, which won't make build fail. We may consider alternatives in the future though: - add a parameter to onWillStart, to disable the timeout, or to specify the delay (which is 3s by default) - do not warn in test mode --- src/runtime/lifecycle_hooks.ts | 2 +- .../__snapshots__/lifecycle.test.ts.snap | 8 ++-- tests/components/lifecycle.test.ts | 38 +++++++++---------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/runtime/lifecycle_hooks.ts b/src/runtime/lifecycle_hooks.ts index 992900864..fa62b3087 100644 --- a/src/runtime/lifecycle_hooks.ts +++ b/src/runtime/lifecycle_hooks.ts @@ -29,7 +29,7 @@ function wrapError(fn: (...args: any[]) => any, hookName: string) { new Promise((resolve) => setTimeout(() => resolve(TIMEOUT), 3000)), ]).then((res) => { if (res === TIMEOUT && node.fiber === fiber && node.status <= 2) { - console.warn(timeoutError); + console.log(timeoutError); } }); } diff --git a/tests/components/__snapshots__/lifecycle.test.ts.snap b/tests/components/__snapshots__/lifecycle.test.ts.snap index 8b5bce58b..516c6a228 100644 --- a/tests/components/__snapshots__/lifecycle.test.ts.snap +++ b/tests/components/__snapshots__/lifecycle.test.ts.snap @@ -683,7 +683,7 @@ exports[`lifecycle hooks sub widget (inside sub node): hooks are correctly calle }" `; -exports[`lifecycle hooks timeout in onWillStart doesn't emit a warning if app is destroyed 1`] = ` +exports[`lifecycle hooks timeout in onWillStart doesn't emit a console log if app is destroyed 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; @@ -696,7 +696,7 @@ exports[`lifecycle hooks timeout in onWillStart doesn't emit a warning if app is }" `; -exports[`lifecycle hooks timeout in onWillStart emits a warning 1`] = ` +exports[`lifecycle hooks timeout in onWillStart emits a console log 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; @@ -709,7 +709,7 @@ exports[`lifecycle hooks timeout in onWillStart emits a warning 1`] = ` }" `; -exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 1`] = ` +exports[`lifecycle hooks timeout in onWillUpdateProps emits a console log 1`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; @@ -723,7 +723,7 @@ exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 1`] = ` }" `; -exports[`lifecycle hooks timeout in onWillUpdateProps emits a warning 2`] = ` +exports[`lifecycle hooks timeout in onWillUpdateProps emits a console log 2`] = ` "function anonymous(app, bdom, helpers ) { let { text, createBlock, list, multi, html, toggler, comment } = bdom; diff --git a/tests/components/lifecycle.test.ts b/tests/components/lifecycle.test.ts index d223f54e7..af1ec4021 100644 --- a/tests/components/lifecycle.test.ts +++ b/tests/components/lifecycle.test.ts @@ -112,10 +112,10 @@ describe("lifecycle hooks", () => { await mount(Test, fixture); }); - test("timeout in onWillStart emits a warning", async () => { - const { warn } = console; - let warnArgs: any[]; - console.warn = jest.fn((...args) => (warnArgs = args)); + test("timeout in onWillStart emits a console log", async () => { + const { log } = console; + let logArgs: any[]; + console.log = jest.fn((...args) => (logArgs = args)); const { setTimeout } = window; let timeoutCbs: any = {}; let timeoutId = 0; @@ -138,17 +138,17 @@ describe("lifecycle hooks", () => { } await nextMicroTick(); await nextMicroTick(); - expect(console.warn).toHaveBeenCalledTimes(1); - expect(warnArgs![0]!.message).toBe("onWillStart's promise hasn't resolved after 3 seconds"); + expect(console.log).toHaveBeenCalledTimes(1); + expect(logArgs![0]!.message).toBe("onWillStart's promise hasn't resolved after 3 seconds"); } finally { - console.warn = warn; + console.log = log; window.setTimeout = setTimeout; } }); - test("timeout in onWillStart doesn't emit a warning if app is destroyed", async () => { - const { warn } = console; - console.warn = jest.fn(); + test("timeout in onWillStart doesn't emit a console log if app is destroyed", async () => { + const { log } = console; + console.log = jest.fn(); const { setTimeout } = window; let timeoutCbs: any = {}; let timeoutId = 0; @@ -172,14 +172,14 @@ describe("lifecycle hooks", () => { } await nextMicroTick(); await nextMicroTick(); - expect(console.warn).toHaveBeenCalledTimes(0); + expect(console.log).toHaveBeenCalledTimes(0); } finally { - console.warn = warn; + console.log = log; window.setTimeout = setTimeout; } }); - test("timeout in onWillUpdateProps emits a warning", async () => { + test("timeout in onWillUpdateProps emits a console log", async () => { class Child extends Component { static template = xml``; setup() { @@ -193,9 +193,9 @@ describe("lifecycle hooks", () => { } const parent = await mount(Parent, fixture, { test: true }); - const { warn } = console; - let warnArgs: any[]; - console.warn = jest.fn((...args) => (warnArgs = args)); + const { log } = console; + let logArgs: any[]; + console.log = jest.fn((...args) => (logArgs = args)); const { setTimeout } = window; let timeoutCbs: any = {}; let timeoutId = 0; @@ -218,12 +218,12 @@ describe("lifecycle hooks", () => { delete timeoutCbs[id]; } await tick; - expect(console.warn).toHaveBeenCalledTimes(1); - expect(warnArgs![0]!.message).toBe( + expect(console.log).toHaveBeenCalledTimes(1); + expect(logArgs![0]!.message).toBe( "onWillUpdateProps's promise hasn't resolved after 3 seconds" ); } finally { - console.warn = warn; + console.log = log; window.setTimeout = setTimeout; } });