Skip to content

Commit 92a207c

Browse files
mmarchinitargos
authored andcommittedApr 22, 2020
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 PR-URL: #32234 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10287 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 9b6f678 commit 92a207c

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed
 

‎test/fixtures/inspector-global-function.js

+4
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,8 @@ global.sum = function() {
1010
console.log(invocations++, c);
1111
};
1212

13+
// NOTE(mmarchini): Calls console.log two times to ensure we loaded every
14+
// internal module before pausing. See
15+
// https://bugs.chromium.org/p/v8/issues/detail?id=10287.
16+
console.log('Loading');
1317
console.log('Ready!');

‎test/parallel/test-inspector-multisession-ws.js

+5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ session.on('Debugger.paused', () => {
2020
session.connect();
2121
session.post('Debugger.enable');
2222
console.log('Ready');
23+
console.log('Ready');
2324
`;
2425

2526
async function setupSession(node) {
@@ -46,6 +47,10 @@ async function testSuspend(sessionA, sessionB) {
4647
await sessionA.waitForNotification('Debugger.paused', 'Initial pause');
4748
sessionA.send({ 'method': 'Debugger.resume' });
4849

50+
await sessionA.waitForNotification('Runtime.consoleAPICalled',
51+
'Console output');
52+
// NOTE(mmarchini): Remove second console.log when
53+
// https://bugs.chromium.org/p/v8/issues/detail?id=10287 is fixed.
4954
await sessionA.waitForNotification('Runtime.consoleAPICalled',
5055
'Console output');
5156
sessionA.send({ 'method': 'Debugger.pause' });

‎test/sequential/test-inspector-break-when-eval.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,15 @@ async function setupDebugger(session) {
1818
{ 'method': 'Runtime.runIfWaitingForDebugger' },
1919
];
2020
session.send(commands);
21-
await session.waitForNotification('Runtime.consoleAPICalled');
21+
22+
await session.waitForNotification('Debugger.paused', 'Initial pause');
23+
24+
// NOTE(mmarchini): We wait for the second console.log to ensure we loaded
25+
// every internal module before pausing. See
26+
// https://bugs.chromium.org/p/v8/issues/detail?id=10287.
27+
const waitForReady = session.waitForConsoleOutput('log', 'Ready!');
28+
session.send({ 'method': 'Debugger.resume' });
29+
await waitForReady;
2230
}
2331

2432
async function breakOnLine(session) {
@@ -56,7 +64,9 @@ async function stepOverConsoleStatement(session) {
5664
}
5765

5866
async function runTests() {
59-
const child = new NodeInstance(['--inspect=0'], undefined, script);
67+
// NOTE(mmarchini): Use --inspect-brk to improve avoid undeterministic
68+
// behavior.
69+
const child = new NodeInstance(['--inspect-brk=0'], undefined, script);
6070
const session = await child.connectInspectorSession();
6171
await setupDebugger(session);
6272
await breakOnLine(session);

0 commit comments

Comments
 (0)
Please sign in to comment.