Skip to content

Commit

Permalink
Make top github repos install the same (microsoft#13)
Browse files Browse the repository at this point in the history 
regardless of running in 'git' or 'user' mode -- that is, scheduled or
on-demand mode.
  • Loading branch information
@sandersn
sandersn authored Jun 9, 2022
1 parent c158c16 commit 2c35dcb
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
6 changes: 3 additions & 3 deletions getErrors.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +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',
/*topGithubRepos*/ false,
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand All @@ -25,7 +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',
/*topGithubRepos*/ false,
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand All @@ -43,7 +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',
/*topGithubRepos*/ false,
/*skipLibCheck*/ true,
)
expect(errors.hasConfigFailure).toBeFalsy()
Expand Down
11 changes: 4 additions & 7 deletions getErrors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ 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, 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)
export async function buildAndGetErrors(repoDir: string, tscPath: string, topGithubRepos: boolean, skipLibCheck: boolean = true): Promise<RepoErrors> {
const simpleBuildArgs = `--skipLibCheck ${skipLibCheck} --incremental false --pretty false -p`;
const compositeBuildArgs = `-b -f -v`; // Build mode doesn't support --skipLibCheck or --pretty

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

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

let localErrors: LocalError[] = [];
let currProjectUrl = projectUrl;
Expand All @@ -102,7 +99,7 @@ export async function buildAndGetErrors(repoDir: string, tscPath: string, testTy
for (const line of lines) {
const projectMatch = isComposite && line.match(beginProjectRegex);
if (projectMatch) {
currProjectUrl = testType === "user" ? path.resolve(projectDir, projectMatch[1]) : await ghUrl.getGithubUrl(path.resolve(projectDir, projectMatch[1]));
currProjectUrl = topGithubRepos ? await ghUrl.getGithubUrl(path.resolve(projectDir, projectMatch[1])) : path.resolve(projectDir, projectMatch[1]);
continue;
}
const localError = getLocalErrorFromLine(line, currProjectUrl);
Expand All @@ -114,7 +111,7 @@ export async function buildAndGetErrors(repoDir: string, tscPath: string, testTy
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 = testType === "user" ? fileLocalErrors.map(x => `${x.path}(${x.lineNumber},${x.columnNumber})`) : await ghUrl.getGithubUrls(fileLocalErrors);
const fileUrls = topGithubRepos ? await ghUrl.getGithubUrls(fileLocalErrors) : fileLocalErrors.map(x => `${x.path}(${x.lineNumber},${x.columnNumber})`);
for (let i = 0; i < fileLocalErrors.length; i++) {
const localError = fileLocalErrors[i];
errors.push({
Expand Down
3 changes: 3 additions & 0 deletions main.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe("main", () => {
requestingUser: 'sandersn',
sourceIssue: 44585,
statusComment: 990374547,
topRepos: false,
}
const result = await mainAsync(options)
expect(result).toBeDefined()
Expand Down Expand Up @@ -45,10 +46,12 @@ The results of the user tests run you requested are in!
requestingUser: 'sandersn',
sourceIssue: 44585,
statusComment: 990374547,
topRepos: false,
}
const outputs: string[] = []
const hasNewErrors = await innerloop(
options,
/*topGithubRepos*/ false,
"./ts_downloads",
"./userTests",
{
Expand Down
23 changes: 12 additions & 11 deletions main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const skipRepos = [
const processCwd = process.cwd();
const processPid = process.pid;
const executionTimeout = 10 * 60 * 1000;
export async function innerloop(params: GitParams | UserParams, downloadDir: string, userTestDir: string, repo: git.Repo, oldTscPath: string, newTscPath: string, outputs: string[]) {
export async function innerloop(params: GitParams | UserParams, topGithubRepos: boolean, downloadDir: string, userTestDir: string, repo: git.Repo, oldTscPath: string, newTscPath: string, outputs: string[]) {
const { testType } = params
if (params.tmpfs)
await execAsync(processCwd, "sudo mount -t tmpfs -o size=4g tmpfs " + downloadDir);
Expand All @@ -66,7 +66,7 @@ export async function innerloop(params: GitParams | UserParams, downloadDir: str

try {
console.log("Installing packages if absent");
await withTimeout(executionTimeout, installPackages(repoDir, /*recursiveSearch*/ testType !== "user", repo.types));
await withTimeout(executionTimeout, installPackages(repoDir, /*recursiveSearch*/ topGithubRepos, repo.types));
}
catch (err) {
reportError(err, "Error installing packages for " + repo.name);
Expand All @@ -76,7 +76,7 @@ export async function innerloop(params: GitParams | UserParams, downloadDir: str

try {
console.log(`Building with ${oldTscPath} (old)`);
const oldErrors = await buildAndGetErrors(repoDir, oldTscPath, /*skipLibCheck*/ true, testType, params.postResult);
const oldErrors = await buildAndGetErrors(repoDir, oldTscPath, /*skipLibCheck*/ true, topGithubRepos, params.postResult);

if (oldErrors.hasConfigFailure) {
console.log("Unable to build project graph");
Expand All @@ -94,7 +94,7 @@ export async function innerloop(params: GitParams | UserParams, downloadDir: str
}

// User tests ignores build failures.
if (testType !== "user" && numFailed === numProjects) {
if (testType === "git" && numFailed === numProjects) {
console.log(`Skipping build with ${newTscPath} (new)`);
return;
}
Expand All @@ -112,7 +112,7 @@ export async function innerloop(params: GitParams | UserParams, downloadDir: str
}

console.log(`Building with ${newTscPath} (new)`);
const newErrors = await buildAndGetErrors(repoDir, newTscPath, /*skipLibCheck*/ true, testType, params.postResult);
const newErrors = await buildAndGetErrors(repoDir, newTscPath, /*skipLibCheck*/ true, topGithubRepos, params.postResult);

if (newErrors.hasConfigFailure) {
console.log("Unable to build project graph");
Expand All @@ -126,7 +126,7 @@ export async function innerloop(params: GitParams | UserParams, downloadDir: str
console.log("Comparing errors");
for (const oldProjectErrors of oldErrors.projectErrors) {
// To keep things simple, we'll focus on projects that used to build cleanly
if (testType !== "user" && (oldProjectErrors.hasBuildFailure || oldProjectErrors.errors.length)) {
if (testType === "git" && (oldProjectErrors.hasBuildFailure || oldProjectErrors.errors.length)) {
continue;
}

Expand Down Expand Up @@ -216,7 +216,8 @@ export async function mainAsync(params: GitParams | UserParams): Promise<GitResu

const userTestDir = path.join(processCwd, "userTests");

const repos = testType === "git" || params.topRepos ? await git.getPopularTypeScriptRepos(params.repoCount)
const topGithubRepos = testType === "git" || params.topRepos;
const repos = topGithubRepos ? await git.getPopularTypeScriptRepos(params.repoCount)
: testType === "user" ? ur.getUserTestsRepos(userTestDir)
: undefined;

Expand All @@ -236,7 +237,7 @@ export async function mainAsync(params: GitParams | UserParams): Promise<GitResu
i++;
if (i > maxCount) break;
console.log(`Starting #${i} / ${maxCount}: ${repo.url ?? repo.name}`);
if (await innerloop(params, downloadDir, userTestDir, repo, oldTscPath, newTscPath, outputs)) {
if (await innerloop(params, topGithubRepos, downloadDir, userTestDir, repo, oldTscPath, newTscPath, outputs)) {
sawNewErrors = true;
}
}
Expand Down Expand Up @@ -273,20 +274,20 @@ ${summary}`;
}
}

async function buildAndGetErrors(repoDir: string, tscPath: string, skipLibCheck: boolean, testType: 'git' | 'user', realTimeout: boolean): Promise<ge.RepoErrors> {
async function buildAndGetErrors(repoDir: string, tscPath: string, skipLibCheck: boolean, topGithubRepos: boolean, realTimeout: boolean): Promise<ge.RepoErrors> {
if (realTimeout) {
const p = new Promise<ge.RepoErrors>((resolve, reject) => {
const p = cp.fork(path.join(__dirname, "run-build.js"));
p.on('message', (m: 'ready' | ge.RepoErrors) =>
m === 'ready'
? p.send({ repoDir, tscPath, testType, skipLibCheck })
? p.send({ repoDir, tscPath, topGithubRepos, skipLibCheck })
: resolve(m));
p.on('exit', reject);
});
return withTimeout(executionTimeout, p);
}
else {
return ge.buildAndGetErrors(repoDir, tscPath, testType, skipLibCheck)
return ge.buildAndGetErrors(repoDir, tscPath, topGithubRepos, skipLibCheck)
}
}

Expand Down
4 changes: 2 additions & 2 deletions run-build.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ if (!process.send) process.exit(1);

process.send('ready');

process.on('message', ({repoDir, tscPath, testType, skipLibCheck}) => {
ge.buildAndGetErrors(repoDir, tscPath, testType, skipLibCheck)
process.on('message', ({repoDir, tscPath, topGithubRepos, skipLibCheck}) => {
ge.buildAndGetErrors(repoDir, tscPath, topGithubRepos, skipLibCheck)
.then(r => { process.send!(r), process.exit(); });
});

0 comments on commit 2c35dcb

Please sign in to comment.