Skip to content

Commit

Permalink
Merge pull request microsoft#9 from microsoft/restore-git-tests
Browse files Browse the repository at this point in the history
Restore git tests
  • Loading branch information
sandersn authored May 18, 2022
2 parents f3a3b85 + ac844b8 commit f044986
Show file tree
Hide file tree
Showing 16 changed files with 464 additions and 56 deletions.
2 changes: 0 additions & 2 deletions .npmrc

This file was deleted.

5 changes: 1 addition & 4 deletions azure-pipelines-userTests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ pool:
vmImage: 'ubuntu-latest'

jobs:
- job: 'DetectNewErrors'
- job: 'UserTestInline'
timeoutInMinutes: 360
steps:
- task: NodeTool@0
inputs:
versionSpec: '12.x'
displayName: 'Install Node.js'
- task: npmAuthenticate@0
inputs:
workingFile: './.npmrc'
- script: |
npm ci
npm run build
Expand Down
5 changes: 1 addition & 4 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@ pool:
vmImage: 'ubuntu-latest'

jobs:
- job: 'DetectNewErrors'
- job: 'BuildAndTest'
timeoutInMinutes: 360
steps:
- task: NodeTool@0
inputs:
versionSpec: '16.x'
displayName: 'Install Node.js'
- task: npmAuthenticate@0
inputs:
workingFile: './.npmrc'
- script: |
npm ci
npm run build
Expand Down
3 changes: 3 additions & 0 deletions getErrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ describe("getErrors", () => {
"./test/simpleProject",
// TODO: Depends on downloading and building 44585 in main.test.ts
path.resolve("./typescript-test-fake-error/built/local/tsc.js"),
'user',
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand All @@ -24,6 +25,7 @@ describe("getErrors", () => {
"./test/scriptProject",
// TODO: Depends on downloading and building 44585 in main.test.ts
path.resolve("./typescript-test-fake-error/built/local/tsc.js"),
'user',
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand All @@ -41,6 +43,7 @@ describe("getErrors", () => {
"./test/scriptPrettier",
// TODO: Depends on downloading and building 44585 in main.test.ts
path.resolve("./typescript-test-fake-error/built/local/tsc.js"),
'user',
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand Down
12 changes: 8 additions & 4 deletions getErrors.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import ghUrl = require("./github-url/index");
import packageUtils = require("./packageUtils");
import projectGraph = require("./projectGraph");
import cp = require("child_process");
Expand Down Expand Up @@ -68,7 +69,10 @@ export interface RepoErrors {
* @param tscPath The path to tsc.js.
* @param skipLibCheck True pass --skipLibCheck when building non-composite projects. (Defaults to true)
*/
export async function buildAndGetErrors(repoDir: string, tscPath: string, skipLibCheck: boolean = true): Promise<RepoErrors> {
export async function buildAndGetErrors(repoDir: string, tscPath: string, testType: 'git' | 'user', skipLibCheck: boolean = true): Promise<RepoErrors> {
// TODO: Either add a new testType (passed in to here) or a new projectType (detected by projectGraph.getProjectsToBuild(repoDir)
// *probably* it makes more sense to add a testType, especially since the 'git' test type isn't needed here. It's from the fuzzer.
// (if we wanted to keep 'git', the 3 things aren't in one category)
const simpleBuildArgs = `--skipLibCheck ${skipLibCheck} --incremental false --pretty false -p`;
const compositeBuildArgs = `-b -f -v`; // Build mode doesn't support --skipLibCheck or --pretty

Expand All @@ -89,7 +93,7 @@ export async function buildAndGetErrors(repoDir: string, tscPath: string, skipLi
if (isEmpty) continue;

const projectDir = path.dirname(projectPath);
const projectUrl = projectPath; // Use project path for user tests as they don't contain a git project.
const projectUrl = testType === "user" ? projectPath : await ghUrl.getGithubUrl(projectPath); // Use project path for user tests as they don't contain a git project.

let localErrors: LocalError[] = [];
let currProjectUrl = projectUrl;
Expand All @@ -98,7 +102,7 @@ export async function buildAndGetErrors(repoDir: string, tscPath: string, skipLi
for (const line of lines) {
const projectMatch = isComposite && line.match(beginProjectRegex);
if (projectMatch) {
currProjectUrl = path.resolve(projectDir, projectMatch[1]);
currProjectUrl = testType === "user" ? path.resolve(projectDir, projectMatch[1]) : await ghUrl.getGithubUrl(path.resolve(projectDir, projectMatch[1]));
continue;
}
const localError = getLocalErrorFromLine(line, currProjectUrl);
Expand All @@ -110,7 +114,7 @@ export async function buildAndGetErrors(repoDir: string, tscPath: string, skipLi
const errors = localErrors.filter(le => !le.path).map(le => ({ projectUrl: le.projectUrl, code: le.code, text: le.text } as Error));

const fileLocalErrors = localErrors.filter(le => le.path).map(le => ({ ...le, path: path.resolve(projectDir, le.path!) }));
const fileUrls = fileLocalErrors.map(x => `${x.path}(${x.lineNumber},${x.columnNumber})`);
const fileUrls = testType === "user" ? fileLocalErrors.map(x => `${x.path}(${x.lineNumber},${x.columnNumber})`) : await ghUrl.getGithubUrls(fileLocalErrors);
for (let i = 0; i < fileLocalErrors.length; i++) {
const localError = fileLocalErrors[i];
errors.push({
Expand Down
21 changes: 21 additions & 0 deletions gitErrors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import path = require("path");
import { mainAsync, reportError } from "./main";

const { argv } = process;

if (argv.length !== 6) {
console.error(`Usage: ${path.basename(argv[0])} ${path.basename(argv[1])} <post_result> <repo_count> <old_tsc_version> <new_tsc_version>`);
process.exit(-1);
}

mainAsync({
testType: "git",
postResult: argv[2].toLowerCase() === "true", // Only accept true.
tmpfs: true,
repoCount: +argv[3],
oldTscVersion: argv[4],
newTscVersion: argv[5],
}).catch(err => {
reportError(err, "Unhandled exception");
process.exit(1);
});
90 changes: 85 additions & 5 deletions gitUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import octokit = require("@octokit/rest");
import utils = require("./packageUtils");
import git = require("simple-git/promise");
import fs = require("fs");
import path = require("path");
import cp = require("child_process");

Expand All @@ -17,6 +18,51 @@ const repoProperties = {
repo: "typescript",
};

export async function getPopularTypeScriptRepos(count = 100, cachePath?: string): Promise<readonly Repo[]> {
const cacheEncoding = { encoding: "utf-8" } as const;

if (cachePath && await utils.exists(cachePath)) {
const contents = await fs.promises.readFile(cachePath, cacheEncoding);
const cache: Repo[] = JSON.parse(contents);
if (cache.length >= count) {
return cache.slice(0, count);
}
}

const kit = new octokit.Octokit();
const perPage = Math.min(100, count);

let repos: Repo[] = [];
for (let page = 1; repos.length < count; page++) {
const response = await kit.search.repos({
q: "language:TypeScript+stars:>100 archived:no",
sort: "stars",
order: "desc",
per_page: perPage,
page,
});

if (response.status !== 200) throw response;

for (const repo of response.data.items) {
if (repo.full_name !== "microsoft/TypeScript" && repo.full_name !== "DefinitelyTyped/DefinitelyTyped") {
repos.push({ url: repo.html_url, name: repo.name, owner: repo.owner.login });
}
if (repos.length >= count) {
break;
}
}

if (!response.headers.link || !response.headers.link.includes('rel="next"')) break;
}

if (cachePath) {
await fs.promises.writeFile(cachePath, JSON.stringify(repos), cacheEncoding);
}

return repos;
}

export async function cloneRepoIfNecessary(parentDir: string, repo: Repo): Promise<void> {
if (!repo.url) {
throw new Error("Repo url cannot be `undefined`");
Expand All @@ -34,14 +80,48 @@ export async function cloneRepoIfNecessary(parentDir: string, repo: Repo): Promi
}
}

export type Result = {
type Result = {
body: string,
owner: string,
repo: string,
issue_number: number,
}
export type GitResult = Result & { kind: 'git', title: string }
export type UserResult = Result & { kind: 'user', issue_number: number }

export async function createIssue(postResult: boolean, title: string, body: string, sawNewErrors: boolean): Promise<GitResult | undefined> {
const issue = {
...repoProperties,
title,
body,
};

if (!postResult) {
console.log("Issue not posted: ");
console.log(JSON.stringify(issue));
return { kind: 'git', ...issue };
}

console.log("Creating a summary issue");

const kit = new octokit.Octokit({
auth: process.env.GITHUB_PAT,
});

const created = await kit.issues.create(issue);

const issueNumber = created.data.number;
console.log(`Created issue #${issueNumber}: ${created.data.html_url}`);

if (!sawNewErrors) {
await kit.issues.update({
...repoProperties,
issue_number: issueNumber,
state: "closed",
});
}
}

export async function createComment(sourceIssue: number, statusComment: number, postResult: boolean, body: string): Promise<Result | undefined> {
export async function createComment(sourceIssue: number, statusComment: number, postResult: boolean, body: string): Promise<UserResult | undefined> {
const newComment = {
...repoProperties,
issue_number: sourceIssue,
Expand All @@ -51,11 +131,11 @@ export async function createComment(sourceIssue: number, statusComment: number,
if (!postResult) {
console.log("Comment not posted: ");
console.log(JSON.stringify(newComment));
return newComment;
return { kind: 'user', ...newComment };
}

console.log("Creating a github comment");

const kit = new octokit.Octokit({
auth: process.env.GITHUB_PAT,
});
Expand Down
144 changes: 144 additions & 0 deletions github-url/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import cp = require("child_process");
import path = require("path");
import url = require("url");

if (require.main === module) {
const { argv } = process;

if (argv.length !== 3 && argv.length !== 4) {
console.error(`Usage: ${path.basename(argv[0])} ${path.basename(argv[1])} {file_path} [{line_number}]`);
process.exit(-1);
}

let localPath = argv[2];
let lineNumber = argv[3]; // May be undefined

if (!lineNumber) {
const lineNumberMatch = localPath.match(/[:,]([0-9]+)/);
if (lineNumberMatch) {
lineNumber = lineNumberMatch[1];
localPath = localPath.substr(0, lineNumberMatch.index);
}
}

getGithubUrl(localPath, !!lineNumber ? +lineNumber : undefined).then(
url => {
console.log(url);
},
err => {
console.error(err.message);
process.exit(-2);
});
}

export interface SourceLocation {
path: string;
lineNumber?: number;
}

/**
* Returns a GitHub URL (or a file URL, if the file is untracked).
* Throws if no appropriate remote can be identified.
*/
export async function getGithubUrl(path: string, lineNumber?: number): Promise<string> {
return (await getGithubUrlWorker([{ path, lineNumber }]))[0];
}

/**
* Returns a GitHub URL (or a file URL, if untracked) for each location.
* Throws if no appropriate remote can be identified.
*/
export async function getGithubUrls(locations: readonly SourceLocation[]): Promise<string[]> {
return await getGithubUrlWorker(locations);
}

async function getGithubUrlWorker(locations: readonly SourceLocation[]): Promise<string[]> {
if (!locations.length) {
return [];
}

const cwd = path.dirname(locations[0].path);

// This is a clever way to quickly retrieve the current commit
const commit = await getExecOutput("git", ["rev-parse", "@"], cwd);
if (!commit) {
throw new Error(`Couldn't identify commit - not a repository?`);
}

const preferredRemote = await getPreferredRemote(cwd, commit);
if (!preferredRemote) {
throw new Error(`Commit ${commit} is not present on any remote`);
}

const repoUrl = (await getExecOutput("git", ["remote", "get-url", `${preferredRemote}`], cwd)).replace(/\.git$/, "");

// In practice, it's common to see many requests for (different lines in) the same file
const serverPathCache = new Map<string, string>(); // local to server

const urls: string[] = [];
for (const location of locations) {
const localPath = path.resolve(location.path);
const lineNumber = location.lineNumber;

// We would just use path math, but this will also respect git's casing

let serverPath = serverPathCache.get(localPath);
if (!serverPath) {
serverPath = await getExecOutput("git", ["ls-files", "--full-name", "--", `${localPath}`], cwd);
serverPathCache.set(localPath, serverPath);
}

// Use a file URL if the file is untracked
const fileUrl = serverPath
? `${repoUrl}/blob/${commit}/${serverPath}`
: url.pathToFileURL(localPath).toString();

// Cheat and add line numbers to file URLs too - VS Code handles it
urls.push(lineNumber ? `${fileUrl}#L${lineNumber}` : fileUrl);
}
return urls;
}

async function getPreferredRemote(cwd: string, commit: string): Promise<string | undefined> {
let containingRemotes: string[] = [];
const refsRegex = /^refs\/remotes\/([^\/]+)/gm;
const refs = await getExecOutput("git", ["for-each-ref", `--format=%(refname)`, "--contains", commit, "refs/remotes"], cwd);
let refMatch: RegExpExecArray | null;
while (refMatch = refsRegex.exec(refs)) {
containingRemotes.push(refMatch[1]);
}

if (containingRemotes.length) {
return containingRemotes.find(r => r === "origin") || containingRemotes[0];
}

// Sometimes, the for-each-ref trick doesn't work (e.g. in some submodules), so we fall back on a slower method

// Sort `origin` to the front, if it's present
const allRemotes = (await getExecOutput("git", ["remote"], cwd)).split(/\r\n?|\n/).sort((a, b) => +(b === "origin") - +(a === "origin"));

for (const remote of allRemotes) {
const status = await getSpawnExitCode("git", ["fetch", "--dry-run", "--quiet", remote, commit], cwd);
if (status === 0) {
return remote;
}
}

return undefined;
}

function getExecOutput(command: string, args: readonly string[], cwd: string): Promise<string> {
return new Promise(resolve => {
cp.execFile(command, args, { cwd, encoding: "utf-8" }, (err, stdout, stderr) => {
resolve((err || stderr) ? "" : stdout.trim());
});
});
}

function getSpawnExitCode(command: string, args: readonly string[], cwd: string): Promise<number> {
return new Promise(resolve => {
const proc = cp.spawn(command, args, { cwd, stdio: "ignore" });
proc.on("close", code => resolve(code!));
proc.stderr
});
}
Loading

0 comments on commit f044986

Please sign in to comment.