Skip to content

Commit 7ffe24d

Browse files
author
Natallia Harshunova
committed
Fix comments and add tests
1 parent f49aa5b commit 7ffe24d

7 files changed

Lines changed: 232 additions & 122 deletions

File tree

‎src/DevtoolsUtils.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,17 @@
66

77
import {
88
type Issue,
9+
type AggregatedIssue,
910
type IssuesManagerEventTypes,
11+
MarkdownIssueDescription,
12+
Marked,
1013
Common,
1114
I18n,
1215
} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
1316

17+
import {ISSUE_UTILS} from './issue-descriptions.js';
18+
import {logger} from './logger.js';
19+
1420
export function extractUrlLikeFromDevToolsTitle(
1521
title: string,
1622
): string | undefined {
@@ -69,6 +75,48 @@ export class FakeIssuesManager extends Common.ObjectWrapper
6975
}
7076
}
7177

78+
export function mapIssueToMessageObject(issue: AggregatedIssue) {
79+
const count = issue.getAggregatedIssuesCount();
80+
const markdownDescription = issue.getDescription();
81+
const filename = markdownDescription?.file;
82+
if (!markdownDescription) {
83+
logger(`no description found for issue:` + issue.code);
84+
return null;
85+
}
86+
const rawMarkdown = filename
87+
? ISSUE_UTILS.getIssueDescription(filename)
88+
: null;
89+
if (!rawMarkdown) {
90+
logger(`no markdown ${filename} found for issue:` + issue.code);
91+
return null;
92+
}
93+
let processedMarkdown: string;
94+
let title: string | null;
95+
96+
try {
97+
processedMarkdown = MarkdownIssueDescription.substitutePlaceholders(
98+
rawMarkdown,
99+
markdownDescription.substitutions,
100+
);
101+
const markdownAst = Marked.Marked.lexer(processedMarkdown);
102+
title = MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst);
103+
} catch {
104+
logger('error parsing markdown for issue ' + issue.code());
105+
return null;
106+
}
107+
if (!title) {
108+
logger('cannot read issue title from ' + filename);
109+
return null;
110+
}
111+
return {
112+
type: 'issue',
113+
item: issue,
114+
message: title,
115+
count,
116+
description: processedMarkdown,
117+
};
118+
}
119+
72120
I18n.DevToolsLocale.DevToolsLocale.instance({
73121
create: true,
74122
data: {

‎src/McpResponse.ts‎

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@
33
* Copyright 2025 Google LLC
44
* SPDX-License-Identifier: Apache-2.0
55
*/
6-
import {
7-
AggregatedIssue,
8-
Marked,
9-
MarkdownIssueDescription,
10-
} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
6+
import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
117

8+
import {mapIssueToMessageObject} from './DevtoolsUtils.js';
129
import type {ConsoleMessageData} from './formatters/consoleFormatter.js';
1310
import {
1411
formatConsoleEventShort,
@@ -22,8 +19,6 @@ import {
2219
getStatusFromRequest,
2320
} from './formatters/networkFormatter.js';
2421
import {formatSnapshotNode} from './formatters/snapshotFormatter.js';
25-
import {getIssueDescription} from './issue-descriptions.js';
26-
import {logger} from './logger.js';
2722
import type {McpContext} from './McpContext.js';
2823
import type {
2924
ConsoleMessage,
@@ -256,7 +251,7 @@ export class McpResponse implements Response {
256251
),
257252
};
258253
} else if (message instanceof AggregatedIssue) {
259-
const result = mapIssuesMessage(message);
254+
const result = mapIssueToMessageObject(message);
260255
consoleData = {
261256
consoleMessageStableId,
262257
...result,
@@ -314,7 +309,7 @@ export class McpResponse implements Response {
314309
};
315310
}
316311
if (item instanceof AggregatedIssue) {
317-
const message = mapIssuesMessage(item);
312+
const message = mapIssueToMessageObject(item);
318313
return {
319314
consoleMessageStableId,
320315
...message,
@@ -572,37 +567,3 @@ Call ${handleDialog.name} to handle it before continuing.`);
572567
this.#textResponseLines = [];
573568
}
574569
}
575-
576-
function mapIssuesMessage(
577-
message: AggregatedIssue,
578-
): Omit<ConsoleMessageData, 'consoleMessageStableId'> | null {
579-
const count = message.getAggregatedIssuesCount();
580-
const markdownDescription = message.getDescription();
581-
const filename = markdownDescription?.file;
582-
if (!markdownDescription) {
583-
logger(`no description found for issue:` + message.code);
584-
return null;
585-
}
586-
const rawMarkdown = filename ? getIssueDescription(filename) : null;
587-
if (!rawMarkdown) {
588-
logger(`no markdown ${filename} found for issue:` + message.code);
589-
return null;
590-
}
591-
const processedMarkdown = MarkdownIssueDescription.substitutePlaceholders(
592-
rawMarkdown,
593-
markdownDescription.substitutions,
594-
);
595-
const markdownAst = Marked.Marked.lexer(processedMarkdown);
596-
const title = MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst);
597-
if (!title) {
598-
logger('cannot read issue title from ' + filename);
599-
return null;
600-
}
601-
return {
602-
type: 'issue',
603-
item: message,
604-
message: title,
605-
count,
606-
args: [],
607-
};
608-
}

‎src/formatters/consoleFormatter.ts‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,15 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import {
8-
AggregatedIssue,
9-
MarkdownIssueDescription,
10-
} from '../../node_modules/chrome-devtools-frontend/mcp/mcp.js';
11-
import {ISSUE_UTILS} from '../issue-descriptions.js';
7+
import {AggregatedIssue} from '../../node_modules/chrome-devtools-frontend/mcp/mcp.js';
128

139
export interface ConsoleMessageData {
1410
consoleMessageStableId: number;
1511
type?: string;
1612
item?: unknown;
1713
message?: string;
1814
count?: number;
15+
description?: string;
1916
args?: string[];
2017
}
2118

@@ -43,7 +40,7 @@ export function formatConsoleEventVerbose(msg: ConsoleMessageData): string {
4340
if (msg.item instanceof AggregatedIssue) {
4441
const result = [
4542
`ID: ${msg.consoleMessageStableId}`,
46-
`Message: ${msg.type}> ${formatIssue(msg.item)}`,
43+
`Message: ${msg.type}> ${formatIssue(msg.item, msg.description)}`,
4744
];
4845
return result.join('\n');
4946
}
@@ -76,35 +73,28 @@ function formatArgs(consoleData: ConsoleMessageData): string {
7673
return result.join('\n');
7774
}
7875

79-
export function formatIssue(issue: AggregatedIssue): string {
80-
const markdownDescription = issue.getDescription();
81-
const filename = markdownDescription?.file;
82-
const rawMarkdown = filename
83-
? ISSUE_UTILS.getIssueDescription(filename)
84-
: null;
85-
if (!markdownDescription || !rawMarkdown) {
86-
throw new Error('Error parsing issue description ' + issue.code());
87-
}
88-
let processedMarkdown = MarkdownIssueDescription.substitutePlaceholders(
89-
rawMarkdown,
90-
markdownDescription.substitutions,
91-
);
92-
93-
processedMarkdown = processedMarkdown.trim();
94-
// Remove heading in order not to conflict with the result response markdown
95-
if (processedMarkdown.startsWith('# ')) {
76+
export function formatIssue(
77+
issue: AggregatedIssue,
78+
description?: string,
79+
): string {
80+
const result: string[] = [];
81+
82+
let processedMarkdown = description?.trim();
83+
// Remove heading in order not to conflict with the whole console message response markdown
84+
if (processedMarkdown?.startsWith('# ')) {
9685
processedMarkdown = processedMarkdown.substring(2).trimStart();
9786
}
87+
if (processedMarkdown) result.push(processedMarkdown);
9888

99-
const result: string[] = [processedMarkdown];
100-
const links = markdownDescription.links;
101-
102-
if (links.length > 0) {
89+
const links = issue.getDescription()?.links;
90+
if (links && links.length > 0) {
10391
result.push('Learn more:');
10492
for (const link of links) {
10593
result.push(`[${link.linkTitle}](${link.link})`);
10694
}
10795
}
10896

97+
if (result.length === 0)
98+
return 'No details provided for the issue ' + issue.code();
10999
return result.join('\n');
110100
}

‎tests/DevtoolsUtils.test.ts‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66
import assert from 'node:assert';
7-
import {describe, it} from 'node:test';
7+
import {afterEach, describe, it} from 'node:test';
88

9+
import sinon from 'sinon';
10+
11+
import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
912
import {
1013
extractUrlLikeFromDevToolsTitle,
1114
urlsEqual,
15+
mapIssueToMessageObject,
1216
} from '../src/DevtoolsUtils.js';
17+
import {ISSUE_UTILS} from '../src/issue-descriptions.js';
1318

1419
describe('extractUrlFromDevToolsTitle', () => {
1520
it('deals with no trailing /', () => {
@@ -69,3 +74,104 @@ describe('urlsEqual', () => {
6974
);
7075
});
7176
});
77+
78+
describe('mapIssueToMessageObject', () => {
79+
const mockDescription = {
80+
file: 'mock-issue.md',
81+
substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]),
82+
links: [
83+
{link: 'http://example.com/learnmore', linkTitle: 'Learn more'},
84+
{
85+
link: 'http://example.com/another-learnmore',
86+
linkTitle: 'Learn more 2',
87+
},
88+
],
89+
};
90+
91+
afterEach(() => {
92+
sinon.restore();
93+
});
94+
95+
it('maps aggregated issue with substituted description', () => {
96+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
97+
mockAggregatedIssue.getDescription.returns(mockDescription);
98+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
99+
100+
const getIssueDescriptionStub = sinon.stub(
101+
ISSUE_UTILS,
102+
'getIssueDescription',
103+
);
104+
105+
getIssueDescriptionStub
106+
.withArgs('mock-issue.md')
107+
.returns(
108+
'# Mock Issue Title\n\nThis is a mock issue description with a {PLACEHOLDER_VALUE}.',
109+
);
110+
111+
const result = mapIssueToMessageObject(mockAggregatedIssue);
112+
const expected = {
113+
type: 'issue',
114+
item: mockAggregatedIssue,
115+
message: 'Mock Issue Title',
116+
count: 1,
117+
description:
118+
'# Mock Issue Title\n\nThis is a mock issue description with a substitution value.',
119+
};
120+
assert.deepStrictEqual(result, expected);
121+
});
122+
123+
it('returns null for the issue with no description', () => {
124+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
125+
mockAggregatedIssue.getDescription.returns(null);
126+
127+
const result = mapIssueToMessageObject(mockAggregatedIssue);
128+
assert.equal(result, null);
129+
});
130+
131+
it('returns null if there is no desciption file', () => {
132+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
133+
mockAggregatedIssue.getDescription.returns(mockDescription);
134+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
135+
136+
const getIssueDescriptionStub = sinon.stub(
137+
ISSUE_UTILS,
138+
'getIssueDescription',
139+
);
140+
141+
getIssueDescriptionStub.withArgs('mock-issue.md').returns(null);
142+
const result = mapIssueToMessageObject(mockAggregatedIssue);
143+
assert.equal(result, null);
144+
});
145+
146+
it("returns null if can't parse the title", () => {
147+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
148+
mockAggregatedIssue.getDescription.returns(mockDescription);
149+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
150+
151+
const getIssueDescriptionStub = sinon.stub(
152+
ISSUE_UTILS,
153+
'getIssueDescription',
154+
);
155+
156+
getIssueDescriptionStub
157+
.withArgs('mock-issue.md')
158+
.returns('No title test {PLACEHOLDER_VALUE}');
159+
assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null);
160+
});
161+
162+
it('returns null if devtools utill function throws an error', () => {
163+
const mockAggregatedIssue = sinon.createStubInstance(AggregatedIssue);
164+
mockAggregatedIssue.getDescription.returns(mockDescription);
165+
mockAggregatedIssue.getAggregatedIssuesCount.returns(1);
166+
167+
const getIssueDescriptionStub = sinon.stub(
168+
ISSUE_UTILS,
169+
'getIssueDescription',
170+
);
171+
// An error will be thrown if placeholder doesn't start from PLACEHOLDER_
172+
getIssueDescriptionStub
173+
.withArgs('mock-issue.md')
174+
.returns('No title test {WRONG_PLACEHOLDER}');
175+
assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null);
176+
});
177+
});

‎tests/formatters/consoleFormatter.test.js.snapshot‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ Arg #0: file.txt
3636
`;
3737

3838
exports[`consoleFormatter > formats a console.log message with issue type 1`] = `
39-
Mock Issue Title
39+
ID: 5
40+
Message: issue> Mock Issue Title
4041

41-
This is a mock issue description with a http://example.com/issue-detail.
42+
This is a mock issue description
4243
Learn more:
4344
[Learn more](http://example.com/learnmore)
4445
[Learn more 2](http://example.com/another-learnmore)

0 commit comments

Comments
 (0)