Skip to content

Commit

Permalink
inspector: do not hardcode Debugger.CallFrameId in tests
Browse files Browse the repository at this point in the history
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: #35570
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
dgozman authored and nodejs-github-bot committed Oct 13, 2020
1 parent 18ff3c8 commit 87da7a6
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 4 deletions.
9 changes: 9 additions & 0 deletions test/common/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -267,6 +272,10 @@ class InspectorSession {
`break on ${url}:${line}`);
}

pausedDetails() {
return this._pausedDetails;
}

_matchesConsoleOutputNotification(notification, type, values) {
if (!Array.isArray(values))
values = [ values ];
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-inspector-esm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions test/sequential/test-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 87da7a6

Please sign in to comment.