diff --git a/cli/engine/profile/profiler.mjs b/cli/engine/profile/profiler.mjs index b05fbf3ef..3e8de6266 100644 --- a/cli/engine/profile/profiler.mjs +++ b/cli/engine/profile/profiler.mjs @@ -8,20 +8,24 @@ function createDetectorProfile() { return { events: [] }; } -function recordProfileEvent(profile, event) { - if (!profile) return; - const normalized = { +function normalizeProfileEvent(event) { + return { engine: event.engine || 'unknown', phase: event.phase || 'unknown', ruleId: event.ruleId || 'unknown', target: event.target || '', ms: Number.isFinite(event.ms) ? event.ms : 0, findings: Number.isFinite(event.findings) ? event.findings : 0, + ...(event.detail ? { detail: event.detail } : {}), + ...(Array.isArray(event.findingIds) && event.findingIds.length + ? { findingIds: event.findingIds } + : {}), }; - if (event.detail) normalized.detail = event.detail; - if (Array.isArray(event.findingIds) && event.findingIds.length) { - normalized.findingIds = event.findingIds; - } +} + +function recordProfileEvent(profile, event) { + if (!profile) return; + const normalized = normalizeProfileEvent(event); if (typeof profile === 'function') { profile(normalized); } else if (typeof profile.record === 'function') { @@ -38,10 +42,7 @@ function extractFindingIds(findings) { return [...new Set(findings.map(f => f?.id || f?.type || f?.antipattern).filter(Boolean))]; } -function profileFindings(profile, meta, callback) { - if (!profile) return callback(); - const started = profileNow(); - const findings = callback(); +function recordDuration(profile, meta, started, findings) { recordProfileEvent(profile, { ...meta, ms: profileNow() - started, @@ -51,31 +52,26 @@ function profileFindings(profile, meta, callback) { return findings; } +function profileFindings(profile, meta, callback) { + if (!profile) return callback(); + const started = profileNow(); + return recordDuration(profile, meta, started, callback()); +} + function profileStep(profile, meta, callback) { if (!profile) return callback(); const started = profileNow(); try { return callback(); } finally { - recordProfileEvent(profile, { - ...meta, - ms: profileNow() - started, - findings: 0, - }); + recordDuration(profile, meta, started); } } async function profileFindingsAsync(profile, meta, callback) { if (!profile) return callback(); const started = profileNow(); - const findings = await callback(); - recordProfileEvent(profile, { - ...meta, - ms: profileNow() - started, - findings: Array.isArray(findings) ? findings.length : 0, - findingIds: extractFindingIds(findings), - }); - return findings; + return recordDuration(profile, meta, started, await callback()); } async function profileStepAsync(profile, meta, callback) { @@ -84,11 +80,7 @@ async function profileStepAsync(profile, meta, callback) { try { return await callback(); } finally { - recordProfileEvent(profile, { - ...meta, - ms: profileNow() - started, - findings: 0, - }); + recordDuration(profile, meta, started); } } @@ -107,19 +99,15 @@ function summarizeDetectorProfile(profile) { : (Array.isArray(profile?.events) ? profile.events : []); const groups = new Map(); for (const event of events) { - const key = [ - event.engine || 'unknown', - event.phase || 'unknown', - event.ruleId || 'unknown', - event.target || '', - ].join('\u0000'); + const { engine, phase, ruleId, target, ms, findings } = normalizeProfileEvent(event); + const key = [engine, phase, ruleId, target].join('\u0000'); let group = groups.get(key); if (!group) { group = { - engine: event.engine || 'unknown', - phase: event.phase || 'unknown', - ruleId: event.ruleId || 'unknown', - target: event.target || '', + engine, + phase, + ruleId, + target, calls: 0, totalMs: 0, findings: 0, @@ -127,10 +115,9 @@ function summarizeDetectorProfile(profile) { }; groups.set(key, group); } - const ms = Number.isFinite(event.ms) ? event.ms : 0; group.calls += 1; group.totalMs += ms; - group.findings += Number.isFinite(event.findings) ? event.findings : 0; + group.findings += findings; group.samples.push(ms); } return [...groups.values()] diff --git a/tests/detect-antipatterns.test.js b/tests/detect-antipatterns.test.js index 4f0ae4cb7..1b5144c10 100644 --- a/tests/detect-antipatterns.test.js +++ b/tests/detect-antipatterns.test.js @@ -35,6 +35,15 @@ import { scanHtmlForShapeAssembledIllustration, } from '../cli/engine/rules/checks.mjs'; import { parseGradientColors } from '../cli/engine/shared/color.mjs'; +import { + createDetectorProfile, + profileFindings, + profileFindingsAsync, + profileStep, + profileStepAsync, + recordProfileEvent, + summarizeDetectorProfile, +} from '../cli/engine/profile/profiler.mjs'; const FIXTURES = path.join(import.meta.dir, 'fixtures', 'antipatterns'); const SCRIPT = path.join(import.meta.dir, '..', 'cli', 'engine', 'detect-antipatterns.mjs'); @@ -97,6 +106,53 @@ function pageTypographyForGoogleFonts(href) { return checkPageTypography(doc, win); } +describe('detector profiler', () => { + test('normalizes recorded events before summarizing them', () => { + const profile = createDetectorProfile(); + recordProfileEvent(profile, { + engine: 'regex', + ms: Number.NaN, + findings: Number.POSITIVE_INFINITY, + detail: 'source scan', + findingIds: ['side-tab'], + }); + recordProfileEvent(profile, { phase: 'parse', ms: 2, findings: 1 }); + + expect(profile.events[0]).toEqual({ + engine: 'regex', phase: 'unknown', ruleId: 'unknown', target: '', ms: 0, findings: 0, + detail: 'source scan', findingIds: ['side-tab'], + }); + expect(summarizeDetectorProfile(profile).map(({ phase, totalMs, findings }) => ( + { phase, totalMs, findings } + ))).toEqual([ + { phase: 'parse', totalMs: 2, findings: 1 }, + { phase: 'unknown', totalMs: 0, findings: 0 }, + ]); + }); + + test('preserves sync and async result and error contracts', async () => { + const profile = []; + const findings = [{ id: 'side-tab' }, { type: 'side-tab' }, { antipattern: 'dark-glow' }]; + + expect(profileFindings(profile, { phase: 'sync-findings' }, () => findings)).toBe(findings); + expect(await profileFindingsAsync(profile, { phase: 'async-findings' }, async () => findings)).toBe(findings); + expect(() => profileStep(profile, { phase: 'sync-step' }, () => { + throw new Error('sync failure'); + })).toThrow('sync failure'); + await expect(profileStepAsync(profile, { phase: 'async-step' }, async () => { + throw new Error('async failure'); + })).rejects.toThrow('async failure'); + + expect(profile.map(({ phase, findings: count, findingIds }) => ({ phase, count, findingIds }))).toEqual([ + { phase: 'sync-findings', count: 3, findingIds: ['side-tab', 'dark-glow'] }, + { phase: 'async-findings', count: 3, findingIds: ['side-tab', 'dark-glow'] }, + { phase: 'sync-step', count: 0, findingIds: undefined }, + { phase: 'async-step', count: 0, findingIds: undefined }, + ]); + expect(profile.every(event => Number.isFinite(event.ms) && event.ms >= 0)).toBe(true); + }); +}); + // --------------------------------------------------------------------------- // Core: checkElementBorders (computed style simulation)