From 27eb07b4ba30d4144a5ddee9a753aa93fb69df18 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Tue, 15 Sep 2026 17:41:45 -0700 Subject: [PATCH] fix: preserve partial benchmark failure outcomes --- .github/workflows/benchmark.yaml | 13 +- .github/workflows/test.yml | 24 +++ README.md | 31 +++- app/package.json | 1 + app/src/components/history-chart.tsx | 4 + app/src/components/ui/chart.tsx | 10 ++ app/src/components/variation/chart.tsx | 40 ++++- app/src/components/variation/table.tsx | 14 ++ app/src/hooks/use-history-data.ts | 47 +++++- app/src/lib/run-completeness.ts | 13 ++ app/src/lib/utils.ts | 26 ++++ app/src/types/chart-data.ts | 18 ++- app/tests/partial-results.test.js | 101 +++++++++++++ scripts/clean-benchmarks.js | 39 ++++- scripts/generate-chart.js | 23 +++ scripts/partial-results.test.js | 193 +++++++++++++++++++++++++ 16 files changed, 570 insertions(+), 27 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 app/src/lib/run-completeness.ts create mode 100644 app/tests/partial-results.test.js create mode 100644 scripts/partial-results.test.js diff --git a/.github/workflows/benchmark.yaml b/.github/workflows/benchmark.yaml index 04b5afe803..dbee3ba545 100644 --- a/.github/workflows/benchmark.yaml +++ b/.github/workflows/benchmark.yaml @@ -240,8 +240,8 @@ jobs: id: registry-raw # Any non-zero vlt exit code is a potential outage or misconfiguration. # This must run before `bench process`, which rewrites exit codes in - # place (dropping failed runs), so raw per-run failures are only - # visible here. + # place. original_exit_codes also preserves those outcomes in the + # processed output, but validation reads the raw export here. if: github.ref == 'refs/heads/main' && needs.detect-changes.outputs.partial_run != 'true' uses: actions/github-script@v9 with: @@ -335,12 +335,11 @@ jobs: if (!match) continue const label = `${match[1]} ${match[2]}` - // Registries where every run failed are stamped with a non-zero - // exit code and a zero mean by clean-benchmarks and rendered as - // failures on the site, so they are excluded from the speed - // comparison. A failed vlt is already reported by the raw scan. + // Compare only complete results. Partial survivor statistics do + // not establish a fair speed comparison. Failures are already + // reported by the raw scan before processing. const ok = (JSON.parse(fs.readFileSync(path.join(dir, entry), 'utf8')).results ?? []) - .filter((r) => r.mean > 0 && (r.exit_codes ?? []).every((c) => c === 0)) + .filter((r) => r.mean > 0 && r.status !== 'partial' && !(r.dropped_runs > 0) && (r.exit_codes ?? []).every((c) => c === 0)) const vlt = ok.find((r) => r.command === 'vlt') if (!vlt) continue diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000000..52c14048a0 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,24 @@ +name: Data and App Tests +on: + pull_request: + push: + branches: [main] +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v6 + with: + node-version: "24" + package-manager-cache: false + - run: npm install --ignore-scripts --no-package-lock + working-directory: app + - run: node --test scripts/*.test.js + - name: Test app data semantics + run: | + if [ -d app/tests ]; then node --test app/tests/*.test.*; fi + - run: npm run build + working-directory: app diff --git a/README.md b/README.md index b4dc5610e0..b4c9d24826 100644 --- a/README.md +++ b/README.md @@ -176,7 +176,7 @@ This suite also tests the performance of basic script execution (ex. `npm run fo - 4. Process benchmark output: ```bash - # Filter failed runs and generate dated + latest outputs and chart data + # Record run outcomes, filter failed timings, and generate dated + latest output ./bench process # Or pass --process directly when running ./bench run --fixtures=next --runs=3 --process @@ -199,6 +199,35 @@ The workflow: ## Results +### Failed and partial runs + +Processed benchmark results retain `original_exit_codes`, `attempted_runs`, +`successful_runs`, `dropped_runs`, and a `status` of `success`, `partial`, or +`failure`. These counts refer to measured runs, excluding Hyperfine warmups. +Cleaning the same output again preserves the original counts and exit codes. +The existing `times` and `exit_codes` arrays contain successful runs; total +failure keeps the existing zero-time DNF sentinel for compatibility. + +Partial timing statistics describe the successful runs only. A single survivor +has `stddev: null` because it cannot establish variability. CPU `user` and +`system` fields are omitted whenever runs were dropped: Hyperfine exports only +aggregate CPU values, which cannot be recalculated for the surviving sample. + +Chart data adds optional `_partial`, `_attempted_runs`, +`_successful_runs`, and `_dropped_runs` fields in both total and +per-package datasets. Tables and chart notices label partial results with their +success counts. Synthetic averages retain that warning. Commands with any +partial result in the selected comparisons are excluded from rankings; history +omits that command's affected daily variation and category average. + +This is an additive schema change. Historical JSON without these fields remains +readable, but its completeness is unknown: failed attempts discarded by older +processing cannot be reconstructed. Regenerate from raw benchmark artifacts to +recover counts when those artifacts are still available. CI checks that inspect +raw `exit_codes` must run before `./bench process`; the registry failure scan +already does so. Persisted original codes also remain available in dated result +files on gh-pages. + ### Console Output Each benchmark run provides a summary in the console: diff --git a/app/package.json b/app/package.json index 1d82d51026..65e2da43af 100644 --- a/app/package.json +++ b/app/package.json @@ -4,6 +4,7 @@ "version": "0.0.0", "type": "module", "scripts": { + "test": "node --test tests/*.test.js", "dev": "vite", "format": "prettier --write . --log-level warn --ignore-path ./.prettierignore --cache", "build": "tsc -b && vite build", diff --git a/app/src/components/history-chart.tsx b/app/src/components/history-chart.tsx index 305f5c8f42..1e462433b1 100644 --- a/app/src/components/history-chart.tsx +++ b/app/src/components/history-chart.tsx @@ -237,6 +237,10 @@ export const HistoryChart = ({
+

