Skip to content

Commit 596a300

Browse files
author
Natallia Harshunova
committed
Fix comments and add tests
1 parent 2e7bbf6 commit 596a300

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
@@ -4,12 +4,9 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

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

9+
import {mapIssueToMessageObject} from './DevtoolsUtils.js';
1310
import type {ConsoleMessageData} from './formatters/consoleFormatter.js';
1411
import {
1512
formatConsoleEventShort,
@@ -23,8 +20,6 @@ import {
2320
getStatusFromRequest,
2421
} from './formatters/networkFormatter.js';
2522
import {formatSnapshotNode} from './formatters/snapshotFormatter.js';
26-
import {getIssueDescription} from './issue-descriptions.js';
27-
import {logger} from './logger.js';
2823
import type {McpContext} from './McpContext.js';
2924
import type {
3025
ConsoleMessage,
@@ -257,7 +252,7 @@ export class McpResponse implements Response {
257252
),
258253
};
259254
} else if (message instanceof AggregatedIssue) {
260-
const result = mapIssuesMessage(message);
255+
const result = mapIssueToMessageObject(message);
261256
consoleData = {
262257
consoleMessageStableId,
263258
...result,
@@ -315,7 +310,7 @@ export class McpResponse implements Response {
315310
};
316311
}
317312
if (item instanceof AggregatedIssue) {
318-
const message = mapIssuesMessage(item);
313+
const message = mapIssueToMessageObject(item);
319314
return {
320315
consoleMessageStableId,
321316
...message,
@@ -573,37 +568,3 @@ Call ${handleDialog.name} to handle it before continuing.`);
573568
this.#textResponseLines = [];
574569
}
575570
}
576-
577-
function mapIssuesMessage(
578-
message: AggregatedIssue,
579-
): Omit<ConsoleMessageData, 'consoleMessageStableId'> | null {
580-
const count = message.getAggregatedIssuesCount();
581-
const markdownDescription = message.getDescription();
582-
const filename = markdownDescription?.file;
583-
if (!markdownDescription) {
584-
logger(`no description found for issue:` + message.code);
585-
return null;
586-
}
587-
const rawMarkdown = filename ? getIssueDescription(filename) : null;
588-
if (!rawMarkdown) {
589-
logger(`no markdown ${filename} found for issue:` + message.code);
590-
return null;
591-
}
592-
const processedMarkdown = MarkdownIssueDescription.substitutePlaceholders(
593-
rawMarkdown,
594-
markdownDescription.substitutions,
595-
);
596-
const markdownAst = Marked.Marked.lexer(processedMarkdown);
597-
const title = MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst);
598-
if (!title) {
599-
logger('cannot read issue title from ' + filename);
600-
return null;
601-
}
602-
return {
603-
type: 'issue',
604-
item: message,
605-
message: title,
606-
count,
607-
args: [],
608-
};
609-
}

‎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
@@ -5,12 +5,17 @@
55
*/
66

77
import assert from 'node:assert';
8-
import {describe, it} from 'node:test';
8+
import {afterEach, describe, it} from 'node:test';
99

10+
import sinon from 'sinon';
11+
12+
import {AggregatedIssue} from '../node_modules/chrome-devtools-frontend/mcp/mcp.js';
1013
import {
1114
extractUrlLikeFromDevToolsTitle,
1215
urlsEqual,
16+
mapIssueToMessageObject,
1317
} from '../src/DevtoolsUtils.js';
18+
import {ISSUE_UTILS} from '../src/issue-descriptions.js';
1419

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

‎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)