From 87da7a6d151d482494257146d83938b59501e94d Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 9 Oct 2020 09:12:40 -0700 Subject: [PATCH] inspector: do not hardcode Debugger.CallFrameId in tests Debugger.CallFrameId is defined as an opaque string [1]. Some tests currently hardcode the value, relying on undocumented internal details of V8. This makes it hard for V8 to change the internal representation. We should instead use the reported call frame id from the Debugger.paused event directly. This is how every inspector client does it. [1] https://chromedevtools.github.io/devtools-protocol/tot/Debugger/#type-CallFrameId PR-URL: https://github.com/nodejs/node/pull/35570 Reviewed-By: Aleksei Koziatinskii Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott --- test/common/inspector-helper.js | 9 +++++++++ test/parallel/test-inspector-esm.js | 2 +- test/sequential/test-inspector.js | 6 +++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index 77d7928af1..5aea807b7c 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -130,6 +130,7 @@ class InspectorSession { this._unprocessedNotifications = []; this._notificationCallback = null; this._scriptsIdsByUrl = new Map(); + this._pausedDetails = null; let buffer = Buffer.alloc(0); socket.on('data', (data) => { @@ -179,6 +180,10 @@ class InspectorSession { this.mainScriptId = scriptId; } } + if (message.method === 'Debugger.paused') + this._pausedDetails = message.params; + if (message.method === 'Debugger.resumed') + this._pausedDetails = null; if (this._notificationCallback) { // In case callback needs to install another @@ -267,6 +272,10 @@ class InspectorSession { `break on ${url}:${line}`); } + pausedDetails() { + return this._pausedDetails; + } + _matchesConsoleOutputNotification(notification, type, values) { if (!Array.isArray(values)) values = [ values ]; diff --git a/test/parallel/test-inspector-esm.js b/test/parallel/test-inspector-esm.js index 707fa7bb56..b9813e50c2 100644 --- a/test/parallel/test-inspector-esm.js +++ b/test/parallel/test-inspector-esm.js @@ -78,7 +78,7 @@ async function testBreakpoint(session) { let { result } = await session.send({ 'method': 'Debugger.evaluateOnCallFrame', 'params': { - 'callFrameId': '{"ordinal":0,"injectedScriptId":1}', + 'callFrameId': session.pausedDetails().callFrames[0].callFrameId, 'expression': 'k + t', 'objectGroup': 'console', 'includeCommandLineAPI': true, diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js index a1ef109cf1..b58d70c864 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -116,7 +116,7 @@ async function testBreakpoint(session) { let { result } = await session.send({ 'method': 'Debugger.evaluateOnCallFrame', 'params': { - 'callFrameId': '{"ordinal":0,"injectedScriptId":1}', + 'callFrameId': session.pausedDetails().callFrames[0].callFrameId, 'expression': 'k + t', 'objectGroup': 'console', 'includeCommandLineAPI': true, @@ -150,7 +150,7 @@ async function testI18NCharacters(session) { const chars = 'טֶ字и'; session.send({ 'method': 'Debugger.evaluateOnCallFrame', 'params': { - 'callFrameId': '{"ordinal":0,"injectedScriptId":1}', + 'callFrameId': session.pausedDetails().callFrames[0].callFrameId, 'expression': `console.log("${chars}")`, 'objectGroup': 'console', 'includeCommandLineAPI': true, @@ -276,7 +276,7 @@ async function testCommandLineAPI(session) { result = await session.send( { 'method': 'Debugger.evaluateOnCallFrame', 'params': { - 'callFrameId': '{"ordinal":0,"injectedScriptId":1}', + 'callFrameId': session.pausedDetails().callFrames[0].callFrameId, 'expression': `( require(${printBModuleStr}), require.cache[${printBModuleStr}].parent.id