From 4623c2d40ac956179375dfe756d5a4f1d3443c95 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Sep 2026 04:37:40 -0400 Subject: [PATCH 1/2] feat: refresh electron's CI test-shard weights from its own timings electron/electron packs its CI test shards from script/spec-weights.json, a table of seconds per spec file for each test job that has to be regenerated by hand from the spec-timings.json files the test jobs upload. Nobody does, so the tables drift as specs are added, removed, or change speed, and each release branch drifts on its own. Add a spec-weights roller: for main and every supported release branch it takes the last three green push runs of build.yml, reads the timing files out of their test_artifacts_* uploads (one table per job, keyed as the artifact is named; the Wayland allowlist job is left out), takes the median across runs so a single slow shard cannot skew a file, and packs the branch's spec files with the committed table and the fresh one using the same algorithm as split-tests.js. When some job's longest shard would get at least a minute shorter, a spec file has no weight, or a file's weight moved by a quarter and thirty seconds, it opens a PR on that branch (or updates its own open one), labelled semver/none and never backported since the numbers are per branch. Run as `npm run roll-spec-weights [branch]`. --- package.json | 1 + src/constants.ts | 10 + src/spec-weights-cron.ts | 12 + src/spec-weights-handler.ts | 63 ++++ src/utils/roll-spec-weights.ts | 279 ++++++++++++++++++ src/utils/spec-weights.ts | 264 +++++++++++++++++ .../test_artifacts_darwin_x64_1.zip | Bin 0 -> 419 bytes tests/utils/roll-spec-weights.spec.ts | 154 ++++++++++ tests/utils/spec-weights.spec.ts | 236 +++++++++++++++ 9 files changed, 1019 insertions(+) create mode 100644 src/spec-weights-cron.ts create mode 100644 src/spec-weights-handler.ts create mode 100644 src/utils/roll-spec-weights.ts create mode 100644 src/utils/spec-weights.ts create mode 100644 tests/fixtures/spec-weights/test_artifacts_darwin_x64_1.zip create mode 100644 tests/utils/roll-spec-weights.spec.ts create mode 100644 tests/utils/spec-weights.spec.ts diff --git a/package.json b/package.json index b3cfe6e..13659fd 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "prepare": "husky", "roll-chromium": "DEBUG=roller* node lib/chromium-cron.js", "roll-node": "DEBUG=roller* node lib/node-cron.js", + "roll-spec-weights": "DEBUG=roller* node lib/spec-weights-cron.js", "start": "DEBUG=roller* probot run ./lib/index.js", "test": "vitest run --coverage --reporter=verbose" }, diff --git a/src/constants.ts b/src/constants.ts index d40a4bb..7b87b27 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -70,3 +70,13 @@ export interface Repository { owner: string; repo: string; } + +// The CI test-shard weight tables in electron/electron, refreshed from the +// spec-timings.json files the test jobs upload. +export const SPEC_WEIGHTS = { + filePath: 'script/spec-weights.json', + specDir: 'spec', + workflowFile: 'build.yml', + // Green push runs whose timings are combined (median per file) for one refresh. + runsToSample: 3, +}; diff --git a/src/spec-weights-cron.ts b/src/spec-weights-cron.ts new file mode 100644 index 0000000..b851f93 --- /dev/null +++ b/src/spec-weights-cron.ts @@ -0,0 +1,12 @@ +import { realpathSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +import { handleSpecWeightsCheck } from './spec-weights-handler.js'; + +if (realpathSync(process.argv[1]) === fileURLToPath(import.meta.url)) { + handleSpecWeightsCheck(process.argv[2]).catch((err) => { + console.log('Spec Weights Cron Failed'); + console.error(err); + process.exit(1); + }); +} diff --git a/src/spec-weights-handler.ts b/src/spec-weights-handler.ts new file mode 100644 index 0000000..cd33be2 --- /dev/null +++ b/src/spec-weights-handler.ts @@ -0,0 +1,63 @@ +import debug from 'debug'; + +import { MAIN_BRANCH, REPOS } from './constants.js'; +import { getOctokit } from './utils/octokit.js'; +import { getSupportedBranches } from './utils/get-supported-branches.js'; +import { computeRefresh, rollSpecWeights } from './utils/roll-spec-weights.js'; +import type { Branch } from './types.js'; + +/** + * Refreshes script/spec-weights.json on main and every supported release + * branch from the timings their CI test jobs upload, opening a PR per branch + * whose shards would pack materially better with fresh numbers. + */ +export async function handleSpecWeightsCheck(target?: string): Promise { + const d = debug('roller/spec-weights:handleSpecWeightsCheck()'); + const github = await getOctokit(); + + let branches: Branch[]; + if (target) { + const { data } = await github.repos.getBranch({ ...REPOS.electron, branch: target }); + branches = [{ name: data.name, commit: { sha: data.commit.sha } }]; + } else { + d('Fetching release branches for electron/electron'); + const releaseBranches = await getSupportedBranches(github, 4); + const { data: main } = await github.repos.getBranch({ + ...REPOS.electron, + branch: MAIN_BRANCH, + }); + branches = [...releaseBranches, { name: main.name, commit: { sha: main.commit.sha } }]; + } + d(`Checking ${branches.map((b) => b.name).join(', ')}`); + + let failed = false; + for (const branch of branches) { + try { + await refreshBranch(branch); + } catch (e) { + d(`Failed to refresh ${branch.name}: ${e.message}`); + failed = true; + } + } + + if (failed) { + throw new Error('One or more spec weight refreshes failed - see logs for more details'); + } +} + +async function refreshBranch(branch: Branch): Promise { + const d = debug(`roller/spec-weights:refreshBranch(${branch.name})`); + const github = await getOctokit(); + + const refresh = await computeRefresh(github, branch.name); + if (!refresh) return; + + if (!refresh.assessment.material) { + d('Committed weights are within tolerance of the fresh measurements - nothing to do'); + return; + } + + d(`Refresh is material:\n ${refresh.assessment.reasons.join('\n ')}`); + const rolled = await rollSpecWeights(branch, refresh); + d(rolled ? 'PR created or updated' : 'Open PR already up to date'); +} diff --git a/src/utils/roll-spec-weights.ts b/src/utils/roll-spec-weights.ts new file mode 100644 index 0000000..ffeb11c --- /dev/null +++ b/src/utils/roll-spec-weights.ts @@ -0,0 +1,279 @@ +import debug from 'debug'; +import type { Octokit } from '@octokit/rest'; + +import { + BACKPORT_CHECK_SKIP, + MAIN_BRANCH, + NO_BACKPORT, + REPOS, + ROLLER_BOT_LOGIN, + SPEC_WEIGHTS, +} from '../constants.js'; +import type { Branch, PullsListResponseItem } from '../types.js'; +import { getContent } from './github-utils.js'; +import { addLabels } from './label-utils.js'; +import { getOctokit } from './octokit.js'; +import { + aggregateRun, + assessChange, + jobKeyOfArtifact, + mergeRuns, + serializeWeights, + timingsFromArtifactZip, + type JobTimings, + type Materiality, + type Weights, +} from './spec-weights.js'; + +/** Spec files the sharder would see on `ref`: spec/*-spec.ts, as split-tests.js globs them. */ +export async function listSpecFiles(octokit: Octokit, ref: string): Promise { + const { data } = await octokit.git.getTree({ + ...REPOS.electron, + tree_sha: `${ref}:${SPEC_WEIGHTS.specDir}`, + }); + return data.tree + .filter((entry) => entry.type === 'blob' && entry.path?.endsWith('-spec.ts')) + .map((entry) => `${SPEC_WEIGHTS.specDir}/${entry.path}`) + .sort(); +} + +/** The most recent successful push runs of build.yml on `branch`, newest first. */ +export async function listTimingRuns( + octokit: Octokit, + branch: string, + count: number, +): Promise { + const { data } = await octokit.actions.listWorkflowRuns({ + ...REPOS.electron, + workflow_id: SPEC_WEIGHTS.workflowFile, + branch, + event: 'push', + status: 'success', + per_page: count, + }); + return data.workflow_runs.map((run) => run.id); +} + +/** Every spec-timings.json uploaded by the test jobs of one run, tagged with its job key. */ +export async function fetchRunTimings(octokit: Octokit, runId: number): Promise { + const d = debug(`roller/spec-weights:fetchRunTimings(${runId})`); + const artifacts = await octokit.paginate(octokit.actions.listWorkflowRunArtifacts, { + ...REPOS.electron, + run_id: runId, + per_page: 100, + }); + const jobs: JobTimings[] = []; + for (const artifact of artifacts) { + const key = jobKeyOfArtifact(artifact.name); + if (!key || artifact.expired) continue; + const { data } = await octokit.actions.downloadArtifact({ + ...REPOS.electron, + artifact_id: artifact.id, + archive_format: 'zip', + }); + const found = timingsFromArtifactZip(Buffer.from(data as ArrayBuffer)); + d(`${artifact.name}: ${found.length} timing file(s) -> ${key}`); + jobs.push(...found.map((timings) => ({ key, timings }))); + } + return jobs; +} + +export interface SpecWeightsRefresh { + branch: string; + runIds: number[]; + committed: Weights; + fresh: Weights; + content: string; + assessment: Materiality; +} + +/** + * Computes fresh weights for `branch` from its last few green push runs and + * compares them with the table committed on the branch. + */ +export async function computeRefresh( + octokit: Octokit, + branch: string, +): Promise { + const d = debug(`roller/spec-weights:computeRefresh(${branch})`); + + const runIds = await listTimingRuns(octokit, branch, SPEC_WEIGHTS.runsToSample); + if (!runIds.length) { + d('no successful push runs found - skipping'); + return null; + } + + const runs: Weights[] = []; + for (const runId of runIds) { + const jobs = await fetchRunTimings(octokit, runId); + if (jobs.length) runs.push(aggregateRun(jobs)); + else d(`run ${runId} has no timing artifacts (expired?) - ignoring`); + } + if (!runs.length) { + d('no timing data in any sampled run - skipping'); + return null; + } + + const specFiles = await listSpecFiles(octokit, branch); + const fresh = mergeRuns(runs, specFiles); + + const existing = await getContent(octokit, { + ...REPOS.electron, + path: SPEC_WEIGHTS.filePath, + ref: branch, + }); + const committed: Weights = existing ? JSON.parse(existing.content) : {}; + + return { + branch, + runIds, + committed, + fresh, + content: serializeWeights(fresh), + assessment: assessChange(committed, fresh, specFiles), + }; +} + +const minutes = (seconds: number) => (seconds / 60).toFixed(1); + +export function getSpecWeightsPRText(refresh: SpecWeightsRefresh) { + const { branch, runIds, assessment } = refresh; + const runLinks = runIds + .map((id) => `[${id}](https://github.com/electron/electron/actions/runs/${id})`) + .join(', '); + const rows = assessment.changes + .map( + (c) => + `| \`${c.table}\` | ${c.shardCount} | ${minutes(c.before)} min | ${minutes(c.after)} min | ${ + c.unweighted.length + } |`, + ) + .join('\n'); + const moved = assessment.changes + .flatMap((c) => c.moved.map((m) => `- \`${c.table}\` ${m.spec}: ${m.from}s -> ${m.to}s`)) + .join('\n'); + return { + title: `build: refresh spec weights (${branch})`, + body: `Regenerates \`${SPEC_WEIGHTS.filePath}\` from the timings uploaded by the last ${ + runIds.length + } green push run(s) on \`${branch}\`: ${runLinks}. Each file's weight is the median across those runs; the tables drive how \`script/split-tests.js\` packs the CI test shards. + +Longest test shard, timed with the fresh measurements, packed by the committed weights versus these: + +| job | shards | committed | refreshed | unweighted files | +| --- | --- | --- | --- | --- | +${rows} +${moved ? `\nFiles that moved by 25% or more:\n${moved}\n` : ''} +Notes: none`, + }; +} + +/** + * Opens or updates the roller's spec-weights PR on `electronBranch`. Returns + * true when a PR was created or its content changed. + */ +export async function rollSpecWeights( + electronBranch: Branch, + refresh: SpecWeightsRefresh, +): Promise { + const d = debug(`roller/spec-weights:rollSpecWeights(${electronBranch.name})`); + const octokit = await getOctokit(); + + const rollBranchName = `roller/spec-weights/${electronBranch.name}`; + const shortRef = `heads/${rollBranchName}`; + const ref = `refs/${shortRef}`; + const electronRepoFullName = `${REPOS.electron.owner}/${REPOS.electron.repo}`; + const prText = getSpecWeightsPRText(refresh); + const commitMessage = `build: refresh spec weights from CI timings\n\nMedian of push runs ${refresh.runIds.join( + ', ', + )} on ${electronBranch.name}.`; + + const existingPrsForBranch = (await octokit.paginate('GET /repos/:owner/:repo/pulls', { + ...REPOS.electron, + base: electronBranch.name, + state: 'open', + })) as PullsListResponseItem[]; + + // Only ever write to the bot's own PR: authored by the roller, head in the + // electron/electron repo itself, named exactly as the bot names it. Any open + // PR can share the title prefix, and a fork PR's fields are attacker-controlled. + const prs = existingPrsForBranch.filter( + (pr) => + pr.title.startsWith('build: refresh spec weights') && + pr.user?.login === ROLLER_BOT_LOGIN && + pr.head.repo?.full_name === electronRepoFullName && + pr.head.ref === rollBranchName, + ); + + if (prs.length) { + let changed = false; + for (const pr of prs) { + d(`Found existing PR #${pr.number}`); + if (pr.labels.some((label) => label.name === 'roller/pause')) { + d(`Automatic updates have been paused for #${pr.number}, skipping.`); + continue; + } + const current = await getContent(octokit, { + ...REPOS.electron, + path: SPEC_WEIGHTS.filePath, + ref: rollBranchName, + }); + if (current?.content === refresh.content) { + d('PR already carries these weights'); + continue; + } + await octokit.repos.createOrUpdateFileContents({ + ...REPOS.electron, + path: SPEC_WEIGHTS.filePath, + message: commitMessage, + content: Buffer.from(refresh.content).toString('base64'), + branch: rollBranchName, + sha: current?.sha, + }); + await octokit.pulls.update({ ...REPOS.electron, pull_number: pr.number, ...prText }); + changed = true; + } + return changed; + } + + d(`No existing PR - creating ${ref} at ${electronBranch.commit.sha}`); + try { + await octokit.git.getRef({ ...REPOS.electron, ref: shortRef }); + d(`Found orphan ref ${ref} with no open PR - deleting`); + await octokit.git.deleteRef({ ...REPOS.electron, ref: shortRef }); + } catch { + // No orphan ref. + } + await octokit.git.createRef({ ...REPOS.electron, ref, sha: electronBranch.commit.sha }); + + const current = await getContent(octokit, { + ...REPOS.electron, + path: SPEC_WEIGHTS.filePath, + ref: rollBranchName, + }); + await octokit.repos.createOrUpdateFileContents({ + ...REPOS.electron, + path: SPEC_WEIGHTS.filePath, + message: commitMessage, + content: Buffer.from(refresh.content).toString('base64'), + branch: rollBranchName, + sha: current?.sha, + }); + + const { data: pr } = await octokit.pulls.create({ + ...REPOS.electron, + base: electronBranch.name, + head: `${REPOS.electron.owner}:${rollBranchName}`, + ...prText, + }); + // Weights are measured per branch, so a refresh is never backported. + await addLabels(octokit, { + prNumber: pr.number, + labels: [ + 'semver/none', + electronBranch.name === MAIN_BRANCH ? NO_BACKPORT : BACKPORT_CHECK_SKIP, + ], + }); + d(`New PR: ${pr.html_url}`); + return true; +} diff --git a/src/utils/spec-weights.ts b/src/utils/spec-weights.ts new file mode 100644 index 0000000..f4758a9 --- /dev/null +++ b/src/utils/spec-weights.ts @@ -0,0 +1,264 @@ +import { inflateRawSync } from 'node:zlib'; + +// The per-file timing tables that electron/electron's script/split-tests.js +// packs CI test shards from: seconds per spec file, one table per CI test +// job, keyed the way the job names its test_artifacts_* upload - +// `_[_]` (darwin_x64, mas_arm64, linux_x64_asan, +// win_x64). Regenerating them is what script/gen-spec-weights.js does from +// one run; this module does the same from several runs so a single slow +// shard cannot skew a file for a week. + +export type WeightTable = Record; +export type Weights = Record; + +export interface SpecTimings { + platform: string; + arch: string; + mas: boolean; + sanitizer?: 'asan' | 'ubsan' | null; + files: Record; +} + +/** A run's timing file together with the job key its artifact name carries. */ +export interface JobTimings { + key: string; + timings: SpecTimings; +} + +// test_artifacts_[_x11]_: the plain and sanitizer test jobs on +// every platform. The Wayland job runs an allowlist and is left out. +const TIMING_ARTIFACT = + /^test_artifacts_((?:darwin|mas|linux|win)_(?:x64|arm64)(?:_(?:asan|ubsan))?)(?:_x11)?_\d+$/; + +/** The job key of a test_artifacts_* name, or null for artifacts that carry no usable timings. */ +export const jobKeyOfArtifact = (name: string): string | null => + TIMING_ARTIFACT.exec(name)?.[1] ?? null; + +/** + * How many shards a job splits its specs into in CI (the matrix in + * pipeline-electron-build-and-test.yml): Linux and macOS x64 run 3, macOS + * UBSan 3, everything else 2. + */ +export function shardCountOf(key: string): number { + const [buildType, arch, sanitizer] = key.split('_'); + if (buildType === 'linux') return 3; + if ( + (buildType === 'darwin' || buildType === 'mas') && + (arch === 'x64' || sanitizer === 'ubsan') + ) { + return 3; + } + return 2; +} + +/** + * Builds the weight tables for one CI run from that run's timing files, one + * table per job, taking the largest time seen where several shards of a job + * cover a file - what script/gen-spec-weights.js does. + */ +export function aggregateRun(jobs: JobTimings[]): Weights { + const weights: Weights = {}; + for (const { key, timings } of jobs) { + const table = (weights[key] ??= {}); + for (const [spec, seconds] of Object.entries(timings.files)) { + table[spec] = Math.max(table[spec] ?? 0, seconds); + } + } + return weights; +} + +const median = (values: number[]): number => { + const sorted = [...values].sort((a, b) => a - b); + const mid = Math.floor(sorted.length / 2); + return sorted.length % 2 ? sorted[mid] : (sorted[mid - 1] + sorted[mid]) / 2; +}; + +/** + * Merges the per-run tables of several runs: the median across runs of each + * file's time, rounded to whole seconds, restricted to the spec files that + * exist on the branch today. A file measured in only some runs uses the runs + * that have it; a file no run measured gets no entry (split-tests.js then + * gives it the median weight of the known files). + */ +export function mergeRuns(runs: Weights[], specFiles: string[]): Weights { + const merged: Weights = {}; + const tables = new Set(runs.flatMap((r) => Object.keys(r))); + for (const table of [...tables].sort()) { + const out: WeightTable = {}; + for (const spec of [...specFiles].sort()) { + const seen = runs.map((r) => r[table]?.[spec]).filter((s) => s !== undefined); + if (seen.length) out[spec] = Math.round(median(seen)); + } + merged[table] = out; + } + return merged; +} + +export function serializeWeights(weights: Weights): string { + const sorted: Weights = {}; + for (const table of Object.keys(weights).sort()) { + sorted[table] = Object.fromEntries( + Object.entries(weights[table]).sort(([a], [b]) => a.localeCompare(b)), + ); + } + return JSON.stringify(sorted, null, 2) + '\n'; +} + +/** + * Packs `specFiles` into `shardCount` shards exactly as split-tests.js does: + * heaviest first into the currently lightest shard, unknown files at the + * median of the known ones. Returns the files of each shard. + */ +export function packShards( + table: WeightTable, + specFiles: string[], + shardCount: number, +): string[][] { + const known = specFiles.filter((f) => table[f] !== undefined).map((f) => table[f]); + const fallback = known.length + ? [...known].sort((a, b) => a - b)[Math.floor(known.length / 2)] + : 0; + const weightOf = (f: string) => table[f] ?? fallback; + const buckets = Array.from({ length: shardCount }, () => ({ total: 0, files: [] as string[] })); + const ordered = [...specFiles].sort((a, b) => weightOf(b) - weightOf(a) || a.localeCompare(b)); + for (const file of ordered) { + let target = buckets[0]; + for (const bucket of buckets) if (bucket.total < target.total) target = bucket; + target.files.push(file); + target.total += weightOf(file); + } + return buckets.map((b) => b.files); +} + +/** Seconds the longest shard would take, packed by `packedBy`, timed by `actual`. */ +export function longestShard( + packedBy: WeightTable, + actual: WeightTable, + specFiles: string[], + shardCount: number, +): number { + return Math.max( + ...packShards(packedBy, specFiles, shardCount).map((files) => + files.reduce((sum, f) => sum + (actual[f] ?? 0), 0), + ), + ); +} + +// Mirrors pickTable() in electron/electron's script/split-tests.js: the table +// CI actually packs a job from when the file has no table under its own key - +// the nearest of the same build type (MAS falls back to darwin), same arch +// first, then the plainest; else a legacy per-platform table; else anything. +export function pickTable(all: Weights, key: string): WeightTable { + if (all[key]) return all[key]; + const [buildType, arch] = key.split('_'); + const family = buildType === 'mas' ? ['mas', 'darwin'] : [buildType]; + for (const type of family) { + const nearest = Object.keys(all) + .filter((k) => k.startsWith(`${type}_`)) + .sort( + (a, b) => + (a.split('_')[1] === arch ? 0 : 1) - (b.split('_')[1] === arch ? 0 : 1) || + a.split('_').length - b.split('_').length || + a.localeCompare(b), + )[0]; + if (nearest) return all[nearest]; + } + const legacy = { darwin: 'darwin', mas: 'darwin', linux: 'linux', win: 'win32' }[buildType]; + return all[legacy] ?? all.darwin_x64 ?? all.darwin ?? Object.values(all)[0] ?? {}; +} + +export interface TableChange { + table: string; + shardCount: number; + /** Longest shard in seconds if CI keeps packing with the committed table. */ + before: number; + /** Longest shard in seconds packed with the fresh table. */ + after: number; + /** Spec files on the branch with no committed weight. */ + unweighted: string[]; + /** Files whose weight moved by at least 25% and 30 seconds. */ + moved: { spec: string; from: number; to: number }[]; +} + +export interface Materiality { + material: boolean; + reasons: string[]; + changes: TableChange[]; +} + +// Thresholds below which a refresh is not worth a PR: the longest shard of +// some platform must get at least a minute shorter, or a spec file must be +// missing from the table, or a file's weight must have moved substantially. +const MIN_SHARD_GAIN_SECONDS = 60; +const MOVED_RATIO = 0.25; +const MOVED_SECONDS = 30; + +/** + * Decides whether `fresh` is different enough from `committed` to be worth a + * PR, treating `fresh` as the truth about how long files take today. + */ +export function assessChange(committed: Weights, fresh: Weights, specFiles: string[]): Materiality { + const reasons: string[] = []; + const changes: TableChange[] = []; + for (const table of Object.keys(fresh).sort()) { + const shardCount = shardCountOf(table); + const actual = fresh[table]; + const old = pickTable(committed, table); + const before = longestShard(old, actual, specFiles, shardCount); + const after = longestShard(actual, actual, specFiles, shardCount); + const unweighted = specFiles.filter((f) => old[f] === undefined); + const moved = specFiles + .filter((f) => old[f] !== undefined && actual[f] !== undefined) + .map((f) => ({ spec: f, from: old[f], to: actual[f] })) + .filter( + ({ from, to }) => + Math.abs(to - from) >= MOVED_SECONDS && + Math.abs(to - from) >= MOVED_RATIO * Math.max(from, 1), + ); + changes.push({ table, shardCount, before, after, unweighted, moved }); + + if (!committed[table]) reasons.push(`${table}: no committed table`); + if (before - after >= MIN_SHARD_GAIN_SECONDS) { + reasons.push(`${table}: longest shard ${Math.round(before)}s -> ${Math.round(after)}s`); + } + if (unweighted.length) { + reasons.push(`${table}: ${unweighted.length} spec file(s) without a weight`); + } + for (const { spec, from, to } of moved) reasons.push(`${table}: ${spec} ${from}s -> ${to}s`); + } + return { material: reasons.length > 0, reasons, changes }; +} + +/** + * Reads every spec-timings.json out of a test_artifacts_* zip. Artifact zips + * from actions/upload-artifact carry sizes in the central directory, so the + * entries are located from there rather than from the local headers. + */ +export function timingsFromArtifactZip(zip: Buffer): SpecTimings[] { + const found: SpecTimings[] = []; + const eocd = zip.lastIndexOf(Buffer.from([0x50, 0x4b, 0x05, 0x06])); + if (eocd < 0) throw new Error('not a zip file: no end-of-central-directory record'); + const entries = zip.readUInt16LE(eocd + 10); + let offset = zip.readUInt32LE(eocd + 16); + for (let i = 0; i < entries; i++) { + if (zip.readUInt32LE(offset) !== 0x02014b50) throw new Error('corrupt zip central directory'); + const method = zip.readUInt16LE(offset + 10); + const compressedSize = zip.readUInt32LE(offset + 20); + const nameLength = zip.readUInt16LE(offset + 28); + const extraLength = zip.readUInt16LE(offset + 30); + const commentLength = zip.readUInt16LE(offset + 32); + const localHeader = zip.readUInt32LE(offset + 42); + const name = zip.toString('utf8', offset + 46, offset + 46 + nameLength); + offset += 46 + nameLength + extraLength + commentLength; + if (!name.endsWith('spec-timings.json')) continue; + + const localNameLength = zip.readUInt16LE(localHeader + 26); + const localExtraLength = zip.readUInt16LE(localHeader + 28); + const dataStart = localHeader + 30 + localNameLength + localExtraLength; + const data = zip.subarray(dataStart, dataStart + compressedSize); + const raw = method === 0 ? data : method === 8 ? inflateRawSync(data) : null; + if (!raw) throw new Error(`unsupported zip compression method ${method} for ${name}`); + found.push(JSON.parse(raw.toString('utf8'))); + } + return found; +} diff --git a/tests/fixtures/spec-weights/test_artifacts_darwin_x64_1.zip b/tests/fixtures/spec-weights/test_artifacts_darwin_x64_1.zip new file mode 100644 index 0000000000000000000000000000000000000000..1b20492690c5a79047a281782a1f3bda5d7f443e GIT binary patch literal 419 zcmWIWW@Zs#0D&kajo8ZFu|m8+HV6vP)i^q1A`!tE-pw-)-B1*&CE+L*2^l+&&xgM$k(hOzl~TsKGWrd9?w;m3Eaw`4BbcKZKkqtDZWrwOJjlwIS0GEb1R?vnqc<#IK@J?`rK zST3QE@a_Q1*K>2tv`o3#dJeAq#^Wb$;1=EE=M=KfL$|lnFsf+c|BdlhEVl1$lBX}u z-!qpfDn|3|mW$yBU#`{b-)DNuRBX>3*=M5mPAg|^d|%$|9TRiiKA?Hy-*3i?n?hGy zyZnnOz?+dtgc)~;0DT4q4M1;Vh7UqF_CSH?WngGvv<5OUf+)b7l?|kX2?%R}v;f!= E0H@7?82|tP literal 0 HcmV?d00001 diff --git a/tests/utils/roll-spec-weights.spec.ts b/tests/utils/roll-spec-weights.spec.ts new file mode 100644 index 0000000..8405456 --- /dev/null +++ b/tests/utils/roll-spec-weights.spec.ts @@ -0,0 +1,154 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { getOctokit } from '../../src/utils/octokit.js'; +import { + getSpecWeightsPRText, + rollSpecWeights, + type SpecWeightsRefresh, +} from '../../src/utils/roll-spec-weights.js'; +import { BACKPORT_CHECK_SKIP, NO_BACKPORT, ROLLER_BOT_LOGIN } from '../../src/constants.js'; + +vi.mock('../../src/utils/octokit.js'); + +const refresh: SpecWeightsRefresh = { + branch: '45-x-y', + runIds: [1, 2, 3], + committed: { darwin: { 'spec/a-spec.ts': 5 } }, + fresh: { darwin: { 'spec/a-spec.ts': 400 } }, + content: '{\n "darwin": {\n "spec/a-spec.ts": 400\n }\n}\n', + assessment: { + material: true, + reasons: ['darwin: longest shard 700s -> 600s'], + changes: [ + { + table: 'darwin', + shardCount: 3, + before: 700, + after: 600, + unweighted: [], + moved: [{ spec: 'spec/a-spec.ts', from: 5, to: 400 }], + }, + ], + }, +}; +const branch = { name: '45-x-y', commit: { sha: 'abc123' } }; +const encoded = (s: string) => ({ + data: { content: Buffer.from(s).toString('base64'), sha: 'f1' }, +}); + +describe('rollSpecWeights()', () => { + let mockOctokit: any; + + beforeEach(() => { + mockOctokit = { + paginate: vi.fn().mockResolvedValue([]), + pulls: { + update: vi.fn(), + create: vi.fn().mockResolvedValue({ data: { number: 7, html_url: 'https://x' } }), + }, + git: { + getRef: vi.fn().mockRejectedValue({ status: 404 }), + deleteRef: vi.fn(), + createRef: vi.fn(), + }, + repos: { + getContent: vi.fn().mockResolvedValue(encoded('{}')), + createOrUpdateFileContents: vi.fn(), + }, + issues: { addLabels: vi.fn(), listLabelsOnIssue: vi.fn().mockResolvedValue({ data: [] }) }, + }; + vi.mocked(getOctokit).mockResolvedValue(mockOctokit); + }); + + it('creates a roll branch, commits the weights, and opens a labelled PR', async () => { + expect(await rollSpecWeights(branch, refresh)).toBe(true); + + expect(mockOctokit.git.createRef).toHaveBeenCalledWith( + expect.objectContaining({ ref: 'refs/heads/roller/spec-weights/45-x-y', sha: 'abc123' }), + ); + expect(mockOctokit.repos.createOrUpdateFileContents).toHaveBeenCalledWith( + expect.objectContaining({ + path: 'script/spec-weights.json', + branch: 'roller/spec-weights/45-x-y', + content: Buffer.from(refresh.content).toString('base64'), + }), + ); + expect(mockOctokit.pulls.create).toHaveBeenCalledWith( + expect.objectContaining({ + base: '45-x-y', + head: 'electron:roller/spec-weights/45-x-y', + title: 'build: refresh spec weights (45-x-y)', + }), + ); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ issue_number: 7, labels: ['semver/none', BACKPORT_CHECK_SKIP] }), + ); + }); + + it('labels a main refresh no-backport', async () => { + await rollSpecWeights({ name: 'main', commit: { sha: 'm' } }, { ...refresh, branch: 'main' }); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ labels: ['semver/none', NO_BACKPORT] }), + ); + }); + + const ownPr = (overrides = {}) => ({ + number: 9, + title: 'build: refresh spec weights (45-x-y)', + user: { login: ROLLER_BOT_LOGIN }, + head: { repo: { full_name: 'electron/electron' }, ref: 'roller/spec-weights/45-x-y' }, + labels: [], + ...overrides, + }); + + it('updates its own open PR instead of opening another', async () => { + mockOctokit.paginate.mockResolvedValue([ownPr()]); + mockOctokit.repos.getContent.mockResolvedValue(encoded('{"darwin":{}}')); + + expect(await rollSpecWeights(branch, refresh)).toBe(true); + expect(mockOctokit.pulls.create).not.toHaveBeenCalled(); + expect(mockOctokit.git.createRef).not.toHaveBeenCalled(); + expect(mockOctokit.repos.createOrUpdateFileContents).toHaveBeenCalledWith( + expect.objectContaining({ branch: 'roller/spec-weights/45-x-y', sha: 'f1' }), + ); + expect(mockOctokit.pulls.update).toHaveBeenCalledWith( + expect.objectContaining({ pull_number: 9 }), + ); + }); + + it('leaves an open PR alone when it already carries these weights', async () => { + mockOctokit.paginate.mockResolvedValue([ownPr()]); + mockOctokit.repos.getContent.mockResolvedValue(encoded(refresh.content)); + + expect(await rollSpecWeights(branch, refresh)).toBe(false); + expect(mockOctokit.repos.createOrUpdateFileContents).not.toHaveBeenCalled(); + }); + + it('respects roller/pause', async () => { + mockOctokit.paginate.mockResolvedValue([ownPr({ labels: [{ name: 'roller/pause' }] })]); + expect(await rollSpecWeights(branch, refresh)).toBe(false); + expect(mockOctokit.repos.createOrUpdateFileContents).not.toHaveBeenCalled(); + }); + + it('never writes to a PR it did not author, even with the right title and branch name', async () => { + mockOctokit.paginate.mockResolvedValue([ + ownPr({ user: { login: 'someone-else' } }), + ownPr({ head: { repo: { full_name: 'fork/electron' }, ref: 'roller/spec-weights/45-x-y' } }), + ]); + await rollSpecWeights(branch, refresh); + // Neither matched, so it fell through to creating its own PR. + expect(mockOctokit.pulls.update).not.toHaveBeenCalled(); + expect(mockOctokit.pulls.create).toHaveBeenCalledTimes(1); + }); +}); + +describe('getSpecWeightsPRText()', () => { + it('links the sampled runs, tabulates the shard change, and ends with a Notes line', () => { + const { title, body } = getSpecWeightsPRText(refresh); + expect(title).toBe('build: refresh spec weights (45-x-y)'); + expect(body).toContain('[1](https://github.com/electron/electron/actions/runs/1)'); + expect(body).toContain('| `darwin` | 3 | 11.7 min | 10.0 min | 0 |'); + expect(body).toContain('`darwin` spec/a-spec.ts: 5s -> 400s'); + expect(body.trim().endsWith('Notes: none')).toBe(true); + }); +}); diff --git a/tests/utils/spec-weights.spec.ts b/tests/utils/spec-weights.spec.ts new file mode 100644 index 0000000..06d3026 --- /dev/null +++ b/tests/utils/spec-weights.spec.ts @@ -0,0 +1,236 @@ +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +import { + aggregateRun, + assessChange, + jobKeyOfArtifact, + longestShard, + pickTable, + mergeRuns, + packShards, + serializeWeights, + shardCountOf, + timingsFromArtifactZip, +} from '../../src/utils/spec-weights.js'; + +const fixtures = join(dirname(fileURLToPath(import.meta.url)), '../fixtures/spec-weights'); + +describe('spec-weights', () => { + describe('timingsFromArtifactZip()', () => { + it('reads spec-timings.json out of a test_artifacts zip and ignores the rest', () => { + const zip = readFileSync(join(fixtures, 'test_artifacts_darwin_x64_1.zip')); + const timings = timingsFromArtifactZip(zip); + expect(timings).toHaveLength(1); + expect(timings[0]).toMatchObject({ platform: 'darwin', arch: 'x64', mas: false }); + expect(timings[0].files['spec/api-browser-window-spec.ts']).toBeCloseTo(364.971, 3); + }); + + it('rejects something that is not a zip', () => { + expect(() => timingsFromArtifactZip(Buffer.from('nope'))).toThrow(/not a zip/); + }); + }); + + describe('jobKeyOfArtifact()', () => { + it('maps the plain and sanitizer test artifacts to their job key', () => { + expect(jobKeyOfArtifact('test_artifacts_darwin_x64_1')).toBe('darwin_x64'); + expect(jobKeyOfArtifact('test_artifacts_mas_arm64_2')).toBe('mas_arm64'); + expect(jobKeyOfArtifact('test_artifacts_linux_x64_x11_3')).toBe('linux_x64'); + expect(jobKeyOfArtifact('test_artifacts_linux_x64_asan_x11_1')).toBe('linux_x64_asan'); + expect(jobKeyOfArtifact('test_artifacts_linux_arm64_x11_2')).toBe('linux_arm64'); + expect(jobKeyOfArtifact('test_artifacts_darwin_arm64_ubsan_3')).toBe('darwin_arm64_ubsan'); + expect(jobKeyOfArtifact('test_artifacts_win_arm64_2')).toBe('win_arm64'); + }); + + it('ignores the Wayland allowlist job and non-test artifacts', () => { + expect(jobKeyOfArtifact('test_artifacts_linux_x64_wayland_1')).toBeNull(); + expect(jobKeyOfArtifact('generated_artifacts_darwin_x64')).toBeNull(); + expect(jobKeyOfArtifact('test_artifacts_darwin_x64')).toBeNull(); + }); + }); + + describe('pickTable()', () => { + const all = { + darwin_x64: { a: 1 }, + darwin_arm64: { a: 2 }, + mas_x64: { a: 3 }, + linux_x64: { a: 4 }, + linux_x64_asan: { a: 5 }, + }; + + it("uses the job's own table when there is one", () => { + expect(pickTable(all, 'linux_x64_asan')).toBe(all.linux_x64_asan); + }); + + it('falls back to the nearest table of the same build type, same arch first', () => { + expect(pickTable(all, 'darwin_arm64_ubsan')).toBe(all.darwin_arm64); + expect(pickTable(all, 'mas_arm64')).toBe(all.mas_x64); + expect(pickTable(all, 'linux_arm64')).toBe(all.linux_x64); + }); + + it('falls back from mas to darwin, and to a legacy per-platform table', () => { + expect(pickTable({ darwin_arm64: { a: 9 } }, 'mas_x64')).toEqual({ a: 9 }); + const legacy = { darwin: { a: 1 }, linux: { a: 2 }, win32: { a: 3 } }; + expect(pickTable(legacy, 'mas_x64')).toBe(legacy.darwin); + expect(pickTable(legacy, 'linux_x64_asan')).toBe(legacy.linux); + expect(pickTable(legacy, 'win_arm64')).toBe(legacy.win32); + expect(pickTable({}, 'win_x64')).toEqual({}); + }); + }); + + describe('shardCountOf()', () => { + it('matches the CI matrix', () => { + expect(shardCountOf('darwin_x64')).toBe(3); + expect(shardCountOf('mas_x64')).toBe(3); + expect(shardCountOf('darwin_arm64')).toBe(2); + expect(shardCountOf('mas_arm64')).toBe(2); + expect(shardCountOf('darwin_arm64_ubsan')).toBe(3); + expect(shardCountOf('linux_x64')).toBe(3); + expect(shardCountOf('linux_arm64')).toBe(3); + expect(shardCountOf('linux_x64_asan')).toBe(3); + expect(shardCountOf('win_x64')).toBe(2); + expect(shardCountOf('win_arm64')).toBe(2); + }); + }); + + describe('aggregateRun()', () => { + it("keeps one table per job and takes the max across a job's shards", () => { + const t = (files: Record) => ({ + platform: 'x', + arch: 'x', + mas: false, + files, + }); + const weights = aggregateRun([ + { key: 'darwin_x64', timings: t({ 'spec/a-spec.ts': 10 }) }, + { key: 'darwin_x64', timings: t({ 'spec/a-spec.ts': 14, 'spec/b-spec.ts': 1 }) }, + { key: 'darwin_arm64', timings: t({ 'spec/a-spec.ts': 6 }) }, + { key: 'linux_x64_asan', timings: t({ 'spec/a-spec.ts': 40 }) }, + ]); + expect(weights).toEqual({ + darwin_x64: { 'spec/a-spec.ts': 14, 'spec/b-spec.ts': 1 }, + darwin_arm64: { 'spec/a-spec.ts': 6 }, + linux_x64_asan: { 'spec/a-spec.ts': 40 }, + }); + }); + }); + + describe('mergeRuns()', () => { + const files = ['spec/a-spec.ts', 'spec/b-spec.ts', 'spec/new-spec.ts']; + + it('takes the median across runs so one slow run does not stick', () => { + const merged = mergeRuns( + [ + { darwin_x64: { 'spec/a-spec.ts': 10, 'spec/b-spec.ts': 4.4 } }, + { darwin_x64: { 'spec/a-spec.ts': 11, 'spec/b-spec.ts': 4.6 } }, + { darwin_x64: { 'spec/a-spec.ts': 95, 'spec/b-spec.ts': 4.5 } }, + ], + files, + ); + expect(merged.darwin_x64).toEqual({ 'spec/a-spec.ts': 11, 'spec/b-spec.ts': 5 }); + }); + + it('uses the runs that measured a file when some did not, and drops files gone from the tree', () => { + const merged = mergeRuns( + [ + { darwin_x64: { 'spec/a-spec.ts': 10, 'spec/gone-spec.ts': 99 } }, + { darwin_x64: { 'spec/a-spec.ts': 20, 'spec/b-spec.ts': 7 } }, + ], + files, + ); + expect(merged.darwin_x64).toEqual({ 'spec/a-spec.ts': 15, 'spec/b-spec.ts': 7 }); + }); + }); + + describe('packShards()', () => { + it('packs heaviest-first into the lightest shard, unknown files at the median', () => { + const table = { a: 30, b: 20, c: 10, d: 10 }; + const shards = packShards(table, ['a', 'b', 'c', 'd', 'e'], 2); + // e is unknown -> median of [10,10,20,30] = 20 (upper middle, as split-tests.js) + expect(shards).toEqual([ + ['a', 'c', 'd'], + ['b', 'e'], + ]); + expect(longestShard(table, table, ['a', 'b', 'c', 'd'], 2)).toBe(40); + }); + }); + + describe('assessChange()', () => { + const files = ['spec/a-spec.ts', 'spec/b-spec.ts', 'spec/c-spec.ts']; + + it('is not material when the shards already pack within a minute of optimal', () => { + const committed = { + darwin_x64: { 'spec/a-spec.ts': 100, 'spec/b-spec.ts': 100, 'spec/c-spec.ts': 100 }, + }; + const fresh = { + darwin_x64: { 'spec/a-spec.ts': 105, 'spec/b-spec.ts': 98, 'spec/c-spec.ts': 101 }, + }; + const result = assessChange(committed, fresh, files); + expect(result.material).toBe(false); + expect(result.changes[0]).toMatchObject({ + table: 'darwin_x64', + shardCount: 3, + unweighted: [], + }); + }); + + it('is material when the committed table leaves a shard over a minute too long', () => { + // win_x64 runs 2 shards. Committed thinks a is tiny, so it pairs a with b + // (300 + 5) against c (300); a is really 400s, so that shard runs 700s + // where a fresh packing would put a alone and reach 600s. + const committed = { + win_x64: { 'spec/a-spec.ts': 5, 'spec/b-spec.ts': 300, 'spec/c-spec.ts': 300 }, + }; + const fresh = { + win_x64: { 'spec/a-spec.ts': 400, 'spec/b-spec.ts': 300, 'spec/c-spec.ts': 300 }, + }; + const result = assessChange(committed, fresh, files); + expect(result.material).toBe(true); + expect(result.reasons).toContain('win_x64: longest shard 700s -> 600s'); + expect(result.reasons).toContain('win_x64: spec/a-spec.ts 5s -> 400s'); + }); + + it('is material when a spec file has no committed weight or a table is new', () => { + const committed = { darwin_x64: { 'spec/a-spec.ts': 10, 'spec/b-spec.ts': 10 } }; + const fresh = { + darwin_x64: { 'spec/a-spec.ts': 10, 'spec/b-spec.ts': 10, 'spec/c-spec.ts': 10 }, + mas_x64: { 'spec/a-spec.ts': 10, 'spec/b-spec.ts': 10, 'spec/c-spec.ts': 10 }, + win_x64: { 'spec/a-spec.ts': 10, 'spec/b-spec.ts': 10, 'spec/c-spec.ts': 10 }, + }; + const result = assessChange(committed, fresh, files); + // Both new jobs pack from darwin_x64 today (mas by family, win as the + // last resort), so they inherit that table's one gap. + expect(result.reasons).toEqual([ + 'darwin_x64: 1 spec file(s) without a weight', + 'mas_x64: no committed table', + 'mas_x64: 1 spec file(s) without a weight', + 'win_x64: no committed table', + 'win_x64: 1 spec file(s) without a weight', + ]); + }); + + it('ignores small moves on small files', () => { + const committed = { + win_x64: { 'spec/a-spec.ts': 4, 'spec/b-spec.ts': 100, 'spec/c-spec.ts': 100 }, + }; + const fresh = { + win_x64: { 'spec/a-spec.ts': 12, 'spec/b-spec.ts': 100, 'spec/c-spec.ts': 100 }, + }; + expect(assessChange(committed, fresh, files).material).toBe(false); + }); + }); + + describe('serializeWeights()', () => { + it('writes sorted tables and files with a trailing newline, like gen-spec-weights.js', () => { + const out = serializeWeights({ + mas_x64: { 'spec/b-spec.ts': 1, 'spec/a-spec.ts': 2 }, + darwin_x64: {}, + }); + expect(out).toBe( + '{\n "darwin_x64": {},\n "mas_x64": {\n "spec/a-spec.ts": 2,\n "spec/b-spec.ts": 1\n }\n}\n', + ); + }); + }); +}); From 52c286038ddbfb3bb2adb7146f3aba6e97c32bf9 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 8 Sep 2026 05:00:52 -0400 Subject: [PATCH 2/2] fix: isolate a bad run and settle a deleted ref before recreating it A sampled run whose artifacts cannot be read (a corrupt upload, an unsupported zip) or have expired now drops out of the median instead of failing the whole branch, and the PR body lists only the runs whose timings were used. An unparseable committed spec-weights.json is treated as empty rather than aborting the refresh. Recreating an orphan roll branch waits two seconds after deleting it, as roll.ts does, since ref deletion is eventually consistent. --- src/utils/roll-spec-weights.ts | 34 ++++++++++++++++--- tests/utils/roll-spec-weights.spec.ts | 49 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/utils/roll-spec-weights.ts b/src/utils/roll-spec-weights.ts index ffeb11c..a679d11 100644 --- a/src/utils/roll-spec-weights.ts +++ b/src/utils/roll-spec-weights.ts @@ -80,6 +80,7 @@ export async function fetchRunTimings(octokit: Octokit, runId: number): Promise< export interface SpecWeightsRefresh { branch: string; + /** The sampled runs whose timings actually went into `fresh`. */ runIds: number[]; committed: Weights; fresh: Weights; @@ -103,11 +104,24 @@ export async function computeRefresh( return null; } + // A run whose artifacts have expired, or one with a corrupt upload, drops + // out of the median rather than failing the branch: the others still count. const runs: Weights[] = []; + const usedRunIds: number[] = []; for (const runId of runIds) { - const jobs = await fetchRunTimings(octokit, runId); - if (jobs.length) runs.push(aggregateRun(jobs)); - else d(`run ${runId} has no timing artifacts (expired?) - ignoring`); + let jobs: JobTimings[]; + try { + jobs = await fetchRunTimings(octokit, runId); + } catch (e) { + d(`run ${runId}: could not read its timing artifacts (${e.message}) - ignoring`); + continue; + } + if (!jobs.length) { + d(`run ${runId} has no timing artifacts (expired?) - ignoring`); + continue; + } + runs.push(aggregateRun(jobs)); + usedRunIds.push(runId); } if (!runs.length) { d('no timing data in any sampled run - skipping'); @@ -122,11 +136,19 @@ export async function computeRefresh( path: SPEC_WEIGHTS.filePath, ref: branch, }); - const committed: Weights = existing ? JSON.parse(existing.content) : {}; + let committed: Weights = {}; + if (existing) { + try { + committed = JSON.parse(existing.content); + } catch (e) { + // Treat an unparseable file as no weights at all: the refresh replaces it. + d(`committed ${SPEC_WEIGHTS.filePath} is not valid JSON (${e.message}) - treating as empty`); + } + } return { branch, - runIds, + runIds: usedRunIds, committed, fresh, content: serializeWeights(fresh), @@ -241,6 +263,8 @@ export async function rollSpecWeights( await octokit.git.getRef({ ...REPOS.electron, ref: shortRef }); d(`Found orphan ref ${ref} with no open PR - deleting`); await octokit.git.deleteRef({ ...REPOS.electron, ref: shortRef }); + // Ref deletion is eventually consistent; recreating it at once can fail. + await new Promise((r) => setTimeout(r, 2000)); } catch { // No orphan ref. } diff --git a/tests/utils/roll-spec-weights.spec.ts b/tests/utils/roll-spec-weights.spec.ts index 8405456..f3e9064 100644 --- a/tests/utils/roll-spec-weights.spec.ts +++ b/tests/utils/roll-spec-weights.spec.ts @@ -2,11 +2,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getOctokit } from '../../src/utils/octokit.js'; import { + computeRefresh, getSpecWeightsPRText, rollSpecWeights, type SpecWeightsRefresh, } from '../../src/utils/roll-spec-weights.js'; import { BACKPORT_CHECK_SKIP, NO_BACKPORT, ROLLER_BOT_LOGIN } from '../../src/constants.js'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const fixtureZip = readFileSync( + join( + dirname(fileURLToPath(import.meta.url)), + '../fixtures/spec-weights/test_artifacts_darwin_x64_1.zip', + ), +); vi.mock('../../src/utils/octokit.js'); @@ -152,3 +163,41 @@ describe('getSpecWeightsPRText()', () => { expect(body.trim().endsWith('Notes: none')).toBe(true); }); }); + +describe('computeRefresh()', () => { + it('drops a run whose artifacts are unreadable or gone and reports only the runs it used', async () => { + const zipOf = (id: number) => (id === 2 ? Buffer.from('not a zip') : fixtureZip); + const octokit: any = { + actions: { + listWorkflowRuns: vi + .fn() + .mockResolvedValue({ data: { workflow_runs: [{ id: 1 }, { id: 2 }, { id: 3 }] } }), + listWorkflowRunArtifacts: vi.fn(), + downloadArtifact: vi + .fn() + .mockImplementation(({ artifact_id }) => ({ data: zipOf(artifact_id) })), + }, + paginate: vi + .fn() + .mockImplementation((_fn, { run_id }) => + run_id === 3 ? [] : [{ id: run_id, name: 'test_artifacts_darwin_x64_1', expired: false }], + ), + git: { + getTree: vi.fn().mockResolvedValue({ + data: { tree: [{ type: 'blob', path: 'api-browser-window-spec.ts' }] }, + }), + }, + // The committed file is unparseable here too; that must not fail the branch. + repos: { + getContent: vi + .fn() + .mockResolvedValue({ data: { content: Buffer.from('{').toString('base64'), sha: 's' } }), + }, + }; + + const refresh = await computeRefresh(octokit, 'main'); + expect(refresh?.runIds).toEqual([1]); + expect(refresh?.committed).toEqual({}); + expect(refresh?.fresh.darwin_x64['spec/api-browser-window-spec.ts']).toBe(365); + }); +});