From fd4a8d1516e5e5ae8c0c8b7f47ed4191a1799e68 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Thu, 14 Sep 2017 16:22:14 -0700 Subject: [PATCH] Let the RWC harness iterate over files instead of building one big file (#18416) * Let the RWC harness iterate over files instead of building one big file * Handle duplicated-only-in-case outputs better in the type baseliner * Always lowercase output names * Move common code into helper function * Always write .delete for missing files even if there were errors --- Jakefile.js | 6 +- src/harness/harness.ts | 196 +++++++++++++++++++++++++++++++-------- src/harness/rwcRunner.ts | 30 +++--- 3 files changed, 177 insertions(+), 55 deletions(-) diff --git a/Jakefile.js b/Jakefile.js index 39e9e8a0421..6fd2f549015 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -769,15 +769,16 @@ function exec(cmd, completeHandler, errorHandler) { ex.run(); } +const del = require("del"); function cleanTestDirs() { // Clean the local baselines directory if (fs.existsSync(localBaseline)) { - jake.rmRf(localBaseline); + del.sync(localBaseline); } // Clean the local Rwc baselines directory if (fs.existsSync(localRwcBaseline)) { - jake.rmRf(localRwcBaseline); + del.sync(localRwcBaseline); } jake.mkdirP(localRwcBaseline); @@ -1042,6 +1043,7 @@ function acceptBaseline(sourceFolder, targetFolder) { if (fs.existsSync(target)) { fs.unlinkSync(target); } + jake.mkdirP(path.dirname(target)); fs.renameSync(path.join(sourceFolder, filename), target); } } diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 8344be36753..7a6c061c6d4 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -1291,11 +1291,25 @@ namespace Harness { } export function getErrorBaseline(inputFiles: ReadonlyArray, diagnostics: ReadonlyArray, pretty?: boolean) { + let outputLines = ""; + const gen = iterateErrorBaseline(inputFiles, diagnostics, pretty); + for (let {done, value} = gen.next(); !done; { done, value } = gen.next()) { + const [, content] = value; + outputLines += content; + } + return outputLines; + } + + export const diagnosticSummaryMarker = "__diagnosticSummary"; + export const globalErrorsMarker = "__globalErrors"; + export function *iterateErrorBaseline(inputFiles: ReadonlyArray, diagnostics: ReadonlyArray, pretty?: boolean): IterableIterator<[string, string, number]> { diagnostics = diagnostics.slice().sort(ts.compareDiagnostics); let outputLines = ""; // Count up all errors that were found in files other than lib.d.ts so we don't miss any let totalErrorsReportedInNonLibraryFiles = 0; + let errorsReported = 0; + let firstLine = true; function newLine() { if (firstLine) { @@ -1314,6 +1328,7 @@ namespace Harness { .filter(s => s.length > 0) .map(s => "!!! " + ts.DiagnosticCategory[error.category].toLowerCase() + " TS" + error.code + ": " + s); errLines.forEach(e => outputLines += (newLine() + e)); + errorsReported++; // do not count errors from lib.d.ts here, they are computed separately as numLibraryDiagnostics // if lib.d.ts is explicitly included in input files and there are some errors in it (i.e. because of duplicate identifiers) @@ -1325,12 +1340,18 @@ namespace Harness { } } + yield [diagnosticSummaryMarker, minimalDiagnosticsToString(diagnostics, pretty) + Harness.IO.newLine() + Harness.IO.newLine(), diagnostics.length]; + // Report global errors const globalErrors = diagnostics.filter(err => !err.file); globalErrors.forEach(outputErrorText); + yield [globalErrorsMarker, outputLines, errorsReported]; + outputLines = ""; + errorsReported = 0; // 'merge' the lines of each input file with any errors associated with it - inputFiles.filter(f => f.content !== undefined).forEach(inputFile => { + const dupeCase = ts.createMap(); + for (const inputFile of inputFiles.filter(f => f.content !== undefined)) { // Filter down to the errors in the file const fileErrors = diagnostics.filter(e => { const errFn = e.file; @@ -1396,7 +1417,10 @@ namespace Harness { // Verify we didn't miss any errors in this file assert.equal(markedErrorCount, fileErrors.length, "count of errors in " + inputFile.unitName); - }); + yield [checkDuplicatedFileName(inputFile.unitName, dupeCase), outputLines, errorsReported]; + outputLines = ""; + errorsReported = 0; + } const numLibraryDiagnostics = ts.countWhere(diagnostics, diagnostic => { return diagnostic.file && (isDefaultLibraryFile(diagnostic.file.fileName) || isBuiltFile(diagnostic.file.fileName)); @@ -1409,9 +1433,6 @@ namespace Harness { // Verify we didn't miss any errors in total assert.equal(totalErrorsReportedInNonLibraryFiles + numLibraryDiagnostics + numTest262HarnessDiagnostics, diagnostics.length, "total number of errors"); - - return minimalDiagnosticsToString(diagnostics, pretty) + - Harness.IO.newLine() + Harness.IO.newLine() + outputLines; } export function doErrorBaseline(baselinePath: string, inputFiles: TestFile[], errors: ts.Diagnostic[], pretty?: boolean) { @@ -1425,7 +1446,7 @@ namespace Harness { }); } - export function doTypeAndSymbolBaseline(baselinePath: string, result: CompilerResult, allFiles: {unitName: string, content: string}[], opts?: Harness.Baseline.BaselineOptions) { + export function doTypeAndSymbolBaseline(baselinePath: string, result: CompilerResult, allFiles: {unitName: string, content: string}[], opts?: Harness.Baseline.BaselineOptions, multifile?: boolean) { if (result.errors.length !== 0) { return; } @@ -1486,24 +1507,42 @@ namespace Harness { return; function checkBaseLines(isSymbolBaseLine: boolean) { - const fullBaseLine = generateBaseLine(fullResults, isSymbolBaseLine); - const fullExtension = isSymbolBaseLine ? ".symbols" : ".types"; - // When calling this function from rwc-runner, the baselinePath will have no extension. // As rwc test- file is stored in json which ".json" will get stripped off. // When calling this function from compiler-runner, the baselinePath will then has either ".ts" or ".tsx" extension const outputFileName = ts.endsWith(baselinePath, ts.Extension.Ts) || ts.endsWith(baselinePath, ts.Extension.Tsx) ? - baselinePath.replace(/\.tsx?/, fullExtension) : baselinePath.concat(fullExtension); - Harness.Baseline.runBaseline(outputFileName, () => fullBaseLine, opts); + baselinePath.replace(/\.tsx?/, "") : baselinePath; + + if (!multifile) { + const fullBaseLine = generateBaseLine(fullResults, isSymbolBaseLine); + Harness.Baseline.runBaseline(outputFileName + fullExtension, () => fullBaseLine, opts); + } + else { + Harness.Baseline.runMultifileBaseline(outputFileName, fullExtension, () => { + return iterateBaseLine(fullResults, isSymbolBaseLine); + }, opts); + } } function generateBaseLine(typeWriterResults: ts.Map, isSymbolBaseline: boolean): string { - const typeLines: string[] = []; - const typeMap: { [fileName: string]: { [lineNum: number]: string[]; } } = {}; + let result = ""; + const gen = iterateBaseLine(typeWriterResults, isSymbolBaseline); + for (let {done, value} = gen.next(); !done; { done, value } = gen.next()) { + const [, content] = value; + result += content; + } + return result; + } - allFiles.forEach(file => { + function *iterateBaseLine(typeWriterResults: ts.Map, isSymbolBaseline: boolean): IterableIterator<[string, string]> { + let typeLines = ""; + const typeMap: { [fileName: string]: { [lineNum: number]: string[]; } } = {}; + const dupeCase = ts.createMap(); + + for (const file of allFiles) { const codeLines = file.content.split("\n"); + const key = file.unitName; typeWriterResults.get(file.unitName).forEach(result => { if (isSymbolBaseline && !result.symbol) { return; @@ -1511,42 +1550,42 @@ namespace Harness { const typeOrSymbolString = isSymbolBaseline ? result.symbol : result.type; const formattedLine = result.sourceText.replace(/\r?\n/g, "") + " : " + typeOrSymbolString; - if (!typeMap[file.unitName]) { - typeMap[file.unitName] = {}; + if (!typeMap[key]) { + typeMap[key] = {}; } let typeInfo = [formattedLine]; - const existingTypeInfo = typeMap[file.unitName][result.line]; + const existingTypeInfo = typeMap[key][result.line]; if (existingTypeInfo) { typeInfo = existingTypeInfo.concat(typeInfo); } - typeMap[file.unitName][result.line] = typeInfo; + typeMap[key][result.line] = typeInfo; }); - typeLines.push("=== " + file.unitName + " ===\r\n"); + typeLines += "=== " + file.unitName + " ===\r\n"; for (let i = 0; i < codeLines.length; i++) { const currentCodeLine = codeLines[i]; - typeLines.push(currentCodeLine + "\r\n"); - if (typeMap[file.unitName]) { - const typeInfo = typeMap[file.unitName][i]; + typeLines += currentCodeLine + "\r\n"; + if (typeMap[key]) { + const typeInfo = typeMap[key][i]; if (typeInfo) { typeInfo.forEach(ty => { - typeLines.push(">" + ty + "\r\n"); + typeLines += ">" + ty + "\r\n"; }); if (i + 1 < codeLines.length && (codeLines[i + 1].match(/^\s*[{|}]\s*$/) || codeLines[i + 1].trim() === "")) { } else { - typeLines.push("\r\n"); + typeLines += "\r\n"; } } } else { - typeLines.push("No type information for this code."); + typeLines += "No type information for this code."; } } - }); - - return typeLines.join(""); + yield [checkDuplicatedFileName(file.unitName, dupeCase), typeLines]; + typeLines = ""; + } } } @@ -1642,24 +1681,29 @@ namespace Harness { } export function collateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): string { - // Collect, test, and sort the fileNames - outputFiles.sort((a, b) => ts.compareStrings(cleanName(a.fileName), cleanName(b.fileName))); - + const gen = iterateOutputs(outputFiles); // Emit them let result = ""; - for (const outputFile of outputFiles) { + for (let {done, value} = gen.next(); !done; { done, value } = gen.next()) { // Some extra spacing if this isn't the first file if (result.length) { result += "\r\n\r\n"; } - // FileName header + content - result += "/*====== " + outputFile.fileName + " ======*/\r\n"; - - result += outputFile.code; + const [, content] = value; + result += content; } - return result; + } + + export function *iterateOutputs(outputFiles: Harness.Compiler.GeneratedFile[]): IterableIterator<[string, string]> { + // Collect, test, and sort the fileNames + outputFiles.sort((a, b) => ts.compareStrings(cleanName(a.fileName), cleanName(b.fileName))); + const dupeCase = ts.createMap(); + // Yield them + for (const outputFile of outputFiles) { + yield [checkDuplicatedFileName(outputFile.fileName, dupeCase), "/*====== " + outputFile.fileName + " ======*/\r\n" + outputFile.code]; + } function cleanName(fn: string) { const lastSlash = ts.normalizeSlashes(fn).lastIndexOf("/"); @@ -1667,6 +1711,24 @@ namespace Harness { } } + function checkDuplicatedFileName(resultName: string, dupeCase: ts.Map): string { + resultName = sanitizeTestFilePath(resultName); + if (dupeCase.has(resultName)) { + // A different baseline filename should be manufactured if the names differ only in case, for windows compat + const count = 1 + dupeCase.get(resultName); + dupeCase.set(resultName, count); + resultName = `${resultName}.dupe${count}`; + } + else { + dupeCase.set(resultName, 0); + } + return resultName; + } + + function sanitizeTestFilePath(name: string) { + return ts.normalizeSlashes(name.replace(/[\^<>:"|?*%]/g, "_")).replace(/\.\.\//g, "__dotdot/").toLowerCase(); + } + // This does not need to exist strictly speaking, but many tests will need to be updated if it's removed export function compileString(_code: string, _unitName: string, _callback: (result: CompilerResult) => void) { // NEWTODO: Re-implement 'compileString' @@ -2004,6 +2066,66 @@ namespace Harness { const comparison = compareToBaseline(actual, relativeFileName, opts); writeComparison(comparison.expected, comparison.actual, relativeFileName, actualFileName); } + + export function runMultifileBaseline(relativeFileBase: string, extension: string, generateContent: () => IterableIterator<[string, string, number]> | IterableIterator<[string, string]>, opts?: BaselineOptions, referencedExtensions?: string[]): void { + const gen = generateContent(); + const writtenFiles = ts.createMap(); + const canonicalize = ts.createGetCanonicalFileName(/*caseSensitive*/ false); // This is done so tests work on windows _and_ linux + /* tslint:disable-next-line:no-null-keyword */ + const errors: Error[] = []; + if (gen !== null) { + for (let {done, value} = gen.next(); !done; { done, value } = gen.next()) { + const [name, content, count] = value as [string, string, number | undefined]; + if (count === 0) continue; // Allow error reporter to skip writing files without errors + const relativeFileName = ts.combinePaths(relativeFileBase, name) + extension; + const actualFileName = localPath(relativeFileName, opts && opts.Baselinefolder, opts && opts.Subfolder); + const actual = content; + const comparison = compareToBaseline(actual, relativeFileName, opts); + try { + writeComparison(comparison.expected, comparison.actual, relativeFileName, actualFileName); + } + catch (e) { + errors.push(e); + } + const path = ts.toPath(relativeFileName, "", canonicalize); + writtenFiles.set(path, true); + } + } + + const referenceDir = referencePath(relativeFileBase, opts && opts.Baselinefolder, opts && opts.Subfolder); + let existing = Harness.IO.readDirectory(referenceDir, referencedExtensions || [extension]); + if (extension === ".ts" || referencedExtensions && referencedExtensions.indexOf(".ts") > -1 && referencedExtensions.indexOf(".d.ts") === -1) { + // special-case and filter .d.ts out of .ts results + existing = existing.filter(f => !ts.endsWith(f, ".d.ts")); + } + const missing: string[] = []; + for (const name of existing) { + const localCopy = name.substring(referenceDir.length - relativeFileBase.length); + const path = ts.toPath(localCopy, "", canonicalize); + if (!writtenFiles.has(path)) { + missing.push(localCopy); + } + } + if (missing.length) { + for (const file of missing) { + IO.writeFile(localPath(file + ".delete", opts && opts.Baselinefolder, opts && opts.Subfolder), ""); + } + } + + if (errors.length || missing.length) { + let errorMsg = ""; + if (errors.length) { + errorMsg += `The baseline for ${relativeFileBase} has changed:${"\n " + errors.map(e => e.message).join("\n ")}`; + } + if (errors.length && missing.length) { + errorMsg += "\n"; + } + if (missing.length) { + errorMsg += `Baseline missing files:${"\n " + missing.join("\n ") + "\n"}Written:${"\n " + ts.arrayFrom(writtenFiles.keys()).join("\n ")}`; + } + throw new Error(errorMsg); + } + } } export function isDefaultLibraryFile(filePath: string): boolean { diff --git a/src/harness/rwcRunner.ts b/src/harness/rwcRunner.ts index b3ef80b2a31..1f398118c06 100644 --- a/src/harness/rwcRunner.ts +++ b/src/harness/rwcRunner.ts @@ -170,29 +170,29 @@ namespace RWC { it("has the expected emitted code", () => { - Harness.Baseline.runBaseline(`${baseName}.output.js`, () => { - return Harness.Compiler.collateOutputs(compilerResult.files); - }, baselineOpts); + Harness.Baseline.runMultifileBaseline(baseName, "", () => { + return Harness.Compiler.iterateOutputs(compilerResult.files); + }, baselineOpts, [".js", ".jsx"]); }); it("has the expected declaration file content", () => { - Harness.Baseline.runBaseline(`${baseName}.d.ts`, () => { + Harness.Baseline.runMultifileBaseline(baseName, "", () => { if (!compilerResult.declFilesCode.length) { return null; } - return Harness.Compiler.collateOutputs(compilerResult.declFilesCode); - }, baselineOpts); + return Harness.Compiler.iterateOutputs(compilerResult.declFilesCode); + }, baselineOpts, [".d.ts"]); }); it("has the expected source maps", () => { - Harness.Baseline.runBaseline(`${baseName}.map`, () => { + Harness.Baseline.runMultifileBaseline(baseName, "", () => { if (!compilerResult.sourceMaps.length) { return null; } - return Harness.Compiler.collateOutputs(compilerResult.sourceMaps); - }, baselineOpts); + return Harness.Compiler.iterateOutputs(compilerResult.sourceMaps); + }, baselineOpts, [".map"]); }); /*it("has correct source map record", () => { @@ -204,14 +204,14 @@ namespace RWC { });*/ it("has the expected errors", () => { - Harness.Baseline.runBaseline(`${baseName}.errors.txt`, () => { + Harness.Baseline.runMultifileBaseline(baseName, ".errors.txt", () => { if (compilerResult.errors.length === 0) { return null; } // Do not include the library in the baselines to avoid noise const baselineFiles = tsconfigFiles.concat(inputFiles, otherFiles).filter(f => !Harness.isDefaultLibraryFile(f.unitName)); const errors = compilerResult.errors.filter(e => !e.file || !Harness.isDefaultLibraryFile(e.file.fileName)); - return Harness.Compiler.getErrorBaseline(baselineFiles, errors); + return Harness.Compiler.iterateErrorBaseline(baselineFiles, errors); }, baselineOpts); }); @@ -220,14 +220,14 @@ namespace RWC { Harness.Compiler.doTypeAndSymbolBaseline(baseName, compilerResult, inputFiles .concat(otherFiles) .filter(file => !!compilerResult.program.getSourceFile(file.unitName)) - .filter(e => !Harness.isDefaultLibraryFile(e.unitName)), baselineOpts); + .filter(e => !Harness.isDefaultLibraryFile(e.unitName)), baselineOpts, /*multifile*/ true); }); // Ideally, a generated declaration file will have no errors. But we allow generated // declaration file errors as part of the baseline. it("has the expected errors in generated declaration files", () => { if (compilerOptions.declaration && !compilerResult.errors.length) { - Harness.Baseline.runBaseline(`${baseName}.dts.errors.txt`, () => { + Harness.Baseline.runMultifileBaseline(baseName, ".dts.errors.txt", () => { if (compilerResult.errors.length === 0) { return null; } @@ -239,9 +239,7 @@ namespace RWC { compilerResult = undefined; const declFileCompilationResult = Harness.Compiler.compileDeclarationFiles(declContext); - return Harness.Compiler.minimalDiagnosticsToString(declFileCompilationResult.declResult.errors) + - Harness.IO.newLine() + Harness.IO.newLine() + - Harness.Compiler.getErrorBaseline(tsconfigFiles.concat(declFileCompilationResult.declInputFiles, declFileCompilationResult.declOtherFiles), declFileCompilationResult.declResult.errors); + return Harness.Compiler.iterateErrorBaseline(tsconfigFiles.concat(declFileCompilationResult.declInputFiles, declFileCompilationResult.declOtherFiles), declFileCompilationResult.declResult.errors); }, baselineOpts); } });