diff --git a/src/main.ts b/src/main.ts index 013e200..fa79beb 100644 --- a/src/main.ts +++ b/src/main.ts @@ -9,7 +9,6 @@ import fs = require("fs"); import path = require("path"); import mdEscape = require("markdown-escape"); import randomSeed = require("random-seed"); -import { getErrorMessageFromStack, getHash, getHashForStack } from "./utils/hashStackTrace"; interface Params { /** @@ -85,31 +84,9 @@ export type RepoStatus = | "Detected no interesting changes" ; -interface TSServerResult { - oldServerFailed: boolean; - oldSpawnResult?: SpawnResult; - newServerFailed: boolean; - newSpawnResult: SpawnResult; - replayScriptPath: string; - installCommands: ip.InstallCommand[]; -} - -interface Summary { - tsServerResult: TSServerResult; - repo: git.Repo; - oldTsEntrypointPath: string; - rawErrorArtifactPath: string; - replayScriptPath: string; - repoDir: string; - downloadDir: string; - replayScriptArtifactPath: string; - replayScriptName: string; -} - interface RepoResult { readonly status: RepoStatus; readonly summary?: string; - readonly tsServerResult?: TSServerResult; readonly replayScriptPath?: string; readonly rawErrorPath?: string; } @@ -226,6 +203,8 @@ async function getTsServerRepoResult( ? (await installPackagesAndGetCommands(repo, downloadDir, repoDir, monorepoPackages, /*cleanOnFailure*/ true, diagnosticOutput))! : []; + const isUserTestRepo = !repo.url; + const replayScriptName = path.basename(replayScriptArtifactPath); const replayScriptPath = path.join(downloadDir, replayScriptName); @@ -323,173 +302,110 @@ async function getTsServerRepoResult( } } - const tsServerResult = { - oldServerFailed, - oldSpawnResult, - newServerFailed, - newSpawnResult, - replayScriptPath, - installCommands, - }; - - if (oldServerFailed && !newServerFailed) { - return { status: "Detected interesting changes", tsServerResult } - } - if (!newServerFailed) { - return { status: "Detected no interesting changes" }; - } + const owner = repo.owner ? `${mdEscape(repo.owner)}/` : ""; + const url = repo.url ? `(${repo.url})` : ""; - return { status: "Detected interesting changes", tsServerResult, replayScriptPath, rawErrorPath }; - } - catch (err) { - reportError(err, `Error running tsserver on ${repo.url ?? repo.name}`); - return { status: "Unknown failure" }; - } - finally { - console.log(`Done ${repo.url ?? repo.name}`); - logStepTime(diagnosticOutput, repo, "language service", lsStart); - } -} + let summary = `## [${owner}${mdEscape(repo.name)}]${url}\n`; -function groupErrors(summaries: Summary[]) { - const groupedOldErrors = new Map(); - const groupedNewErrors = new Map(); - let group: Map; - let error: ServerHarnessOutput | string; - for (const summary of summaries) { - if (summary.tsServerResult.newServerFailed) { - // Group new errors - error = parseServerHarnessOutput(summary.tsServerResult.newSpawnResult!.stdout); - group = groupedNewErrors; - } - else if (summary.tsServerResult.oldServerFailed) { - // Group old errors - const { oldSpawnResult } = summary.tsServerResult; - error = oldSpawnResult?.stdout - ? parseServerHarnessOutput(oldSpawnResult.stdout) + if (oldServerFailed) { + const oldServerError = oldSpawnResult?.stdout + ? prettyPrintServerHarnessOutput(oldSpawnResult.stdout, /*filter*/ true) : `Timed out after ${executionTimeout} ms`; - - group = groupedOldErrors; - } - else { - continue; - } - - const key = typeof error === "string" ? getHash([error]) : getHashForStack(error.message); - const value = group.get(key) ?? []; - value.push(summary); - group.set(key, value); - } - - return { groupedOldErrors, groupedNewErrors } -} - -function getErrorMessage(output: string): string { - const error = parseServerHarnessOutput(output); - - return typeof error === "string" ? error : getErrorMessageFromStack(error.message); -} - -function createOldErrorSummary(summaries: Summary[]): string { - const { oldSpawnResult } = summaries[0].tsServerResult; - - const oldServerError = oldSpawnResult?.stdout - ? prettyPrintServerHarnessOutput(oldSpawnResult.stdout, /*filter*/ true) - : `Timed out after ${executionTimeout} ms`; - - const errorMessage = oldSpawnResult?.stdout ? getErrorMessage(oldSpawnResult.stdout) : oldServerError; - - let text = ` + summary += `
-${errorMessage} +:warning: Note that ${path.basename(path.dirname(path.dirname(oldTsServerPath)))} had errors :warning: \`\`\` ${oldServerError} \`\`\` -

Repos no longer reporting the error

-
    -`; - - for (const summary of summaries) { - const owner = summary.repo.owner ? `${mdEscape(summary.repo.owner)}/` : ""; - const url = summary.repo.url ?? ""; - - text += `
  • ${owner + mdEscape(summary.repo.name)}
    • \n` - } - - text += ` -
-`; - return text; -} +`; + if (!newServerFailed) { + summary += ` +:tada: New server no longer has errors :tada: +`; + return { status: "Detected interesting changes", summary } + } + } + + if (!newServerFailed) { + return { status: "Detected no interesting changes" }; + } -async function createNewErrorSummaryAsync(summaries: Summary[]): Promise { - let text = `

${getErrorMessage(summaries[0].tsServerResult.newSpawnResult.stdout)}

+ summary += ` \`\`\` -${prettyPrintServerHarnessOutput(summaries[0].tsServerResult.newSpawnResult.stdout, /*filter*/ true)} +${prettyPrintServerHarnessOutput(newSpawnResult.stdout, /*filter*/ true)} \`\`\` +That is a filtered view of the text. To see the raw error text, go to ${rawErrorArtifactPath} in the artifact folder\n +`; -

Affected repos

`; - - for (const summary of summaries) { - const owner = summary.repo.owner ? `${mdEscape(summary.repo.owner)}/` : ""; - const url = summary.repo.url ?? ""; - - text += ` + summary += `
-${owner + mdEscape(summary.repo.name)} -Raw error text: ${summary.rawErrorArtifactPath} in the artifact folder -

Last few requests

+

Last few requests

 \`\`\`json -${fs.readFileSync(summary.replayScriptPath, { encoding: "utf-8" }).split(/\r?\n/).slice(-5).join("\n")} +${fs.readFileSync(replayScriptPath, { encoding: "utf-8" }).split(/\r?\n/).slice(-5).join("\n")} \`\`\` -

Repro steps

+
+ +`; + + // Markdown doesn't seem to support a
list item, so this chunk is in HTML + + summary += `
+

Repro Steps
    `; - // No url means is user test repo - if (!summary.repo.url) { - text += `
  1. Download user test ${summary.repo.name}
    2. \n`; + if (isUserTestRepo) { + summary += `
  2. Download user test ${repo.name}
    3. \n`; } else { - text += `
  3. git clone ${summary.repo.url} --recurse-submodules
    4. \n`; + summary += `
  4. git clone ${repo.url!} --recurse-submodules
    5. \n`; try { console.log("Extracting commit SHA for repro steps"); - const commit = (await execAsync(summary.repoDir, `git rev-parse @`)).trim(); - text += `
  5. In dir ${summary.repo.name}, run git reset --hard ${commit}
    6. \n`; + const commit = (await execAsync(repoDir, `git rev-parse @`)).trim(); + summary += `
  6. In dir ${repo.name}, run git reset --hard ${commit}
    7. \n`; } catch { } } - if (summary.tsServerResult.installCommands.length > 1) { - text += "
  7. Install packages (exact steps are below, but it might be easier to follow the repo readme)
      \n"; + if (installCommands.length > 1) { + summary += "
    1. Install packages (exact steps are below, but it might be easier to follow the repo readme)
        \n"; } - for (const command of summary.tsServerResult.installCommands) { - text += `
      1. In dir ${path.relative(summary.downloadDir, command.directory)}, run ${command.tool} ${command.arguments.join(" ")}
        2. \n`; + for (const command of installCommands) { + summary += `
      2. In dir ${path.relative(downloadDir, command.directory)}, run ${command.tool} ${command.arguments.join(" ")}
        3. \n`; } - if (summary.tsServerResult.installCommands.length > 1) { - text += "
      \n"; + if (installCommands.length > 1) { + summary += "
    \n"; } // The URL of the artifact can be determined via AzDO REST APIs, but not until after the artifact is published - text += `
  8. Back in the initial folder, download ${summary.replayScriptArtifactPath} from the artifact folder
    9. \n`; - text += `
  9. npm install --no-save @typescript/server-replay
    10. \n`; - text += `
  10. npx tsreplay ./${summary.repo.name} ./${summary.replayScriptName} path/to/tsserver.js
    11. \n`; - text += `
  11. npx tsreplay --help to learn about helpful switches for debugging, logging, etc
    12. \n`; + summary += `
  12. Back in the initial folder, download ${replayScriptArtifactPath} from the artifact folder
    13. \n`; + summary += `
  13. npm install --no-save @typescript/server-replay
    14. \n`; + summary += `
  14. npx tsreplay ./${repo.name} ./${replayScriptName} path/to/tsserver.js
    15. \n`; + summary += `
  15. npx tsreplay --help to learn about helpful switches for debugging, logging, etc
    16. \n`; - text += `
+ summary += `
+ `; - } - return text; + return { status: "Detected interesting changes", summary, replayScriptPath, rawErrorPath }; + } + catch (err) { + reportError(err, `Error running tsserver on ${repo.url ?? repo.name}`); + return { status: "Unknown failure" }; + } + finally { + console.log(`Done ${repo.url ?? repo.name}`); + logStepTime(diagnosticOutput, repo, "language service", lsStart); + } } // Exported for testing @@ -742,8 +658,6 @@ export async function mainAsync(params: GitParams | UserParams): Promise { const isPr = params.testType === "user" && !!params.prNumber - var summaries: Summary[] = []; - let i = 1; for (const repo of repos) { console.log(`Starting #${i++} / ${repos.length}: ${repo.url ?? repo.name}`); @@ -758,36 +672,16 @@ export async function mainAsync(params: GitParams | UserParams): Promise { : repo.name; const replayScriptFileName = `${repoPrefix}.${replayScriptFileNameSuffix}`; const rawErrorFileName = `${repoPrefix}.${rawErrorFileNameSuffix}`; - - const rawErrorArtifactPath = path.join(params.resultDirName, rawErrorFileName); - const replayScriptArtifactPath = path.join(params.resultDirName, replayScriptFileName); - - const { status, summary, tsServerResult, replayScriptPath, rawErrorPath } = params.entrypoint === "tsc" + const { status, summary, replayScriptPath, rawErrorPath } = params.entrypoint === "tsc" ? await getTscRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, params.buildWithNewWhenOldFails, downloadDir, diagnosticOutput) - : await getTsServerRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, downloadDir, replayScriptArtifactPath, rawErrorArtifactPath, diagnosticOutput, isPr); + : await getTsServerRepoResult(repo, userTestsDir, oldTsEntrypointPath, newTsEntrypointPath, downloadDir, path.join(params.resultDirName, replayScriptFileName), path.join(params.resultDirName, rawErrorFileName), diagnosticOutput, isPr); console.log(`Repo ${repo.url ?? repo.name} had status "${status}"`); statusCounts[status] = (statusCounts[status] ?? 0) + 1; - if (summary) { + const resultFileName = `${repoPrefix}.${resultFileNameSuffix}`; await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); - } - - if (tsServerResult) { - summaries.push({ - tsServerResult, - repo, - oldTsEntrypointPath, - rawErrorArtifactPath, - replayScriptPath: path.join(downloadDir, path.basename(replayScriptArtifactPath)), - repoDir: path.join(downloadDir, repo.name), - downloadDir, - replayScriptArtifactPath, - replayScriptName: path.basename(replayScriptArtifactPath), - }); - } - if (summary || tsServerResult) { // In practice, there will only be a replay script when the entrypoint is tsserver // There can be replay steps without a summary, but then they're not interesting if (replayScriptPath) { @@ -796,7 +690,9 @@ export async function mainAsync(params: GitParams | UserParams): Promise { if (rawErrorPath) { await fs.promises.copyFile(rawErrorPath, path.join(resultDirPath, rawErrorFileName)); } + } + } finally { // Throw away the repo so we don't run out of space @@ -831,25 +727,6 @@ export async function mainAsync(params: GitParams | UserParams): Promise { } } - // Group errors and create summary files. - if (summaries.length > 0) { - const { groupedOldErrors, groupedNewErrors } = groupErrors(summaries); - - for (let [key, value] of groupedOldErrors) { - const summary = createOldErrorSummary(value); - const resultFileName = `!${key}.${resultFileNameSuffix}`; // Exclamation point makes the file to be put first when ordering. - - await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); - } - - for (let [key, value] of groupedNewErrors) { - const summary = await createNewErrorSummaryAsync(value); - const resultFileName = `${key}.${resultFileNameSuffix}`; - - await fs.promises.writeFile(path.join(resultDirPath, resultFileName), summary, { encoding: "utf-8" }); - } - } - if (params.tmpfs) { await execAsync(processCwd, "sudo rm -rf " + downloadDir); await execAsync(processCwd, "sudo rm -rf " + oldTscDirPath); @@ -1116,4 +993,4 @@ async function downloadTsNpmAsync(cwd: string, version: string, entrypoint: TsEn } return { tsEntrypointPath, resolvedVersion }; -} \ No newline at end of file +} diff --git a/src/postGithubComments.ts b/src/postGithubComments.ts index 9fddbaf..8933b8b 100644 --- a/src/postGithubComments.ts +++ b/src/postGithubComments.ts @@ -55,9 +55,6 @@ else { summary = `Everything looks good!`; } -// Files starting with an exclamation point are old server errors. -const hasOldErrors = pu.glob(resultDirPath, `**/!*.${resultFileNameSuffix}`).length !== 0; - const resultPaths = pu.glob(resultDirPath, `**/*.${resultFileNameSuffix}`).sort((a, b) => path.basename(a).localeCompare(path.basename(b))); const outputs = resultPaths.map(p => fs.readFileSync(p, { encoding: "utf-8" }).replace(new RegExp(artifactFolderUrlPlaceholder, "g"), artifactsUri)); @@ -70,10 +67,9 @@ if (!outputs.length) { git.createComment(+prNumber, +commentNumber, postResult, [header]); } else { - const oldErrorHeader = `

:warning: Old server errors :warning:

`; const openDetails = `\n\n
\nDetails\n\n`; const closeDetails = `\n
`; - const initialHeader = header + openDetails + (hasOldErrors ? oldErrorHeader : ''); + const initialHeader = header + openDetails; const continuationHeader = `@${userToTag} Here are some more interesting changes from running the ${suiteDescription} suite${openDetails}`; const trunctationSuffix = `\n:error: Truncated - see log for full output :error:`; diff --git a/src/utils/hashStackTrace.ts b/src/utils/hashStackTrace.ts deleted file mode 100644 index 3246d69..0000000 --- a/src/utils/hashStackTrace.ts +++ /dev/null @@ -1,18 +0,0 @@ - import { createHash } from "crypto"; - -export function getHash(methods: string[]): string { - const lines = methods.join("\n"); - return createHash("md5").update(lines).digest("hex"); -} - -export function getHashForStack(stack: string): string { - const stackLines = stack.split(/\r?\n/); - - return getHash(stackLines); -} - -export function getErrorMessageFromStack(stack: string): string { - const stackLines = stack.split(/\r?\n/, 2); - - return stackLines[1]; -} \ No newline at end of file