From a7e0eb20fe6baad41e4f997a2cdef6fb2d2130de Mon Sep 17 00:00:00 2001
From: Matheus Marchini <mmarchini@netflix.com>
Date: Thu, 12 Mar 2020 11:57:22 -0700
Subject: [PATCH] test: workaround for V8 8.1 inspector pause issue

`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
---
 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);