From 92a207cd2d0e5433f7b36e91860fbd8647fd487c Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 12 Mar 2020 11:57:22 -0700 Subject: [PATCH] test: workaround for V8 8.1 inspector pause issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test-inspector-multisession-ws` and `test-inspector-break-when-eval` will be affected by an upstream bug when we upgrade V8 to 8.1. The bug is caused when the Inspector sets a pause at the start of a function compiled with `CompileFunctionInContext`, but that function hasn't been executed yet. On both tests, this issue is triggered by pausing while in C++ executing LookupAndCompile, which is called by requiring internal modules while running `console.log`. To eliminate this issue in both tests, we add an extra `console.log` to ensure we only pause we required all internal modules we need. On `test-inspector-break-when-eval`, we also need to start the child process with `--inspect-brk` instead of `--inspect` to ensure the test is predictable (this test would occasianlly fail on slower machines, when console.log doesn't run fast enough to finish after emitting `Runtime.consoleAPICalled` and before the parent process sending `Runtime.evaluate` message. Ref: https://bugs.chromium.org/p/v8/issues/detail?id=10287 PR-URL: https://github.com/nodejs/node/pull/32234 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10287 Reviewed-By: Anna Henningsen Reviewed-By: Jiawen Geng Reviewed-By: Michaël Zasso --- test/fixtures/inspector-global-function.js | 4 ++++ test/parallel/test-inspector-multisession-ws.js | 5 +++++ test/sequential/test-inspector-break-when-eval.js | 14 ++++++++++++-- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/test/fixtures/inspector-global-function.js b/test/fixtures/inspector-global-function.js index 6155ae5298cdef..b89808b88079bb 100644 --- a/test/fixtures/inspector-global-function.js +++ b/test/fixtures/inspector-global-function.js @@ -10,4 +10,8 @@ global.sum = function() { console.log(invocations++, c); }; +// NOTE(mmarchini): Calls console.log two times to ensure we loaded every +// internal module before pausing. See +// https://bugs.chromium.org/p/v8/issues/detail?id=10287. +console.log('Loading'); console.log('Ready!'); diff --git a/test/parallel/test-inspector-multisession-ws.js b/test/parallel/test-inspector-multisession-ws.js index 6b07af0b85e987..c5ebfd6ef81ea2 100644 --- a/test/parallel/test-inspector-multisession-ws.js +++ b/test/parallel/test-inspector-multisession-ws.js @@ -20,6 +20,7 @@ session.on('Debugger.paused', () => { session.connect(); session.post('Debugger.enable'); console.log('Ready'); +console.log('Ready'); `; async function setupSession(node) { @@ -46,6 +47,10 @@ async function testSuspend(sessionA, sessionB) { await sessionA.waitForNotification('Debugger.paused', 'Initial pause'); sessionA.send({ 'method': 'Debugger.resume' }); + await sessionA.waitForNotification('Runtime.consoleAPICalled', + 'Console output'); + // NOTE(mmarchini): Remove second console.log when + // https://bugs.chromium.org/p/v8/issues/detail?id=10287 is fixed. await sessionA.waitForNotification('Runtime.consoleAPICalled', 'Console output'); sessionA.send({ 'method': 'Debugger.pause' }); diff --git a/test/sequential/test-inspector-break-when-eval.js b/test/sequential/test-inspector-break-when-eval.js index c419a0c530dc4d..371a60daaf62c9 100644 --- a/test/sequential/test-inspector-break-when-eval.js +++ b/test/sequential/test-inspector-break-when-eval.js @@ -18,7 +18,15 @@ async function setupDebugger(session) { { 'method': 'Runtime.runIfWaitingForDebugger' }, ]; session.send(commands); - await session.waitForNotification('Runtime.consoleAPICalled'); + + await session.waitForNotification('Debugger.paused', 'Initial pause'); + + // NOTE(mmarchini): We wait for the second console.log to ensure we loaded + // every internal module before pausing. See + // https://bugs.chromium.org/p/v8/issues/detail?id=10287. + const waitForReady = session.waitForConsoleOutput('log', 'Ready!'); + session.send({ 'method': 'Debugger.resume' }); + await waitForReady; } async function breakOnLine(session) { @@ -56,7 +64,9 @@ async function stepOverConsoleStatement(session) { } async function runTests() { - const child = new NodeInstance(['--inspect=0'], undefined, script); + // NOTE(mmarchini): Use --inspect-brk to improve avoid undeterministic + // behavior. + const child = new NodeInstance(['--inspect-brk=0'], undefined, script); const session = await child.connectInspectorSession(); await setupDebugger(session); await breakOnLine(session);