+ Days with partial results are omitted for that command and variation. + Older results without run counts have unknown completeness. +

{itemConfig?.label || item.name} + {partial && ( + + {partial} + + )}
{(item.value !== undefined || isDnf) && ( diff --git a/app/src/components/variation/chart.tsx b/app/src/components/variation/chart.tsx index 552f18ac54..e57a12c486 100644 --- a/app/src/components/variation/chart.tsx +++ b/app/src/components/variation/chart.tsx @@ -33,6 +33,7 @@ import { CHART_DEFAULTS } from "@/constants"; import { formatPackageManagerLabel, getFixtureId } from "@/lib/utils"; import { getFrameworkIcon } from "@/lib/get-icons"; import { useMediaQuery } from "@/hooks/use-media-query"; +import { partialResultLabel } from "@/lib/run-completeness"; import type { ChartConfig } from "@/components/ui/chart"; import type { @@ -126,6 +127,7 @@ const HorizontalBarTooltipContent = ({ item.color; const isBaseline = item.payload?.isBaseline === true; + const partial = partialResultLabel(item.payload); return (
@@ -140,6 +142,11 @@ const HorizontalBarTooltipContent = ({
{item.name ?? item.dataKey} + {partial && ( + + {partial} + + )} {isBaseline && ( baseline @@ -323,6 +330,24 @@ export const VariationChart = ({ // Variation data already respects fixture filters upstream const filteredVariationData = variationData; + const partialResults = filteredVariationData.flatMap((row) => + filteredPackageManagers.flatMap((pm) => { + const label = partialResultLabel({ ...row }, pm); + return label ? [`${row.fixture} / ${pm}: ${label}`] : []; + }), + ); + const partialNotice = partialResults.length > 0 && ( +
+

+ Partial results include successful runs only and may appear faster + because failed or timed-out runs were dropped. Commands with partial + results are excluded from rankings and history averages. +

+
    + {partialResults.map((label) =>
  • {label}
  • )} +
+
+ ); const variationActivePackageManagers = useMemo(() => { const active = new Set(); @@ -372,7 +397,7 @@ export const VariationChart = ({ // Always compute both data structures to avoid conditional hook calls const consolidatedData = useMemo(() => { return filteredVariationData.map((item): ConsolidatedChartItem => { - const chartItem: ConsolidatedChartItem = { fixture: item.fixture }; + const chartItem: ConsolidatedChartItem = { ...item, fixture: item.fixture }; const slowest = fixtureSlowestValues.get(item.fixture); filteredPackageManagers.forEach((pm) => { @@ -430,6 +455,7 @@ export const VariationChart = ({ // Task runners & registries: horizontal bar charts per fixture, sorted by speed return (
+ {partialNotice}

@@ -491,6 +517,9 @@ export const VariationChart = ({ ? getDnfPatternFill(fixtureId, pm) : getColor(pm), dnf: isDnf, + partial: fixtureResult[`${pm}_partial`] === true, + attempted_runs: fixtureResult[`${pm}_attempted_runs`], + successful_runs: fixtureResult[`${pm}_successful_runs`], dnfColor: getColor(pm), isBaseline: isBaselinePackageManager(pm, isRegistry), }; @@ -684,6 +713,7 @@ export const VariationChart = ({ if (isMobile) { return (
+ {partialNotice} {consolidatedHeader} {/* Legend */} @@ -752,6 +782,9 @@ export const VariationChart = ({ ? getDnfPatternFill(fixtureId, pm) : getColor(pm), dnf: isDnf, + partial: fixtureResult[`${pm}_partial`] === true, + attempted_runs: fixtureResult[`${pm}_attempted_runs`], + successful_runs: fixtureResult[`${pm}_successful_runs`], dnfColor: getColor(pm), pm, }; @@ -836,6 +869,7 @@ export const VariationChart = ({ // DESKTOP: Grouped bar chart (existing layout) return (
+ {partialNotice} {consolidatedHeader}
@@ -956,6 +990,7 @@ export const VariationChart = ({ // Uses horizontal bar charts sorted fastest-to-slowest for all screen sizes return (
+ {partialNotice}

@@ -1000,6 +1035,9 @@ export const VariationChart = ({ value: resolvedValue, fill: isDnf ? getDnfPatternFill(fixtureId, pm) : fillColor, dnf: isDnf, + partial: fixtureResult[`${pm}_partial`] === true, + attempted_runs: fixtureResult[`${pm}_attempted_runs`], + successful_runs: fixtureResult[`${pm}_successful_runs`], dnfColor: getColor(pm), }; }) diff --git a/app/src/components/variation/table.tsx b/app/src/components/variation/table.tsx index 9612bdfb76..b32222607a 100644 --- a/app/src/components/variation/table.tsx +++ b/app/src/components/variation/table.tsx @@ -28,6 +28,7 @@ import { import { ShareButton } from "@/components/share-button"; import { usePackageManagerFilter } from "@/contexts/package-manager-filter-context"; import { Clock, StopWatch } from "@/components/icons"; +import { partialResultLabel } from "@/lib/run-completeness"; import type { BenchmarkChartData, @@ -53,6 +54,7 @@ interface TransposedVariationRow { packageManager: PackageManager; fixtureValues: Partial>; fixtureDnf: Partial>; + fixturePartial: Partial>; } const columnHelper = createColumnHelper(); @@ -91,12 +93,17 @@ export const VariationTable = ({ filteredPackageManagers.map((packageManager) => { const fixtureValues: Partial> = {}; const fixtureDnf: Partial> = {}; + const fixturePartial: Partial> = {}; variationData.forEach((fixtureResult) => { const fixture = fixtureResult.fixture; const dnfKey = `${packageManager}_dnf` as keyof FixtureResult; const value = fixtureResult[packageManager]; const isDnf = fixtureResult[dnfKey] === true; + fixturePartial[fixture] = partialResultLabel( + { ...fixtureResult }, + packageManager, + ); if (isDnf) { fixtureDnf[fixture] = true; @@ -110,6 +117,7 @@ export const VariationTable = ({ packageManager, fixtureValues, fixtureDnf, + fixturePartial, }; }), [filteredPackageManagers, variationData], @@ -168,6 +176,7 @@ export const VariationTable = ({ header: () => {getFixtureDisplayName(fixture)}, cell: (info) => { const isDnf = info.row.original.fixtureDnf[fixture] === true; + const partial = info.row.original.fixturePartial[fixture]; const value = info.getValue(); if (isDnf) { return ( @@ -185,6 +194,11 @@ export const VariationTable = ({ {value.toFixed(decimals)} {unit} + {partial && ( +
+ {partial} +
+ )}

); } diff --git a/app/src/hooks/use-history-data.ts b/app/src/hooks/use-history-data.ts index 7a19274f68..0a715fd6fb 100644 --- a/app/src/hooks/use-history-data.ts +++ b/app/src/hooks/use-history-data.ts @@ -61,13 +61,15 @@ async function parallelLimit( } } - await Promise.all(Array.from({ length: Math.min(limit, tasks.length) }, worker)); + await Promise.all( + Array.from({ length: Math.min(limit, tasks.length) }, worker), + ); return results; } type FixtureDataSet = Record< string, - Array & { fixture: string }> + Array & { fixture: string }> >; interface ChartDataResponse { @@ -105,7 +107,7 @@ interface ChartDataResponse { * * Registry and task-runner variations always use total-time data. */ -function extractDayData( +export function extractDayData( response: ChartDataResponse, ): Record> { const result: Record> = {}; @@ -132,7 +134,7 @@ function extractDayData( function extractFromDataSet( data: Record< string, - Array & { fixture: string }> + Array & { fixture: string }> >, result: Record>, ): void { @@ -140,9 +142,15 @@ function extractFromDataSet( if (!Array.isArray(fixtures) || fixtures.length === 0) continue; const pmTotals: Record = {}; + const partialPMs = new Set( + PACKAGE_MANAGERS.filter((pm) => + fixtures.some((fixture) => fixture[`${pm}_partial`] === true), + ), + ); for (const fixture of fixtures) { for (const pm of PACKAGE_MANAGERS) { + if (partialPMs.has(pm) || fixture[`${pm}_dnf`] === true) continue; const val = fixture[pm]; if (typeof val === "number" && Number.isFinite(val)) { if (!pmTotals[pm]) pmTotals[pm] = { sum: 0, count: 0 }; @@ -163,6 +171,27 @@ function extractFromDataSet( } } +// Preserve the reason a daily value is missing when building category averages. +// Otherwise an average could silently use only the variations that succeeded. +export function hasPartialResult( + response: ChartDataResponse, + variations: string[], + pm: string, +): boolean { + return [ + response.chartData, + response.perPackageCountChartData, + response.registryChartData, + response.registryPerPackageCountChartData, + ].some((source) => + variations.some((variation) => + source?.data[variation]?.some( + (fixture) => fixture[`${pm}_partial`] === true, + ), + ), + ); +} + export const useHistoryData = (): UseHistoryDataReturn => { const [historyData, setHistoryData] = useState(null); const [loading, setLoading] = useState(true); @@ -259,6 +288,16 @@ export const useHistoryData = (): UseHistoryDataReturn => { for (const pm of PACKAGE_MANAGERS) { avgSeries[pm] = []; for (let i = 0; i < dates.length; i++) { + if ( + hasPartialResult( + successfulResults[i].data, + sourceVariations, + pm, + ) + ) { + avgSeries[pm].push(null); + continue; + } let sum = 0; let count = 0; for (const v of present) { diff --git a/app/src/lib/run-completeness.ts b/app/src/lib/run-completeness.ts new file mode 100644 index 0000000000..79bb39a452 --- /dev/null +++ b/app/src/lib/run-completeness.ts @@ -0,0 +1,13 @@ +// Accept both fixture rows and Recharts payloads. Older data has no metadata. +export const partialResultLabel = ( + result: Record | undefined, + packageManager?: string, +): string | undefined => { + const prefix = packageManager ? `${packageManager}_` : ""; + if (result?.[`${prefix}partial`] !== true) return undefined; + const attempted = result[`${prefix}attempted_runs`]; + const successful = result[`${prefix}successful_runs`]; + return typeof attempted === "number" && typeof successful === "number" + ? `Partial: ${successful}/${attempted} runs succeeded` + : "Partial result"; +}; diff --git a/app/src/lib/utils.ts b/app/src/lib/utils.ts index 1e615c0b82..c90fd7b90f 100644 --- a/app/src/lib/utils.ts +++ b/app/src/lib/utils.ts @@ -98,6 +98,16 @@ export const calculateAverageVariationData = ( packageManagers.forEach((pm: PackageManager) => { const dnfKey: DnfKey = `${pm}_dnf`; + if (results.some((r) => r[`${pm}_partial`] === true)) { + averagedResult[`${pm}_partial`] = true; + for (const field of ["attempted_runs", "successful_runs", "dropped_runs"] as const) { + const key = `${pm}_${field}` as const; + // Do not claim a complete count if some historical rows lack it. + if (results.every((r) => typeof r[key] === "number")) { + averagedResult[key] = results.reduce((sum, r) => sum + (r[key] ?? 0), 0); + } + } + } const values = results .filter((r) => r[dnfKey] !== true) .map((r) => r[pm]) @@ -300,6 +310,21 @@ export const calculateLeaderboard = ( ?.variations.filter((v) => v !== "average") || []; } + // A survivor-only result cannot establish a fair rank, including when other + // fixtures succeeded. Exclude the command from this selected comparison. + const partialPMs = new Set(); + const rankingData = usePerPackageData + ? chartData.perPackageCountChartData.data + : chartData.chartData.data; + for (const variation of variationsToUse) { + for (const row of rankingData[variation] ?? []) { + if (enabledFixtures && !enabledFixtures.has(row.fixture)) continue; + for (const pm of availablePackageManagers) { + if (row[`${pm}_partial`] === true) partialPMs.add(pm); + } + } + } + // Calculate performance — DNF runs are imputed as the slowest successful // time for that fixture, matching the "Performance Over Time" chart data variationsToUse.forEach((variation) => { @@ -320,6 +345,7 @@ export const calculateLeaderboard = ( // First pass: collect successful times and DNFs (availablePackageManagers as PackageManager[]).forEach((pm) => { + if (partialPMs.has(pm)) return; const time = fixtureResult[pm]; const dnfKey = `${pm}_dnf` as keyof FixtureResult; if (fixtureResult[dnfKey] === true) { diff --git a/app/src/types/chart-data.ts b/app/src/types/chart-data.ts index b2474f337b..db8867934f 100644 --- a/app/src/types/chart-data.ts +++ b/app/src/types/chart-data.ts @@ -19,13 +19,7 @@ export type PackageManager = | "jfrog"; export type Fixture = - | "next" - | "astro" - | "svelte" - | "vue" - | "large" - | "babylon" - | "run"; + "next" | "astro" | "svelte" | "vue" | "large" | "babylon" | "run"; export type Variation = | "average" @@ -71,7 +65,15 @@ export interface BaseFixtureResult { fixture: Fixture; } -export interface PackageManagerData { +type RunCompletenessData = Partial< + Record<`${PackageManager}_partial`, boolean> & + Record< + `${PackageManager}_${"attempted_runs" | "successful_runs" | "dropped_runs"}`, + number + > +>; + +export interface PackageManagerData extends RunCompletenessData { npm?: number; yarn?: number; pnpm?: number; diff --git a/app/tests/partial-results.test.js b/app/tests/partial-results.test.js new file mode 100644 index 0000000000..fa54fd5bda --- /dev/null +++ b/app/tests/partial-results.test.js @@ -0,0 +1,101 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { partialResultLabel } from "../src/lib/run-completeness.ts"; +import { + calculateAverageVariationData, + calculateLeaderboard, +} from "../src/lib/utils.ts"; +import { + extractDayData, + hasPartialResult, +} from "../src/hooks/use-history-data.ts"; + +const complete = { fixture: "next", npm: 5, vlt: 4 }; +const partial = { + fixture: "astro", + npm: 0.1, + vlt: 3, + npm_partial: true, + npm_attempted_runs: 3, + npm_successful_runs: 1, + npm_dropped_runs: 2, +}; +const dataSet = { + variations: ["clean"], + data: { clean: [complete, partial] }, + packageManagers: ["npm", "vlt"], + colors: { npm: "#cb0606", vlt: "#000000" }, +}; +const chartData = { + date: "2026-09-15", + chartData: dataSet, + perPackageCountChartData: dataSet, +}; + +test("partial labels show attempted and successful counts without inferring historical completeness", () => { + assert.equal( + partialResultLabel(partial, "npm"), + "Partial: 1/3 runs succeeded", + ); + assert.equal( + partialResultLabel({ npm_partial: true }, "npm"), + "Partial result", + ); + assert.equal(partialResultLabel(complete, "npm"), undefined); +}); + +test("a partial command cannot rank using only its other successful fixtures", () => { + const ranking = calculateLeaderboard(chartData, "clean", "package-managers"); + assert.deepEqual( + ranking.map((row) => row.packageManager), + ["vlt"], + ); + // Restricting to a fixture with complete results permits normal comparisons. + assert.equal( + calculateLeaderboard( + chartData, + "clean", + "package-managers", + new Set(["next"]), + ).length, + 2, + ); +}); + +test("synthetic averages keep partial labels and run counts", () => { + const average = calculateAverageVariationData(chartData); + assert.equal( + average.find((row) => row.fixture === "astro").npm_partial, + true, + ); + assert.equal( + average.find((row) => row.fixture === "astro").npm_attempted_runs, + 3, + ); + assert.equal( + average.find((row) => row.fixture === "next").npm_partial, + undefined, + ); +}); + +test("daily history omits the entire partial command instead of averaging only surviving fixtures", () => { + assert.deepEqual(extractDayData(chartData), { clean: { vlt: 3.5 } }); + assert.equal(hasPartialResult(chartData, ["clean", "cache"], "npm"), true); + assert.equal(hasPartialResult(chartData, ["clean", "cache"], "vlt"), false); +}); + +test("complete and historical daily results remain usable and DNF placeholders are excluded", () => { + const response = { + ...chartData, + perPackageCountChartData: { + ...dataSet, + data: { + clean: [ + complete, + { fixture: "astro", npm: 100, npm_dnf: true, vlt: 6 }, + ], + }, + }, + }; + assert.deepEqual(extractDayData(response), { clean: { npm: 5, vlt: 5 } }); +}); diff --git a/scripts/clean-benchmarks.js b/scripts/clean-benchmarks.js index 3cc29547f3..786c060a17 100644 --- a/scripts/clean-benchmarks.js +++ b/scripts/clean-benchmarks.js @@ -37,7 +37,9 @@ const collectBenchmarkFiles = (targetPath) => { const stats = fs.statSync(resolvedPath); if (stats.isFile()) { - return path.basename(resolvedPath) === "benchmarks.json" ? [resolvedPath] : []; + return path.basename(resolvedPath) === "benchmarks.json" + ? [resolvedPath] + : []; } if (!stats.isDirectory()) { @@ -84,7 +86,9 @@ const cleanBenchmarkFile = (filePath) => { data.results.forEach((result, index) => { const times = Array.isArray(result.times) ? result.times : null; - const exitCodes = Array.isArray(result.exit_codes) ? result.exit_codes : null; + const exitCodes = Array.isArray(result.exit_codes) + ? result.exit_codes + : null; if (!times || !exitCodes || times.length !== exitCodes.length) { console.warn( @@ -96,6 +100,28 @@ const cleanBenchmarkFile = (filePath) => { const cleanTimes = times.filter((time, idx) => exitCodes[idx] === 0); const cleanExitCodes = exitCodes.filter((code) => code === 0); + // Keep the original outcomes even when this file is cleaned repeatedly. + // Hyperfine's user/system values cover all attempts, not just survivors. + const originalExitCodes = Array.isArray(result.original_exit_codes) + ? result.original_exit_codes + : [...exitCodes]; + result.original_exit_codes = originalExitCodes; + result.attempted_runs = originalExitCodes.length; + result.successful_runs = originalExitCodes.filter( + (code) => code === 0, + ).length; + result.dropped_runs = result.attempted_runs - result.successful_runs; + result.status = + result.successful_runs === 0 + ? "failure" + : result.dropped_runs > 0 + ? "partial" + : "success"; + if (result.dropped_runs > 0) { + delete result.user; + delete result.system; + } + if (cleanTimes.length > 0) { const mean = calculateMean(cleanTimes); const stddev = calculateStddev(cleanTimes, mean); @@ -106,7 +132,8 @@ const cleanBenchmarkFile = (filePath) => { result.times = cleanTimes; result.exit_codes = cleanExitCodes; result.mean = mean; - result.stddev = stddev; + result.stddev = + result.dropped_runs > 0 && cleanTimes.length < 2 ? null : stddev; result.median = median; result.min = min; result.max = max; @@ -124,8 +151,6 @@ const cleanBenchmarkFile = (filePath) => { result.median = 0; result.min = 0; result.max = 0; - result.user = 0; - result.system = 0; updated = true; } @@ -142,7 +167,9 @@ const cleanBenchmarkFile = (filePath) => { }; const filesToClean = Array.from( - new Set(inputPaths.flatMap((targetPath) => collectBenchmarkFiles(targetPath))), + new Set( + inputPaths.flatMap((targetPath) => collectBenchmarkFiles(targetPath)), + ), ); if (filesToClean.length === 0) { diff --git a/scripts/generate-chart.js b/scripts/generate-chart.js index faa4e9335c..b4eafade4d 100644 --- a/scripts/generate-chart.js +++ b/scripts/generate-chart.js @@ -90,6 +90,12 @@ function readResults(file) { mean: parseNumeric(r.mean), stddev: parseNumeric(r.stddev), exitCodes, + partial: + r.status === "partial" || + (r.dropped_runs > 0 && r.successful_runs > 0), + attempted_runs: r.attempted_runs, + successful_runs: r.successful_runs, + dropped_runs: r.dropped_runs, failed: exitCodes.some((code) => typeof code === "number" && code !== 0) || r.success === false || @@ -107,6 +113,17 @@ function readResults(file) { } } +// Older published results have no completeness metadata. Leave it absent: +// discarded historical attempts cannot be recovered from survivor-only files. +function copyRunMetadata(target, command, result) { + if (result.partial) target[`${command}_partial`] = true; + for (const field of ["attempted_runs", "successful_runs", "dropped_runs"]) { + if (Number.isInteger(result[field]) && result[field] >= 0) { + target[`${command}_${field}`] = result[field]; + } + } +} + // Generate chart data for Recharts function generateChartData(option = {}) { const fixtures = ["next", "astro", "svelte", "vue", "large", "babylon", "run"]; @@ -192,6 +209,7 @@ function generateChartData(option = {}) { } pmEntries[pm] = { + ...pmResult, didFail, value: timing?.value, stddev: timing?.stddev, @@ -217,6 +235,7 @@ function generateChartData(option = {}) { const fallback = entry.slowestValid ?? fallbackGlobal; Object.entries(entry.pmEntries).forEach(([pm, pmEntry]) => { + copyRunMetadata(fixtureResults, pm, pmEntry); fixtureResults[`${pm}_fill`] = COLORS[pm]; if (pmEntry.count !== undefined) { fixtureResults[`${pm}_count`] = pmEntry.count; @@ -224,6 +243,7 @@ function generateChartData(option = {}) { if (pmEntry.didFail) { fixtureResults[`${pm}_dnf`] = true; + hasData = true; if (typeof fallback === "number") { fixtureResults[pm] = fallback; hasData = true; @@ -351,6 +371,7 @@ function generateRegistryChartData(option = {}) { } pmEntries[registry] = { + ...registryResult, didFail, value: timing?.value, stddev: timing?.stddev, @@ -376,6 +397,7 @@ function generateRegistryChartData(option = {}) { const fallback = entry.slowestValid ?? fallbackGlobal; Object.entries(entry.pmEntries).forEach(([registry, regEntry]) => { + copyRunMetadata(fixtureResults, registry, regEntry); fixtureResults[`${registry}_fill`] = REGISTRY_COLORS[registry]; if (regEntry.count !== undefined) { fixtureResults[`${registry}_count`] = regEntry.count; @@ -383,6 +405,7 @@ function generateRegistryChartData(option = {}) { if (regEntry.didFail) { fixtureResults[`${registry}_dnf`] = true; + hasData = true; if (typeof fallback === "number") { fixtureResults[registry] = fallback; hasData = true; diff --git a/scripts/partial-results.test.js b/scripts/partial-results.test.js new file mode 100644 index 0000000000..b6feb0d6b0 --- /dev/null +++ b/scripts/partial-results.test.js @@ -0,0 +1,193 @@ +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { spawnSync } = require("node:child_process"); +const test = require("node:test"); + +const temporaryDirectory = (t) => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "partial-results-")); + t.after(() => fs.rmSync(directory, { recursive: true, force: true })); + return directory; +}; + +const runScript = (script, args, cwd) => { + const result = spawnSync( + process.execPath, + [path.join(__dirname, script), ...args], + { + cwd, + encoding: "utf8", + }, + ); + assert.equal(result.status, 0, result.stderr); +}; + +const rawResult = (exit_codes, times = [1, 2, 300]) => ({ + command: "npm", + times, + exit_codes, + mean: 101, + stddev: 140, + median: 2, + min: 1, + max: 300, + user: 90, + system: 10, +}); + +for (const scenario of [ + { + name: "complete success", + codes: [0, 0, 0], + status: "success", + successes: 3, + }, + { + name: "partial success", + codes: [0, 0, 124], + status: "partial", + successes: 2, + }, + { + name: "single survivor", + codes: [0, 1, 124], + status: "partial", + successes: 1, + }, + { + name: "total failure", + codes: [1, 2, 124], + status: "failure", + successes: 0, + }, +]) { + test(`preserves outcomes for ${scenario.name}, including repeated cleanup`, (t) => { + const directory = temporaryDirectory(t); + const file = path.join(directory, "benchmarks.json"); + fs.writeFileSync( + file, + JSON.stringify({ results: [rawResult(scenario.codes)] }), + ); + runScript("clean-benchmarks.js", [directory], directory); + const cleaned = JSON.parse(fs.readFileSync(file, "utf8")).results[0]; + assert.equal(cleaned.attempted_runs, 3); + assert.equal(cleaned.successful_runs, scenario.successes); + assert.equal(cleaned.dropped_runs, 3 - scenario.successes); + assert.equal(cleaned.status, scenario.status); + assert.deepEqual(cleaned.original_exit_codes, scenario.codes); + if (scenario.successes === 3) { + assert.equal(cleaned.user, 90); + assert.equal(cleaned.system, 10); + } else { + assert.equal(Object.hasOwn(cleaned, "user"), false); + assert.equal(Object.hasOwn(cleaned, "system"), false); + } + if (scenario.successes > 0) { + assert.deepEqual(cleaned.times, [1, 2, 300].slice(0, scenario.successes)); + assert.deepEqual(cleaned.exit_codes, Array(scenario.successes).fill(0)); + assert.equal(cleaned.mean, [0, 1, 1.5, 101][scenario.successes]); + if (scenario.successes === 1) assert.equal(cleaned.stddev, null); + } else { + // Preserve the historical DNF sentinel for existing downstream consumers. + assert.equal(cleaned.mean, 0); + assert.deepEqual(cleaned.exit_codes, [1]); + } + runScript("clean-benchmarks.js", [directory], directory); + assert.deepEqual( + JSON.parse(fs.readFileSync(file, "utf8")).results[0], + cleaned, + ); + }); +} + +test("chart generation preserves partial metadata for PMs and registries in both units", (t) => { + const directory = temporaryDirectory(t); + const date = "2026-09-15"; + const resultDirectory = path.join(directory, "results", date); + fs.mkdirSync(resultDirectory, { recursive: true }); + const file = path.join(directory, "benchmarks.json"); + fs.writeFileSync( + file, + JSON.stringify({ + results: [ + rawResult([0, 1, 124]), + { ...rawResult([1, 1, 124]), command: "vlt" }, + { ...rawResult([0, 0, 0]), command: "bun" }, + ], + }), + ); + runScript("clean-benchmarks.js", [file], directory); + for (const variation of ["clean", "registry-clean"]) { + fs.copyFileSync(file, path.join(resultDirectory, `next-${variation}.json`)); + fs.writeFileSync( + path.join(resultDirectory, `next-${variation}-package-count.json`), + JSON.stringify({ + npm: { count: 100 }, + vlt: { count: 100 }, + bun: { count: 100 }, + }), + ); + } + runScript("generate-chart.js", [date], directory); + const output = JSON.parse( + fs.readFileSync(path.join(resultDirectory, "chart-data.json"), "utf8"), + ); + for (const [key, variation, value] of [ + ["chartData", "clean", 1], + ["perPackageCountChartData", "clean", 10], + ["registryChartData", "registry-clean", 1], + ["registryPerPackageCountChartData", "registry-clean", 10], + ]) { + const row = output[key].data[variation][0]; + assert.equal(row.npm, value); + assert.equal(row.npm_partial, true); + assert.equal(row.npm_attempted_runs, 3); + assert.equal(row.npm_successful_runs, 1); + assert.equal(row.npm_dropped_runs, 2); + assert.equal(Object.hasOwn(row, "npm_stddev"), false); + assert.equal(row.vlt_dnf, true); + assert.equal(row.vlt_successful_runs, 0); + assert.equal(row.vlt_dropped_runs, 3); + assert.equal(row.bun_partial, undefined); + } +}); + +test("historical output remains readable without inventing completeness counts", (t) => { + const directory = temporaryDirectory(t); + const date = "2025-01-01"; + const resultDirectory = path.join(directory, "results", date); + fs.mkdirSync(resultDirectory, { recursive: true }); + fs.writeFileSync( + path.join(resultDirectory, "next-clean.json"), + JSON.stringify({ + results: [{ command: "npm", mean: 2, stddev: 0.5, exit_codes: [0, 0] }], + }), + ); + // An all-DNF fixture must still reach the table even without a timing fallback. + fs.writeFileSync( + path.join(resultDirectory, "next-build.json"), + JSON.stringify({ + results: [{ command: "npm", mean: 0, stddev: 0, exit_codes: [1] }], + }), + ); + runScript("generate-chart.js", [date], directory); + const output = JSON.parse( + fs.readFileSync(path.join(resultDirectory, "chart-data.json"), "utf8"), + ); + const row = output.chartData.data.clean[0]; + assert.equal(row.npm, 2); + assert.equal(Object.hasOwn(row, "npm_attempted_runs"), false); + assert.equal(Object.hasOwn(row, "npm_partial"), false); + assert.equal(output.chartData.data.build[0].npm_dnf, true); +}); + +test("CI scans raw exit codes before processing results", () => { + const workflow = fs.readFileSync( + path.join(__dirname, "../.github/workflows/benchmark.yaml"), + "utf8", + ); + const rawScan = workflow.indexOf("- name: Scan Raw Registry Results"); + const processResults = workflow.indexOf("- name: Process Results"); + assert.ok(rawScan >= 0 && rawScan < processResults); +});