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..4b4fe59 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,42 @@ 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`); + 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); + 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..e66cb5c 100644 --- a/test/main.test.ts +++ b/test/main.test.ts @@ -1,11 +1,14 @@ -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, + baseWorktreeExists: undefined as boolean | undefined, })); vi.mock('random-seed', () => ({ @@ -23,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", @@ -41,17 +46,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 +163,69 @@ 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"; + 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" })); + } + 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); + + 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 }); + } + }, + ); + + 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,