-
Notifications
You must be signed in to change notification settings - Fork 12
Implemented end-to-end coverage reporting for dev evals #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
421035d
6454526
6cf6ae2
e9b227a
441ddb3
1d75460
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import type { Context } from '../types.ts'; | ||
|
|
||
| export function isDevEvaluation(context: Context): boolean { | ||
| return context.type === 'storybook-mcp-dev'; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,117 @@ | ||||||||||
| import * as path from 'node:path'; | ||||||||||
| import * as fs from 'node:fs/promises'; | ||||||||||
| import type { EvaluationSummary } from '../../types'; | ||||||||||
| import type { CoverageFiles, CoverageSummary } from './result-types'; | ||||||||||
| import { createCoverageMap } from 'istanbul-lib-coverage'; | ||||||||||
|
|
||||||||||
| export async function computeCoverage( | ||||||||||
| projectPath: string, | ||||||||||
| resultsPath: string, | ||||||||||
| ): Promise<{ | ||||||||||
| coverage?: EvaluationSummary['coverage']; | ||||||||||
| coverageFiles?: CoverageFiles; | ||||||||||
| }> { | ||||||||||
| let coverage: EvaluationSummary['coverage']; | ||||||||||
| let coverageFiles: CoverageFiles | undefined; | ||||||||||
|
|
||||||||||
| const finalCoveragePath = path.join( | ||||||||||
| projectPath, | ||||||||||
| 'coverage', | ||||||||||
| 'coverage-final.json', | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| try { | ||||||||||
| let normalizedTotal: CoverageSummary | undefined; | ||||||||||
|
|
||||||||||
| const coverageData = JSON.parse( | ||||||||||
| await fs.readFile(finalCoveragePath, 'utf8'), | ||||||||||
| ); | ||||||||||
|
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer the modern dynamic import of JSON files now support in Node, over readFile+parse. It gives nicer errors and provide type safety when the path is static, and then dynamic paths is just about being consistent. There are a bunch of instances of this alraedy in the code base that you can find, but it's something like this, if my memory is correct:
Suggested change
|
||||||||||
|
|
||||||||||
| // Derive from coverage-final using istanbul-lib-coverage | ||||||||||
| const coverageMap = createCoverageMap(coverageData); | ||||||||||
| const summary = coverageMap.getCoverageSummary().toJSON(); | ||||||||||
| const coverageJson = coverageMap.toJSON(); | ||||||||||
|
|
||||||||||
| coverageFiles = {}; | ||||||||||
|
|
||||||||||
| for (const filePath of Object.keys(coverageJson)) { | ||||||||||
| if (filePath === 'total') continue; | ||||||||||
| const fileCoverage = coverageMap.fileCoverageFor(filePath); | ||||||||||
| const fileSummary = fileCoverage.toSummary().toJSON(); | ||||||||||
| let source: string | undefined; | ||||||||||
| try { | ||||||||||
| source = await fs.readFile(filePath, 'utf8'); | ||||||||||
| } catch { | ||||||||||
valentinpalkovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| source = undefined; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| let lineHits: Record<string, number> | undefined; | ||||||||||
| let branchesByLine: | ||||||||||
| | Record<string, { covered: number | null; total: number | null }> | ||||||||||
| | undefined; | ||||||||||
| try { | ||||||||||
| lineHits = fileCoverage.getLineCoverage() as Record<string, number>; | ||||||||||
| const branches = fileCoverage.getBranchCoverageByLine?.(); | ||||||||||
| if (branches && typeof branches === 'object') { | ||||||||||
| branchesByLine = {}; | ||||||||||
| for (const [line, data] of Object.entries( | ||||||||||
| branches as Record<string, any>, | ||||||||||
| )) { | ||||||||||
| branchesByLine[line] = { | ||||||||||
| covered: data.covered ?? null, | ||||||||||
| total: data.total ?? null, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } catch { | ||||||||||
| // ignore | ||||||||||
valentinpalkovic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| coverageFiles[filePath] = { | ||||||||||
| branches: { pct: fileSummary.branches.pct }, | ||||||||||
| functions: { pct: fileSummary.functions.pct }, | ||||||||||
| lines: { pct: fileSummary.lines.pct }, | ||||||||||
| statements: { pct: fileSummary.statements.pct }, | ||||||||||
| lineHits, | ||||||||||
| branchesByLine, | ||||||||||
| source, | ||||||||||
| }; | ||||||||||
| } | ||||||||||
| normalizedTotal = { | ||||||||||
| branches: { pct: summary.branches.pct }, | ||||||||||
| functions: { pct: summary.functions.pct }, | ||||||||||
| lines: { pct: summary.lines.pct }, | ||||||||||
| statements: { pct: summary.statements.pct }, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| if (normalizedTotal) { | ||||||||||
| coverage = { | ||||||||||
| branches: normalizedTotal.branches?.pct ?? null, | ||||||||||
| functions: normalizedTotal.functions?.pct ?? null, | ||||||||||
| lines: normalizedTotal.lines?.pct ?? null, | ||||||||||
| statements: normalizedTotal.statements?.pct ?? null, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| const targetCoveragePath = path.join( | ||||||||||
| resultsPath, | ||||||||||
| 'coverage', | ||||||||||
| 'coverage-summary.json', | ||||||||||
| ); | ||||||||||
| await fs.mkdir(path.dirname(targetCoveragePath), { recursive: true }); | ||||||||||
| await fs.writeFile( | ||||||||||
| targetCoveragePath, | ||||||||||
| JSON.stringify({ total: normalizedTotal }, null, 2), | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| await fs.writeFile( | ||||||||||
| path.join(resultsPath, 'coverage', 'coverage-final.json'), | ||||||||||
| JSON.stringify(coverageFiles ?? {}, null, 2), | ||||||||||
valentinpalkovic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| ); | ||||||||||
| } | ||||||||||
valentinpalkovic marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| } catch { | ||||||||||
valentinpalkovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
| coverage = undefined; | ||||||||||
| coverageFiles = undefined; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return { coverage, coverageFiles }; | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||||||||||||||||||||||||||
| import * as path from 'node:path'; | ||||||||||||||||||||||||||||||
| import * as fs from 'node:fs/promises'; | ||||||||||||||||||||||||||||||
| import type { JsonAssertionResult, JsonTestResults } from 'vitest/reporters'; | ||||||||||||||||||||||||||||||
| import type { A11yViolations, StoryResult, TestSummary } from './result-types'; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| export async function parseTestResults(resultsPath: string): Promise<{ | ||||||||||||||||||||||||||||||
| testSummary: TestSummary; | ||||||||||||||||||||||||||||||
| a11y: A11yViolations; | ||||||||||||||||||||||||||||||
| storyResults: StoryResult[]; | ||||||||||||||||||||||||||||||
| }> { | ||||||||||||||||||||||||||||||
| const testResultsPath = path.join(resultsPath, 'tests.json'); | ||||||||||||||||||||||||||||||
| const { default: jsonTestResults } = (await import(testResultsPath, { | ||||||||||||||||||||||||||||||
| with: { type: 'json' }, | ||||||||||||||||||||||||||||||
| })) as { default: JsonTestResults }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // write the file again to pretty-print it | ||||||||||||||||||||||||||||||
| await fs.writeFile(testResultsPath, JSON.stringify(jsonTestResults, null, 2)); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const a11yViolations: A11yViolations = {}; | ||||||||||||||||||||||||||||||
| const storyAssertions: Record< | ||||||||||||||||||||||||||||||
| string, | ||||||||||||||||||||||||||||||
| { status: JsonAssertionResult['status'] } | ||||||||||||||||||||||||||||||
| > = {}; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const testSuites = jsonTestResults.testResults | ||||||||||||||||||||||||||||||
| ? Object.values(jsonTestResults.testResults) | ||||||||||||||||||||||||||||||
| : []; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| for (const jsonTestResult of testSuites) { | ||||||||||||||||||||||||||||||
| for (const assertionResult of jsonTestResult.assertionResults ?? []) { | ||||||||||||||||||||||||||||||
| const storyId = (assertionResult.meta as any)?.storyId; | ||||||||||||||||||||||||||||||
| if (!storyId) continue; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| storyAssertions[storyId] = { | ||||||||||||||||||||||||||||||
| status: assertionResult.status, | ||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+35
|
||||||||||||||||||||||||||||||
| storyAssertions[storyId] = { | |
| status: assertionResult.status, | |
| // Aggregate statuses: if any assertion fails, mark as failed; otherwise, keep the worst status | |
| const prevStatus = storyAssertions[storyId]?.status; | |
| let newStatus = assertionResult.status; | |
| if (prevStatus) { | |
| // Priority: failed > todo > skipped > passed | |
| const statusPriority = { failed: 3, todo: 2, skipped: 1, passed: 0 }; | |
| const prevPriority = statusPriority[prevStatus] ?? -1; | |
| const newPriority = statusPriority[newStatus] ?? -1; | |
| newStatus = prevPriority > newPriority ? prevStatus : newStatus; | |
| } | |
| storyAssertions[storyId] = { | |
| status: newStatus, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import type { EvaluationSummary } from '../../types'; | ||
|
|
||
| export type TestSummary = Pick<EvaluationSummary['test'], 'passed' | 'failed'>; | ||
|
|
||
| export type StoryResult = { | ||
| storyId: string; | ||
| status: 'passed' | 'failed'; | ||
| }; | ||
|
|
||
| export type A11yViolations = Record<string, any[]>; | ||
|
|
||
| export type CoverageSummary = { | ||
| branches: { pct: number | null }; | ||
| functions: { pct: number | null }; | ||
| lines: { pct: number | null }; | ||
| statements: { pct: number | null }; | ||
| }; | ||
|
|
||
| export type CoverageFiles = Record< | ||
| string, | ||
| { | ||
| lineHits?: Record<string, number>; | ||
| branchesByLine?: Record< | ||
| string, | ||
| { covered: number | null; total: number | null } | ||
| >; | ||
| source?: string; | ||
| } & CoverageSummary | ||
| >; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import * as path from 'node:path'; | ||
| import * as fs from 'node:fs/promises'; | ||
| import { x } from 'tinyexec'; | ||
| import { dedent } from 'ts-dedent'; | ||
| import type { ExperimentArgs } from '../../types'; | ||
|
|
||
| export async function runTests( | ||
| experimentArgs: ExperimentArgs, | ||
| testScript: string, | ||
| ): Promise<number> { | ||
| const { projectPath, resultsPath } = experimentArgs; | ||
| const result = await x('pnpm', [testScript], { | ||
| nodeOptions: { cwd: projectPath }, | ||
| }); | ||
|
|
||
| await fs.writeFile( | ||
| path.join(resultsPath, 'tests.md'), | ||
| dedent`# Test Results | ||
|
|
||
| **Exit Code:** ${result.exitCode} | ||
|
|
||
| ## stdout | ||
|
|
||
| \`\`\`sh | ||
| ${result.stdout} | ||
| \`\`\` | ||
|
|
||
| ## stderr | ||
|
|
||
| \`\`\` | ||
| ${result.stderr} | ||
| \`\`\` | ||
| `, | ||
| ); | ||
|
|
||
| return result.exitCode ?? 0; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.