diff --git a/src/DevtoolsUtils.ts b/src/DevtoolsUtils.ts index 21db7e314..2ffde1997 100644 --- a/src/DevtoolsUtils.ts +++ b/src/DevtoolsUtils.ts @@ -5,8 +5,6 @@ */ import {PuppeteerDevToolsConnection} from './DevToolsConnectionAdapter.js'; -import {ISSUE_UTILS} from './issue-descriptions.js'; -import {logger} from './logger.js'; import {Mutex} from './Mutex.js'; import {DevTools} from './third_party/index.js'; import type { @@ -81,50 +79,6 @@ export class FakeIssuesManager extends DevTools.Common.ObjectWrapper } } -export function mapIssueToMessageObject(issue: DevTools.AggregatedIssue) { - const count = issue.getAggregatedIssuesCount(); - const markdownDescription = issue.getDescription(); - const filename = markdownDescription?.file; - if (!markdownDescription) { - logger(`no description found for issue:` + issue.code); - return null; - } - const rawMarkdown = filename - ? ISSUE_UTILS.getIssueDescription(filename) - : null; - if (!rawMarkdown) { - logger(`no markdown ${filename} found for issue:` + issue.code); - return null; - } - let processedMarkdown: string; - let title: string | null; - - try { - processedMarkdown = - DevTools.MarkdownIssueDescription.substitutePlaceholders( - rawMarkdown, - markdownDescription.substitutions, - ); - const markdownAst = DevTools.Marked.Marked.lexer(processedMarkdown); - title = - DevTools.MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst); - } catch { - logger('error parsing markdown for issue ' + issue.code()); - return null; - } - if (!title) { - logger('cannot read issue title from ' + filename); - return null; - } - return { - type: 'issue', - item: issue, - message: title, - count, - description: processedMarkdown, - }; -} - // DevTools CDP errors can get noisy. DevTools.ProtocolClient.InspectorBackend.test.suppressRequestErrors = true; diff --git a/src/McpResponse.ts b/src/McpResponse.ts index 17a8c098e..82e76851a 100644 --- a/src/McpResponse.ts +++ b/src/McpResponse.ts @@ -4,12 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {mapIssueToMessageObject} from './DevtoolsUtils.js'; import type {ConsoleMessageData} from './formatters/consoleFormatter.js'; import { formatConsoleEventShort, formatConsoleEventVerbose, } from './formatters/consoleFormatter.js'; +import {IssueFormatter} from './formatters/IssueFormatter.js'; import {NetworkFormatter} from './formatters/NetworkFormatter.js'; import {SnapshotFormatter} from './formatters/SnapshotFormatter.js'; import type {McpContext} from './McpContext.js'; @@ -222,7 +222,7 @@ export class McpResponse implements Response { detailedNetworkRequest = formatter; } - let consoleData: ConsoleMessageData | undefined; + let consoleData: ConsoleMessageData | IssueFormatter | undefined; if (this.#attachedConsoleMessageId) { const message = context.getConsoleMessageById( @@ -247,16 +247,17 @@ export class McpResponse implements Response { ), }; } else if (message instanceof DevTools.AggregatedIssue) { - const mappedIssueMessage = mapIssueToMessageObject(message); - if (!mappedIssueMessage) { + const formatter = new IssueFormatter(message, { + id: consoleMessageStableId, + requestIdResolver: context.resolveCdpRequestId.bind(context), + elementIdResolver: context.resolveCdpElementId.bind(context), + }); + if (!formatter.isValid()) { throw new Error( "Can't provide detals for the msgid " + consoleMessageStableId, ); } - consoleData = { - consoleMessageStableId, - ...mappedIssueMessage, - }; + consoleData = formatter; } else { consoleData = { consoleMessageStableId, @@ -267,7 +268,7 @@ export class McpResponse implements Response { } } - let consoleListData: ConsoleMessageData[] | undefined; + let consoleListData: Array | undefined; if (this.#consoleDataOptions?.include) { let messages = context.getConsoleData( this.#consoleDataOptions.includePreservedMessages, @@ -288,44 +289,47 @@ export class McpResponse implements Response { consoleListData = ( await Promise.all( - messages.map(async (item): Promise => { - const consoleMessageStableId = - context.getConsoleMessageStableId(item); - if ('args' in item) { - const consoleMessage = item as ConsoleMessage; - return { - consoleMessageStableId, - type: consoleMessage.type(), - message: consoleMessage.text(), - args: await Promise.all( - consoleMessage.args().map(async arg => { - const stringArg = await arg.jsonValue().catch(() => { - // Ignore errors. - }); - return typeof stringArg === 'object' - ? JSON.stringify(stringArg) - : String(stringArg); - }), - ), - }; - } - if (item instanceof DevTools.AggregatedIssue) { - const mappedIssueMessage = mapIssueToMessageObject(item); - if (!mappedIssueMessage) { - return null; + messages.map( + async ( + item, + ): Promise => { + const consoleMessageStableId = + context.getConsoleMessageStableId(item); + if ('args' in item) { + const consoleMessage = item as ConsoleMessage; + return { + consoleMessageStableId, + type: consoleMessage.type(), + message: consoleMessage.text(), + args: await Promise.all( + consoleMessage.args().map(async arg => { + const stringArg = await arg.jsonValue().catch(() => { + // Ignore errors. + }); + return typeof stringArg === 'object' + ? JSON.stringify(stringArg) + : String(stringArg); + }), + ), + }; + } + if (item instanceof DevTools.AggregatedIssue) { + const formatter = new IssueFormatter(item, { + id: consoleMessageStableId, + }); + if (!formatter.isValid()) { + return null; + } + return formatter; } return { consoleMessageStableId, - ...mappedIssueMessage, + type: 'error', + message: (item as Error).message, + args: [], }; - } - return { - consoleMessageStableId, - type: 'error', - message: (item as Error).message, - args: [], - }; - }), + }, + ), ) ).filter(item => item !== null); } @@ -380,8 +384,8 @@ export class McpResponse implements Response { toolName: string, context: McpContext, data: { - consoleData: ConsoleMessageData | undefined; - consoleListData: ConsoleMessageData[] | undefined; + consoleData: ConsoleMessageData | IssueFormatter | undefined; + consoleListData: Array | undefined; snapshot: SnapshotFormatter | string | undefined; detailedNetworkRequest?: NetworkFormatter; networkRequests?: NetworkFormatter[]; @@ -504,7 +508,12 @@ Call ${handleDialog.name} to handle it before continuing.`); ); response.push(...data.info); response.push( - ...data.items.map(message => formatConsoleEventShort(message)), + ...data.items.map(message => { + if (message instanceof IssueFormatter) { + return message.toString(); + } + return formatConsoleEventShort(message); + }), ); } else { response.push(''); @@ -556,14 +565,18 @@ Call ${handleDialog.name} to handle it before continuing.`); #formatConsoleData( context: McpContext, - data: ConsoleMessageData | undefined, + data: ConsoleMessageData | IssueFormatter | undefined, ): string[] { const response: string[] = []; if (!data) { return response; } - response.push(formatConsoleEventVerbose(data, context)); + if (data instanceof IssueFormatter) { + response.push(data.toStringDetailed()); + } else { + response.push(formatConsoleEventVerbose(data, context)); + } return response; } diff --git a/src/formatters/IssueFormatter.ts b/src/formatters/IssueFormatter.ts new file mode 100644 index 000000000..418986a36 --- /dev/null +++ b/src/formatters/IssueFormatter.ts @@ -0,0 +1,219 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ISSUE_UTILS} from '../issue-descriptions.js'; +import {logger} from '../logger.js'; +import {DevTools} from '../third_party/index.js'; + +export interface IssueFormatterOptions { + requestIdResolver?: (requestId: string) => number | undefined; + elementIdResolver?: (backendNodeId: number) => string | undefined; + id?: number; +} + +export class IssueFormatter { + #issue: DevTools.AggregatedIssue; + #options: IssueFormatterOptions; + + constructor(issue: DevTools.AggregatedIssue, options: IssueFormatterOptions) { + this.#issue = issue; + this.#options = options; + } + + toString(): string { + const title = this.#getTitle(); + const count = this.#issue.getAggregatedIssuesCount(); + const idPart = + this.#options.id !== undefined ? `msgid=${this.#options.id} ` : ''; + return `${idPart}[issue] ${title} (count: ${count})`; + } + + toStringDetailed(): string { + const result: string[] = []; + if (this.#options.id !== undefined) { + result.push(`ID: ${this.#options.id}`); + } + + const bodyParts: string[] = []; + + const description = this.#getDescription(); + let processedMarkdown = description?.trim(); + // Remove heading in order not to conflict with the whole console message response markdown + if (processedMarkdown?.startsWith('# ')) { + processedMarkdown = processedMarkdown.substring(2).trimStart(); + } + if (processedMarkdown) { + bodyParts.push(processedMarkdown); + } else { + bodyParts.push(this.#getTitle() ?? 'Unknown Issue'); + } + + const links = this.#issue.getDescription()?.links; + if (links && links.length > 0) { + bodyParts.push('Learn more:'); + for (const link of links) { + bodyParts.push(`[${link.linkTitle}](${link.link})`); + } + } + + const issues = this.#issue.getAllIssues(); + const affectedResources: Array<{ + uid?: string; + data?: object; + request?: string | number; + }> = []; + for (const singleIssue of issues) { + const details = singleIssue.details(); + if (!details) { + continue; + } + + // We send the remaining details as untyped JSON because the DevTools + // frontend code is currently not re-usable. + // eslint-disable-next-line + const data = structuredClone(details) as any; + + let uid; + let request: number | string | undefined; + if ( + 'violatingNodeId' in details && + details.violatingNodeId && + this.#options.elementIdResolver + ) { + uid = this.#options.elementIdResolver(details.violatingNodeId); + delete data.violatingNodeId; + } + if ( + 'nodeId' in details && + details.nodeId && + this.#options.elementIdResolver + ) { + uid = this.#options.elementIdResolver(details.nodeId); + delete data.nodeId; + } + if ( + 'documentNodeId' in details && + details.documentNodeId && + this.#options.elementIdResolver + ) { + uid = this.#options.elementIdResolver(details.documentNodeId); + delete data.documentNodeId; + } + + if ('request' in details && details.request) { + request = details.request.url; + if (details.request.requestId && this.#options.requestIdResolver) { + const resolvedId = this.#options.requestIdResolver( + details.request.requestId, + ); + if (resolvedId) { + request = resolvedId; + delete data.request.requestId; + } + } + } + + // These fields has no use for the MCP client (redundant or irrelevant). + delete data.errorType; + delete data.frameId; + affectedResources.push({ + uid, + data: data, + request, + }); + } + if (affectedResources.length) { + bodyParts.push('### Affected resources'); + } + bodyParts.push( + ...affectedResources.map(item => { + const details = []; + if (item.uid) { + details.push(`uid=${item.uid}`); + } + if (item.request) { + details.push( + (typeof item.request === 'number' ? `reqid=` : 'url=') + + item.request, + ); + } + if (item.data) { + details.push(`data=${JSON.stringify(item.data)}`); + } + return details.join(' '); + }), + ); + + result.push(`Message: issue> ${bodyParts.join('\n')}`); + + return result.join('\n'); + } + + isValid(): boolean { + return this.#getTitle() !== undefined; + } + + // Helper to extract title + #getTitle(): string | undefined { + const markdownDescription = this.#issue.getDescription(); + const filename = markdownDescription?.file; + if (!filename) { + logger(`no description found for issue:` + this.#issue.code()); + return undefined; + } + + // We already have the description logic in #getDescription, but title extraction is separate + // We can reuse the logic or cache it. + // Ideally we should process markdown once. + + const rawMarkdown = ISSUE_UTILS.getIssueDescription(filename); + if (!rawMarkdown) { + logger(`no markdown ${filename} found for issue:` + this.#issue.code()); + return undefined; + } + + try { + const processedMarkdown = + DevTools.MarkdownIssueDescription.substitutePlaceholders( + rawMarkdown, + markdownDescription?.substitutions, + ); + const markdownAst = DevTools.Marked.Marked.lexer(processedMarkdown); + const title = + DevTools.MarkdownIssueDescription.findTitleFromMarkdownAst(markdownAst); + if (!title) { + logger('cannot read issue title from ' + filename); + return undefined; + } + return title; + } catch { + logger('error parsing markdown for issue ' + this.#issue.code()); + return undefined; + } + } + + #getDescription(): string | undefined { + const markdownDescription = this.#issue.getDescription(); + const filename = markdownDescription?.file; + if (!filename) { + return undefined; + } + + const rawMarkdown = ISSUE_UTILS.getIssueDescription(filename); + if (!rawMarkdown) { + return undefined; + } + + try { + return DevTools.MarkdownIssueDescription.substitutePlaceholders( + rawMarkdown, + markdownDescription?.substitutions, + ); + } catch { + return undefined; + } + } +} diff --git a/src/formatters/consoleFormatter.ts b/src/formatters/consoleFormatter.ts index fdfa8a361..89548aeb1 100644 --- a/src/formatters/consoleFormatter.ts +++ b/src/formatters/consoleFormatter.ts @@ -7,6 +7,8 @@ import type {McpContext} from '../McpContext.js'; import type {DevTools} from '../third_party/index.js'; +import {IssueFormatter} from './IssueFormatter.js'; + export interface ConsoleMessageData { consoleMessageStableId: number; type?: string; @@ -20,8 +22,11 @@ export interface ConsoleMessageData { // The short format for a console message, based on a previous format. export function formatConsoleEventShort(msg: ConsoleMessageData): string { - if (msg.type === 'issue') { - return `msgid=${msg.consoleMessageStableId} [${msg.type}] ${msg.message} (count: ${msg.count})`; + if (msg.type === 'issue' && msg.item) { + const formatter = new IssueFormatter(msg.item, { + id: msg.consoleMessageStableId, + }); + return formatter.toString(); } return `msgid=${msg.consoleMessageStableId} [${msg.type}] ${msg.message} (${msg.args?.length ?? 0} args)`; } @@ -43,10 +48,22 @@ export function formatConsoleEventVerbose( context?: McpContext, ): string { const aggregatedIssue = msg.item; + if (aggregatedIssue) { + return new IssueFormatter(aggregatedIssue, { + id: msg.consoleMessageStableId, + requestIdResolver: context + ? context.resolveCdpRequestId.bind(context) + : undefined, + elementIdResolver: context + ? context.resolveCdpElementId.bind(context) + : undefined, + }).toStringDetailed(); + } + const result = [ `ID: ${msg.consoleMessageStableId}`, - `Message: ${msg.type}> ${aggregatedIssue ? formatIssue(aggregatedIssue, msg.description, context) : msg.message}`, - aggregatedIssue ? undefined : formatArgs(msg), + `Message: ${msg.type}> ${msg.message}`, + formatArgs(msg), formatStackTrace(msg.stackTrace), ].filter(line => !!line); return result.join('\n'); @@ -72,110 +89,6 @@ function formatArgs(consoleData: ConsoleMessageData): string { return result.join('\n'); } -export function formatIssue( - issue: DevTools.AggregatedIssue, - description?: string, - context?: McpContext, -): string { - const result: string[] = []; - - let processedMarkdown = description?.trim(); - // Remove heading in order not to conflict with the whole console message response markdown - if (processedMarkdown?.startsWith('# ')) { - processedMarkdown = processedMarkdown.substring(2).trimStart(); - } - if (processedMarkdown) { - result.push(processedMarkdown); - } - - const links = issue.getDescription()?.links; - if (links && links.length > 0) { - result.push('Learn more:'); - for (const link of links) { - result.push(`[${link.linkTitle}](${link.link})`); - } - } - - const issues = issue.getAllIssues(); - const affectedResources: Array<{ - uid?: string; - data?: object; - request?: string | number; - }> = []; - for (const singleIssue of issues) { - const details = singleIssue.details(); - if (!details) { - continue; - } - - // We send the remaining details as untyped JSON because the DevTools - // frontend code is currently not re-usable. - // eslint-disable-next-line - const data = structuredClone(details) as any; - - let uid; - let request: number | string | undefined; - if ('violatingNodeId' in details && details.violatingNodeId && context) { - uid = context.resolveCdpElementId(details.violatingNodeId); - delete data.violatingNodeId; - } - if ('nodeId' in details && details.nodeId && context) { - uid = context.resolveCdpElementId(details.nodeId); - delete data.nodeId; - } - if ('documentNodeId' in details && details.documentNodeId && context) { - uid = context.resolveCdpElementId(details.documentNodeId); - delete data.documentNodeId; - } - - if ('request' in details && details.request) { - request = details.request.url; - if (details.request.requestId && context) { - const resolvedId = context.resolveCdpRequestId( - details.request.requestId, - ); - if (resolvedId) { - request = resolvedId; - delete data.request.requestId; - } - } - } - - // These fields has no use for the MCP client (redundant or irrelevant). - delete data.errorType; - delete data.frameId; - affectedResources.push({ - uid, - data: data, - request, - }); - } - if (affectedResources.length) { - result.push('### Affected resources'); - } - result.push( - ...affectedResources.map(item => { - const details = []; - if (item.uid) { - details.push(`uid=${item.uid}`); - } - if (item.request) { - details.push( - (typeof item.request === 'number' ? `reqid=` : 'url=') + item.request, - ); - } - if (item.data) { - details.push(`data=${JSON.stringify(item.data)}`); - } - return details.join(' '); - }), - ); - if (result.length === 0) { - return 'No affected resources found'; - } - return result.join('\n'); -} - function formatStackTrace( stackTrace: DevTools.StackTrace.StackTrace.StackTrace | undefined, ): string { diff --git a/tests/DevtoolsUtils.test.ts b/tests/DevtoolsUtils.test.ts index 24ac7ffb0..a6f5d113c 100644 --- a/tests/DevtoolsUtils.test.ts +++ b/tests/DevtoolsUtils.test.ts @@ -5,17 +5,15 @@ */ import assert from 'node:assert'; -import {afterEach, describe, it} from 'node:test'; +import {describe, it} from 'node:test'; import sinon from 'sinon'; import { extractUrlLikeFromDevToolsTitle, urlsEqual, - mapIssueToMessageObject, UniverseManager, } from '../src/DevtoolsUtils.js'; -import {ISSUE_UTILS} from '../src/issue-descriptions.js'; import {DevTools} from '../src/third_party/index.js'; import type {Browser, Target} from '../src/third_party/index.js'; @@ -96,117 +94,6 @@ describe('urlsEqual', () => { }); }); -describe('mapIssueToMessageObject', () => { - const mockDescription = { - file: 'mock-issue.md', - substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]), - links: [ - {link: 'http://example.com/learnmore', linkTitle: 'Learn more'}, - { - link: 'http://example.com/another-learnmore', - linkTitle: 'Learn more 2', - }, - ], - }; - - afterEach(() => { - sinon.restore(); - }); - - it('maps aggregated issue with substituted description', () => { - const mockAggregatedIssue = sinon.createStubInstance( - DevTools.AggregatedIssue, - ); - mockAggregatedIssue.getDescription.returns(mockDescription); - mockAggregatedIssue.getAggregatedIssuesCount.returns(1); - - const getIssueDescriptionStub = sinon.stub( - ISSUE_UTILS, - 'getIssueDescription', - ); - - getIssueDescriptionStub - .withArgs('mock-issue.md') - .returns( - '# Mock Issue Title\n\nThis is a mock issue description with a {PLACEHOLDER_VALUE}.', - ); - - const result = mapIssueToMessageObject(mockAggregatedIssue); - const expected = { - type: 'issue', - item: mockAggregatedIssue, - message: 'Mock Issue Title', - count: 1, - description: - '# Mock Issue Title\n\nThis is a mock issue description with a substitution value.', - }; - assert.deepStrictEqual(result, expected); - }); - - it('returns null for the issue with no description', () => { - const mockAggregatedIssue = sinon.createStubInstance( - DevTools.AggregatedIssue, - ); - mockAggregatedIssue.getDescription.returns(null); - - const result = mapIssueToMessageObject(mockAggregatedIssue); - assert.equal(result, null); - }); - - it('returns null if there is no desciption file', () => { - const mockAggregatedIssue = sinon.createStubInstance( - DevTools.AggregatedIssue, - ); - mockAggregatedIssue.getDescription.returns(mockDescription); - mockAggregatedIssue.getAggregatedIssuesCount.returns(1); - - const getIssueDescriptionStub = sinon.stub( - ISSUE_UTILS, - 'getIssueDescription', - ); - - getIssueDescriptionStub.withArgs('mock-issue.md').returns(null); - const result = mapIssueToMessageObject(mockAggregatedIssue); - assert.equal(result, null); - }); - - it("returns null if can't parse the title", () => { - const mockAggregatedIssue = sinon.createStubInstance( - DevTools.AggregatedIssue, - ); - mockAggregatedIssue.getDescription.returns(mockDescription); - mockAggregatedIssue.getAggregatedIssuesCount.returns(1); - - const getIssueDescriptionStub = sinon.stub( - ISSUE_UTILS, - 'getIssueDescription', - ); - - getIssueDescriptionStub - .withArgs('mock-issue.md') - .returns('No title test {PLACEHOLDER_VALUE}'); - assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null); - }); - - it('returns null if devtools utill function throws an error', () => { - const mockAggregatedIssue = sinon.createStubInstance( - DevTools.AggregatedIssue, - ); - mockAggregatedIssue.getDescription.returns(mockDescription); - mockAggregatedIssue.getAggregatedIssuesCount.returns(1); - - const getIssueDescriptionStub = sinon.stub( - ISSUE_UTILS, - 'getIssueDescription', - ); - // An error will be thrown if placeholder doesn't start from PLACEHOLDER_ - getIssueDescriptionStub - .withArgs('mock-issue.md') - .returns('No title test {WRONG_PLACEHOLDER}'); - assert.deepStrictEqual(mapIssueToMessageObject(mockAggregatedIssue), null); - }); -}); - describe('UniverseManager', () => { it('calls the factory for existing pages', async () => { const browser = getMockBrowser(); diff --git a/tests/formatters/IssueFormatter.test.js.snapshot b/tests/formatters/IssueFormatter.test.js.snapshot new file mode 100644 index 000000000..10d939e22 --- /dev/null +++ b/tests/formatters/IssueFormatter.test.js.snapshot @@ -0,0 +1,9 @@ +exports[`IssueFormatter > formats an issue message 1`] = ` +ID: 5 +Message: issue> Mock Issue Title + +This is a mock issue description +Learn more: +[Learn more](http://example.com/learnmore) +[Learn more 2](http://example.com/another-learnmore) +`; diff --git a/tests/formatters/IssueFormatter.test.ts b/tests/formatters/IssueFormatter.test.ts new file mode 100644 index 000000000..499d34078 --- /dev/null +++ b/tests/formatters/IssueFormatter.test.ts @@ -0,0 +1,137 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert'; +import {describe, it, beforeEach, afterEach} from 'node:test'; + +import sinon from 'sinon'; + +import {IssueFormatter} from '../../src/formatters/IssueFormatter.js'; +import {ISSUE_UTILS} from '../../src/issue-descriptions.js'; +import {getMockAggregatedIssue} from '../utils.js'; + +describe('IssueFormatter', () => { + let getIssueDescriptionStub: sinon.SinonStub; + + beforeEach(() => { + getIssueDescriptionStub = sinon.stub(ISSUE_UTILS, 'getIssueDescription'); + }); + + afterEach(() => { + getIssueDescriptionStub.restore(); + }); + + it('formats an issue message', t => { + const testGenericIssue = { + details: () => { + return { + violatingNodeId: 2, + violatingNodeAttribute: 'test', + }; + }, + }; + const mockAggregatedIssue = getMockAggregatedIssue(); + const mockDescription = { + file: 'mock.md', + links: [ + {link: 'http://example.com/learnmore', linkTitle: 'Learn more'}, + { + link: 'http://example.com/another-learnmore', + linkTitle: 'Learn more 2', + }, + ], + }; + mockAggregatedIssue.getDescription.returns(mockDescription); + // @ts-expect-error generic issue stub bypass + mockAggregatedIssue.getGenericIssues.returns(new Set([testGenericIssue])); + + const mockDescriptionFileContent = + '# Mock Issue Title\n\nThis is a mock issue description'; + + getIssueDescriptionStub + .withArgs('mock.md') + .returns(mockDescriptionFileContent); + + const formatter = new IssueFormatter(mockAggregatedIssue, { + id: 5, + }); + + const result = formatter.toStringDetailed(); + t.assert.snapshot?.(result); + }); + + describe('isValid', () => { + it('returns false for the issue with no description', () => { + const mockAggregatedIssue = getMockAggregatedIssue(); + mockAggregatedIssue.getDescription.returns(null); + + const formatter = new IssueFormatter(mockAggregatedIssue, {id: 1}); + assert.strictEqual(formatter.isValid(), false); + }); + + it('returns false if there is no description file', () => { + const mockAggregatedIssue = getMockAggregatedIssue(); + mockAggregatedIssue.getDescription.returns({ + file: 'mock.md', + links: [], + }); + getIssueDescriptionStub.withArgs('mock.md').returns(null); + + const formatter = new IssueFormatter(mockAggregatedIssue, {id: 1}); + assert.strictEqual(formatter.isValid(), false); + }); + + it("returns false if can't parse the title", () => { + const mockAggregatedIssue = getMockAggregatedIssue(); + mockAggregatedIssue.getDescription.returns({ + file: 'mock.md', + links: [], + }); + getIssueDescriptionStub + .withArgs('mock.md') + .returns('No title test {PLACEHOLDER_VALUE}'); + + const formatter = new IssueFormatter(mockAggregatedIssue, {id: 1}); + assert.strictEqual(formatter.isValid(), false); + }); + + it('returns false if devtools util function throws an error', () => { + const mockAggregatedIssue = getMockAggregatedIssue(); + mockAggregatedIssue.getDescription.returns({ + file: 'mock.md', + links: [], + substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]), + }); + + getIssueDescriptionStub + .withArgs('mock.md') + .returns('No title test {WRONG_PLACEHOLDER}'); + + const formatter = new IssueFormatter(mockAggregatedIssue, {id: 1}); + assert.strictEqual(formatter.isValid(), false); + }); + + it('returns true for valid issue', () => { + const mockAggregatedIssue = getMockAggregatedIssue(); + mockAggregatedIssue.getDescription.returns({ + file: 'mock.md', + links: [], + substitutions: new Map([['PLACEHOLDER_VALUE', 'substitution value']]), + }); + getIssueDescriptionStub + .withArgs('mock.md') + .returns('# Valid Title\n\nContent {PLACEHOLDER_VALUE}'); + + const formatter = new IssueFormatter(mockAggregatedIssue, {id: 1}); + assert.strictEqual(formatter.isValid(), true); + + // Verify usage of substitutions in detailed output + const detailed = formatter.toStringDetailed(); + assert.ok(detailed.includes('substitution value')); + assert.ok(detailed.includes('Valid Title')); + }); + }); +}); diff --git a/tests/formatters/consoleFormatter.test.ts b/tests/formatters/consoleFormatter.test.ts index 4c7e69b71..26ced09d7 100644 --- a/tests/formatters/consoleFormatter.test.ts +++ b/tests/formatters/consoleFormatter.test.ts @@ -12,7 +12,6 @@ import { formatConsoleEventVerbose, } from '../../src/formatters/consoleFormatter.js'; import type {DevTools} from '../../src/third_party/index.js'; -import {getMockAggregatedIssue} from '../utils.js'; describe('consoleFormatter', () => { describe('formatConsoleEventShort', () => { @@ -136,41 +135,4 @@ describe('consoleFormatter', () => { t.assert.snapshot?.(result); }); }); - - it('formats a console.log message with issue type', t => { - const testGenericIssue = { - details: () => { - return { - violatingNodeId: 2, - violatingNodeAttribute: 'test', - }; - }, - }; - const mockAggregatedIssue = getMockAggregatedIssue(); - const mockDescription = { - file: 'mock.md', - links: [ - {link: 'http://example.com/learnmore', linkTitle: 'Learn more'}, - { - link: 'http://example.com/another-learnmore', - linkTitle: 'Learn more 2', - }, - ], - }; - mockAggregatedIssue.getDescription.returns(mockDescription); - // @ts-expect-error generic issue stub bypass - mockAggregatedIssue.getGenericIssues.returns(new Set([testGenericIssue])); - const mockDescriptionFileContent = - '# Mock Issue Title\n\nThis is a mock issue description'; - - const message: ConsoleMessageData = { - consoleMessageStableId: 5, - type: 'issue', - description: mockDescriptionFileContent, - item: mockAggregatedIssue, - }; - - const result = formatConsoleEventVerbose(message); - t.assert.snapshot?.(result); - }); });