From 7f9df65732fca6fffec96ab1d4eb165d2e43337a Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:38:06 -0700 Subject: [PATCH 1/2] Compare PR deltas against the merge commit base A PR merge ref can lag behind main, so using a fresh main checkout as the baseline reports unrelated fixes as regressions. Tie the baseline to the merge commit to keep both sides of each comparison consistent. --- README.md | 3 ++ azure-pipelines-userTests.yml | 2 +- src/checkReposTriggered.ts | 3 +- src/main.ts | 34 +++++++++---------- test/main.test.ts | 61 ++++++++++++++++++++++++++++++++--- 5 files changed, 79 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index b3aa49f..b4bfa38 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ Download and compile popular open source repos in order to compare new versions of the TypeScript compiler with the current version. For example, this project will clone the prettier repo and compile it with the current version of TypeScript. Then it will compile it with a version of TypeScript from a pull request. +For pull requests, the baseline is the first parent of the PR merge commit, not the current `main` branch. Afterward it will compile new errors that are issued only with the new version and post them as a comment on the pull request. There is no comparison of types, errors, symbols or language service output. @@ -25,6 +26,8 @@ node dist/checkReposTriggered.js ` argument is retained for compatibility but ignored; both builds use the same PR merge snapshot. + You can view example usage of these commands from how they're currently triggered on Azure Pipelines: * [New Error Detector](https://github.com/microsoft/typescript-error-deltas/blob/main/azure-pipelines-gitTests.yml) diff --git a/azure-pipelines-userTests.yml b/azure-pipelines-userTests.yml index 7afdc94..5a45b99 100644 --- a/azure-pipelines-userTests.yml +++ b/azure-pipelines-userTests.yml @@ -20,7 +20,7 @@ parameters: type: string default: https://github.com/microsoft/TypeScript.git - name: OLD_HEAD_REF - displayName: Old head reference + displayName: Legacy baseline ref (ignored; PR baseline is the merge commit's first parent) type: string default: main - name: DISTINCT_ID diff --git a/src/checkReposTriggered.ts b/src/checkReposTriggered.ts index 9c3f0ac..0a1b012 100644 --- a/src/checkReposTriggered.ts +++ b/src/checkReposTriggered.ts @@ -8,14 +8,13 @@ if (argv.length < 13) { process.exit(-1); } -const [,, entrypoint, oldTsRepoUrl, oldHeadRef, prNumber, buildWithNewWhenOldFails, repoListPath, workerCount, workerNumber, resultDirName, diagnosticOutput, prngSeed, useOverlayFs, expectedHeadSha, expectedBaseSha, expectedMergeSha] = argv; +const [,, entrypoint, oldTsRepoUrl, , prNumber, buildWithNewWhenOldFails, repoListPath, workerCount, workerNumber, resultDirName, diagnosticOutput, prngSeed, useOverlayFs, expectedHeadSha, expectedBaseSha, expectedMergeSha] = argv; mainAsync({ testType: "triggered", useOverlayFs: useOverlayFs && useOverlayFs.toLowerCase() === "false" ? false : true, entrypoint: entrypoint as TsEntrypoint, oldTsRepoUrl, - oldHeadRef, prNumber: +prNumber, buildWithNewWhenOldFails: buildWithNewWhenOldFails.toLowerCase() !== "true", repoListPath, diff --git a/src/main.ts b/src/main.ts index 3577b55..79f42f4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -69,7 +69,6 @@ interface PrSnapshot { export interface TriggeredParams extends Params { testType: "triggered"; oldTsRepoUrl: string; - oldHeadRef: string; prNumber: number; expectedPrSnapshot: PrSnapshot | undefined; } @@ -1294,9 +1293,9 @@ async function downloadTsAsync(cwd: string, params: ScheduledParams | TriggeredP if (params.entrypoint === "fuzzer") { throw new Error("Not implemented"); } - const { tsEntrypointPath: oldTsEntrypointPath, resolvedVersion: oldTsResolvedVersion, implementation: oldImplementation } = await downloadTsRepoAsync(cwd, params.oldTsRepoUrl, params.oldHeadRef, entrypoint); - // We need to handle the ref/pull/*/merge differently as it is not a branch and cannot be pulled during clone. - const { tsEntrypointPath: newTsEntrypointPath, resolvedVersion: newTsResolvedVersion, implementation } = await downloadTsPrAsync(cwd, params.oldTsRepoUrl, params.prNumber, entrypoint, params.expectedPrSnapshot); + const { baseline, candidate } = await downloadTsPrAsync(cwd, params.oldTsRepoUrl, params.prNumber, entrypoint, params.expectedPrSnapshot); + const { tsEntrypointPath: oldTsEntrypointPath, resolvedVersion: oldTsResolvedVersion, implementation: oldImplementation } = baseline; + const { tsEntrypointPath: newTsEntrypointPath, resolvedVersion: newTsResolvedVersion, implementation } = candidate; if (entrypoint === "tsserver" && oldImplementation !== implementation) { throw new Error("Cannot compare tsserver refs across the TypeScript-to-tsgo migration boundary"); @@ -1356,7 +1355,7 @@ export async function downloadTsRepoAsync(cwd: string, repoUrl: string, headRef: }; } -async function downloadTsPrAsync(cwd: string, repoUrl: string, prNumber: number, target: TsEntrypoint, expectedPrSnapshot: PrSnapshot | undefined): Promise { +export async function downloadTsPrAsync(cwd: string, repoUrl: string, prNumber: number, target: TsEntrypoint, expectedPrSnapshot: PrSnapshot | undefined): Promise<{ baseline: DownloadedTs, candidate: DownloadedTs }> { console.log(`Cloning ${repoUrl} at pull ${prNumber}`); const repoName = getTsRepoDownloadName(repoUrl, prNumber.toString()); @@ -1368,36 +1367,37 @@ async function downloadTsPrAsync(cwd: string, repoUrl: string, prNumber: number, const headRef = `refs/pull/${prNumber}/merge`; await git.checkout(repoPath, headRef); - await verifyPrSnapshot(repoPath, expectedPrSnapshot); - const { tsEntrypointPath, implementation } = await buildTs(repoPath, target); + const { baseSha } = await verifyPrSnapshot(repoPath, expectedPrSnapshot); + const basePath = path.resolve(cwd, `${repoName}-base`); + await execFileAsync(repoPath, "git", ["worktree", "add", "--detach", basePath, baseSha]); + await execFileAsync(basePath, "git", ["submodule", "update", "--init", "--recursive", "--depth=1"]); + const { tsEntrypointPath: oldTsEntrypointPath, implementation: oldImplementation } = await buildTs(basePath, target); + const { tsEntrypointPath: newTsEntrypointPath, implementation } = await buildTs(repoPath, target); return { - tsEntrypointPath, - resolvedVersion: headRef, - implementation + baseline: { tsEntrypointPath: oldTsEntrypointPath, resolvedVersion: baseSha, implementation: oldImplementation }, + candidate: { tsEntrypointPath: newTsEntrypointPath, resolvedVersion: headRef, implementation }, }; } -async function verifyPrSnapshot(repoPath: string, expected: PrSnapshot | undefined): Promise { - if (!expected) { - return; - } - if (!expected.headSha || !expected.baseSha || !expected.mergeSha) { +async function verifyPrSnapshot(repoPath: string, expected: PrSnapshot | undefined): Promise<{ baseSha: string }> { + if (expected && (!expected.headSha || !expected.baseSha || !expected.mergeSha)) { throw new Error("Expected PR snapshot must include head, base, and merge SHAs"); } const [actualMergeSha, actualBaseSha, actualHeadSha] = (await execFileAsync(repoPath, "git", ["rev-parse", "HEAD", "HEAD^1", "HEAD^2"])).trim().split(/\s+/); - if ( + if (expected && ( actualMergeSha !== expected.mergeSha || actualBaseSha !== expected.baseSha || actualHeadSha !== expected.headSha - ) { + )) { throw new Error( `PR snapshot changed: expected merge ${expected.mergeSha}, base ${expected.baseSha}, head ${expected.headSha}; ` + `got merge ${actualMergeSha}, base ${actualBaseSha}, head ${actualHeadSha}`, ); } + return { baseSha: actualBaseSha }; } export function detectTypeScriptImplementation(packageJson: { name?: string }): TypeScriptImplementation { diff --git a/test/main.test.ts b/test/main.test.ts index 3c63023..383e53e 100644 --- a/test/main.test.ts +++ b/test/main.test.ts @@ -1,11 +1,13 @@ -import { detectTypeScriptImplementation, detectTypeScriptNpmImplementation, downloadTsRepoAsync, mainAsync, reduceSpew } from '../src/main.js' +import { detectTypeScriptImplementation, detectTypeScriptNpmImplementation, downloadTsPrAsync, downloadTsRepoAsync, mainAsync, reduceSpew } from '../src/main.js' import * as path from "node:path" +import { execFileAsync } from '../src/utils/execUtils.js'; import type { SpawnResult } from '../src/utils/execUtils.js'; import { describe, expect, it, vi } from "vitest"; const testState = vi.hoisted(() => ({ typeScriptSpawnResult: undefined as ((args: readonly string[]) => SpawnResult) | undefined, writeFile: vi.fn(), + prRefs: undefined as string | undefined, })); vi.mock('random-seed', () => ({ @@ -41,17 +43,17 @@ vi.mock("../src/utils/execUtils", () => ({ return testState.typeScriptSpawnResult!(args); }), - execFileAsync: async (cwd: string, command: string, args: readonly string[] = []) => { + execFileAsync: vi.fn(async (cwd: string, command: string, args: readonly string[] = []) => { if (command === "npm" && args[0] === "pack" && args[1] === "typescript@latest") { return ' typescript-0.0.0.tgz'; } else if (command === "npm" && args[0] === "pack" && args[1] === "typescript@next") { return ' typescript-1.1.1.tgz'; } else if (command === "git" && args[0] === "rev-parse") { - return '57b462387e88aa7e363af0daf867a5dc1e83a935'; + return testState.prRefs ?? '57b462387e88aa7e363af0daf867a5dc1e83a935'; } return ''; - }, + }), execFileWithRetryAsync: vi.fn().mockResolvedValue(''), })); @@ -158,6 +160,57 @@ describe("main", () => { } }); + it.each([undefined, { mergeSha: 'merge', baseSha: 'base', headSha: 'head' }])( + "compares a PR merge with its first parent (expected snapshot: %s)", + async expectedPrSnapshot => { + const actualFs = await vi.importActual("node:fs"); + const repoPath = path.join("./testDownloads/main", "typescript-123"); + const basePath = path.resolve("./testDownloads/main", "typescript-123-base"); + testState.prRefs = "merge\nbase\nhead\n"; + for (const dir of [repoPath, basePath]) { + actualFs.mkdirSync(dir, { recursive: true }); + actualFs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "typescript" })); + } + vi.mocked(execFileAsync).mockClear(); + try { + const result = await downloadTsPrAsync("./testDownloads/main", "https://github.com/microsoft/TypeScript", 123, "tsc", expectedPrSnapshot); + expect(result.baseline).toEqual({ + tsEntrypointPath: path.join(basePath, "built/local/tsc.js"), + resolvedVersion: "base", + implementation: "strada", + }); + expect(result.candidate.tsEntrypointPath).toBe(path.join(repoPath, "built/local/tsc.js")); + expect(execFileAsync).toHaveBeenCalledWith(repoPath, "git", ["rev-parse", "HEAD", "HEAD^1", "HEAD^2"]); + expect(execFileAsync).toHaveBeenCalledWith(repoPath, "git", ["worktree", "add", "--detach", basePath, "base"]); + expect(execFileAsync).toHaveBeenCalledWith(basePath, "git", ["submodule", "update", "--init", "--recursive", "--depth=1"]); + expect(vi.mocked(execFileAsync).mock.calls.some(([, command, args]) => command === "git" && args?.includes("main"))).toBe(false); + } + finally { + testState.prRefs = undefined; + actualFs.rmSync(basePath, { recursive: true }); + actualFs.rmSync(repoPath, { recursive: true }); + } + }, + ); + + it("rejects a changed PR snapshot before building either compiler", async () => { + const actualFs = await vi.importActual("node:fs"); + const repoPath = "./testDownloads/main/typescript-123"; + actualFs.mkdirSync(repoPath, { recursive: true }); + testState.prRefs = "merge\nbase\nhead\n"; + vi.mocked(execFileAsync).mockClear(); + try { + await expect(downloadTsPrAsync("./testDownloads/main", "https://github.com/microsoft/TypeScript", 123, "tsc", { + mergeSha: "merge", baseSha: "different-base", headSha: "head", + })).rejects.toThrow("PR snapshot changed"); + expect(vi.mocked(execFileAsync).mock.calls.some(([, , args]) => args?.[0] === "worktree" || args?.[0] === "ci")).toBe(false); + } + finally { + testState.prRefs = undefined; + actualFs.rmSync(repoPath, { recursive: true }); + } + }); + it("outputs server errors", async () => { testState.typeScriptSpawnResult = () => ({ stdout: errorStdout, From 407828f264dfae26f9252e6fc276ccde35e403a8 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:15:09 -0700 Subject: [PATCH 2/2] Reuse the PR baseline worktree on repeat runs Triggered comparisons can run more than once in the same workspace. Readding an existing worktree aborts before either compiler is built, so reuse it at the merge commit base instead. --- src/main.ts | 7 ++++++- test/main.test.ts | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/main.ts b/src/main.ts index 79f42f4..4b4fe59 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1369,7 +1369,12 @@ export async function downloadTsPrAsync(cwd: string, repoUrl: string, prNumber: await git.checkout(repoPath, headRef); const { baseSha } = await verifyPrSnapshot(repoPath, expectedPrSnapshot); const basePath = path.resolve(cwd, `${repoName}-base`); - await execFileAsync(repoPath, "git", ["worktree", "add", "--detach", basePath, baseSha]); + if (await pu.exists(basePath)) { + await execFileAsync(basePath, "git", ["checkout", "--detach", baseSha]); + } + else { + await execFileAsync(repoPath, "git", ["worktree", "add", "--detach", basePath, baseSha]); + } await execFileAsync(basePath, "git", ["submodule", "update", "--init", "--recursive", "--depth=1"]); const { tsEntrypointPath: oldTsEntrypointPath, implementation: oldImplementation } = await buildTs(basePath, target); diff --git a/test/main.test.ts b/test/main.test.ts index 383e53e..e66cb5c 100644 --- a/test/main.test.ts +++ b/test/main.test.ts @@ -8,6 +8,7 @@ const testState = vi.hoisted(() => ({ typeScriptSpawnResult: undefined as ((args: readonly string[]) => SpawnResult) | undefined, writeFile: vi.fn(), prRefs: undefined as string | undefined, + baseWorktreeExists: undefined as boolean | undefined, })); vi.mock('random-seed', () => ({ @@ -25,7 +26,9 @@ vi.mock("../src/utils/packageUtils", async () => { const fs = await vi.importActual("node:fs"); return { exists: vi.fn((filePath: string) => filePath.includes("testDownloads") - ? fs.existsSync(filePath) + ? filePath.endsWith("typescript-123-base") && testState.baseWorktreeExists !== undefined + ? testState.baseWorktreeExists + : fs.existsSync(filePath) : Promise.resolve(true)), getMonorepoOrder: vi.fn().mockResolvedValue([ "./dirA/package.json", @@ -167,6 +170,7 @@ describe("main", () => { const repoPath = path.join("./testDownloads/main", "typescript-123"); const basePath = path.resolve("./testDownloads/main", "typescript-123-base"); testState.prRefs = "merge\nbase\nhead\n"; + testState.baseWorktreeExists = false; for (const dir of [repoPath, basePath]) { actualFs.mkdirSync(dir, { recursive: true }); actualFs.writeFileSync(path.join(dir, "package.json"), JSON.stringify({ name: "typescript" })); @@ -184,9 +188,20 @@ describe("main", () => { expect(execFileAsync).toHaveBeenCalledWith(repoPath, "git", ["worktree", "add", "--detach", basePath, "base"]); expect(execFileAsync).toHaveBeenCalledWith(basePath, "git", ["submodule", "update", "--init", "--recursive", "--depth=1"]); expect(vi.mocked(execFileAsync).mock.calls.some(([, command, args]) => command === "git" && args?.includes("main"))).toBe(false); + + testState.baseWorktreeExists = true; + testState.prRefs = "next-merge\nnext-base\nnext-head\n"; + vi.mocked(execFileAsync).mockClear(); + const next = await downloadTsPrAsync("./testDownloads/main", "https://github.com/microsoft/TypeScript", 123, "tsc", expectedPrSnapshot && { + mergeSha: "next-merge", baseSha: "next-base", headSha: "next-head", + }); + expect(next.baseline.resolvedVersion).toBe("next-base"); + expect(execFileAsync).toHaveBeenCalledWith(basePath, "git", ["checkout", "--detach", "next-base"]); + expect(vi.mocked(execFileAsync).mock.calls.some(([, , args]) => args?.[0] === "worktree")).toBe(false); } finally { testState.prRefs = undefined; + testState.baseWorktreeExists = undefined; actualFs.rmSync(basePath, { recursive: true }); actualFs.rmSync(repoPath, { recursive: true }); }