Skip to content

Commit

Permalink
Refactor executableTests to use runner.ts where possible (palantir#3374)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ajafff @adidahiya
ajafff authored and adidahiya committed Nov 28, 2017
1 parent 22ec110 commit 0b83cbe
Show file tree
Hide file tree
Showing 6 changed files with 327 additions and 430 deletions.
42 changes: 23 additions & 19 deletions src/runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ export interface Options {
rulesDirectory?: string | string[];

/**
* That TSLint produces the correct output for the specified directory.
* Run the tests in the given directories to ensure a (custom) TSLint rule's output matches the expected output.
* When this property is `true` the `files` property is used to specify the directories from which the tests should be executed.
*/
test?: string;
test?: boolean;

/**
* Whether to enable type checking when linting a project.
Expand All @@ -122,7 +123,7 @@ export async function run(options: Options, logger: Logger): Promise<Status> {
return await runWorker(options, logger);
} catch (error) {
if (error instanceof FatalError) {
logger.error(error.message);
logger.error(`${error.message}\n`);
return Status.FatalError;
}
throw error;
Expand All @@ -142,7 +143,7 @@ async function runWorker(options: Options, logger: Logger): Promise<Status> {
if (options.test) {
const test = await import("./test");
const results = test.runTests((options.files || []).map(trimSingleQuotes), options.rulesDirectory);
return test.consoleTestResultsHandler(results) ? Status.Ok : Status.FatalError;
return test.consoleTestResultsHandler(results, logger) ? Status.Ok : Status.FatalError;
}

if (options.config && !fs.existsSync(options.config)) {
Expand All @@ -151,20 +152,20 @@ async function runWorker(options: Options, logger: Logger): Promise<Status> {

const { output, errorCount } = await runLinter(options, logger);
if (output && output.trim()) {
logger.log(output);
logger.log(`${output}\n`);
}
return options.force || errorCount === 0 ? Status.Ok : Status.LintError;
}

async function runLinter(options: Options, logger: Logger): Promise<LintResult> {
const { files, program } = resolveFilesAndProgram(options);
const { files, program } = resolveFilesAndProgram(options, logger);
// if type checking, run the type checker
if (program && options.typeCheck) {
const diagnostics = ts.getPreEmitDiagnostics(program);
if (diagnostics.length !== 0) {
const message = diagnostics.map((d) => showDiagnostic(d, program, options.outputAbsolutePaths)).join("\n");
if (options.force) {
logger.error(message);
logger.error(`${message}\n`);
} else {
throw new FatalError(message);
}
Expand All @@ -173,12 +174,15 @@ async function runLinter(options: Options, logger: Logger): Promise<LintResult>
return doLinting(options, files, program, logger);
}

function resolveFilesAndProgram({ files, project, exclude, outputAbsolutePaths }: Options): { files: string[]; program?: ts.Program } {
function resolveFilesAndProgram(
{ files, project, exclude, outputAbsolutePaths }: Options,
logger: Logger,
): { files: string[]; program?: ts.Program } {
// remove single quotes which break matching on Windows when glob is passed in single quotes
exclude = exclude.map(trimSingleQuotes);

if (project === undefined) {
return { files: resolveGlobs(files, exclude, outputAbsolutePaths) };
return { files: resolveGlobs(files, exclude, outputAbsolutePaths, logger) };
}

const projectPath = findTsconfig(project);
Expand All @@ -202,7 +206,7 @@ function resolveFilesAndProgram({ files, project, exclude, outputAbsolutePaths }
if (fs.existsSync(file)) {
throw new FatalError(`'${file}' is not included in project.`);
}
console.warn(`'${file}' does not exist. This will be an error in TSLint 6.`); // TODO make this an error in v6.0.0
logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0
}
}
}
Expand All @@ -217,15 +221,15 @@ function filterFiles(files: string[], patterns: string[], include: boolean): str
return files.filter((file) => include === matcher.some((pattern) => pattern.match(file)));
}

function resolveGlobs(files: string[], ignore: string[], outputAbsolutePaths?: boolean): string[] {
function resolveGlobs(files: string[], ignore: string[], outputAbsolutePaths: boolean | undefined, logger: Logger): string[] {
const results = flatMap(
files,
(file) => glob.sync(trimSingleQuotes(file), { ignore, nodir: true }),
);
// warn if `files` contains non-existent files, that are not patters and not excluded by any of the exclude patterns
for (const file of filterFiles(files, ignore, false)) {
if (!glob.hasMagic(file)) {
console.warn(`'${file}' does not exist. This will be an error in TSLint 6.`); // TODO make this an error in v6.0.0
if (!glob.hasMagic(file) && !results.some(createMinimatchFilter(file))) {
logger.error(`'${file}' does not exist. This will be an error in TSLint 6.\n`); // TODO make this an error in v6.0.0
}
}
const cwd = process.cwd();
Expand All @@ -248,17 +252,17 @@ async function doLinting(
let configFile: IConfigurationFile | undefined;

for (const file of files) {
const folder = path.dirname(file);
if (lastFolder !== folder) {
configFile = findConfiguration(possibleConfigAbsolutePath, folder).results;
lastFolder = folder;
}
if (isFileExcluded(file)) {
continue;
}

const contents = program !== undefined ? program.getSourceFile(file).text : await tryReadFile(file, logger);
if (contents !== undefined) {
const folder = path.dirname(file);
if (lastFolder !== folder) {
configFile = findConfiguration(possibleConfigAbsolutePath, folder).results;
lastFolder = folder;
}
linter.lint(file, contents, configFile);
}
}
Expand Down Expand Up @@ -287,7 +291,7 @@ async function tryReadFile(filename: string, logger: Logger): Promise<string | u
// MPEG transport streams use the '.ts' file extension. They use 0x47 as the frame
// separator, repeating every 188 bytes. It is unlikely to find that pattern in
// TypeScript source, so tslint ignores files with the specific pattern.
logger.error(`${filename}: ignoring MPEG transport stream`);
logger.error(`${filename}: ignoring MPEG transport stream\n`);
return undefined;
}
} finally {
Expand Down
31 changes: 14 additions & 17 deletions src/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import * as ts from "typescript";

import { Replacement } from "./language/rule/rule";
import * as Linter from "./linter";
import { Logger } from "./runner";
import { denormalizeWinPath, mapDefined, readBufferWithDetectedEncoding } from "./utils";
import { LintError } from "./verify/lintError";
import * as parse from "./verify/parse";
Expand Down Expand Up @@ -195,60 +196,57 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
return results;
}

export function consoleTestResultsHandler(testResults: TestResult[]): boolean {
export function consoleTestResultsHandler(testResults: TestResult[], logger: Logger): boolean {
let didAllTestsPass = true;

for (const testResult of testResults) {
if (!consoleTestResultHandler(testResult)) {
if (!consoleTestResultHandler(testResult, logger)) {
didAllTestsPass = false;
}
}

return didAllTestsPass;
}

export function consoleTestResultHandler(testResult: TestResult): boolean {
export function consoleTestResultHandler(testResult: TestResult, logger: Logger): boolean {
// needed to get colors to show up when passing through Grunt
(chalk as any).enabled = true;

let didAllTestsPass = true;

for (const fileName of Object.keys(testResult.results)) {
const results = testResult.results[fileName];
process.stdout.write(`${fileName}:`);
logger.log(`${fileName}:`);

/* tslint:disable:no-console */
if (results.skipped) {
console.log(chalk.yellow(` Skipped, requires typescript ${results.requirement}`));
logger.log(chalk.yellow(` Skipped, requires typescript ${results.requirement}\n`));
} else {
const markupDiffResults = diff.diffLines(results.markupFromMarkup, results.markupFromLinter);
const fixesDiffResults = diff.diffLines(results.fixesFromLinter, results.fixesFromMarkup);
const didMarkupTestPass = !markupDiffResults.some((hunk) => hunk.added === true || hunk.removed === true);
const didFixesTestPass = !fixesDiffResults.some((hunk) => hunk.added === true || hunk.removed === true);

if (didMarkupTestPass && didFixesTestPass) {
console.log(chalk.green(" Passed"));
logger.log(chalk.green(" Passed\n"));
} else {
console.log(chalk.red(" Failed!"));
logger.log(chalk.red(" Failed!\n"));
didAllTestsPass = false;
if (!didMarkupTestPass) {
displayDiffResults(markupDiffResults, MARKUP_FILE_EXTENSION);
displayDiffResults(markupDiffResults, MARKUP_FILE_EXTENSION, logger);
}
if (!didFixesTestPass) {
displayDiffResults(fixesDiffResults, FIXES_FILE_EXTENSION);
displayDiffResults(fixesDiffResults, FIXES_FILE_EXTENSION, logger);
}
}
}
/* tslint:enable:no-console */
}

return didAllTestsPass;
}

function displayDiffResults(diffResults: diff.IDiffResult[], extension: string) {
/* tslint:disable:no-console */
console.log(chalk.green(`Expected (from ${extension} file)`));
console.log(chalk.red("Actual (from TSLint)"));
function displayDiffResults(diffResults: diff.IDiffResult[], extension: string, logger: Logger) {
logger.log(chalk.green(`Expected (from ${extension} file)\n`));
logger.log(chalk.red("Actual (from TSLint)\n"));

for (const diffResult of diffResults) {
let color = chalk.grey;
Expand All @@ -257,7 +255,6 @@ function displayDiffResults(diffResults: diff.IDiffResult[], extension: string)
} else if (diffResult.removed) {
color = chalk.red.underline;
}
process.stdout.write(color(diffResult.value));
logger.log(color(diffResult.value));
}
/* tslint:enable:no-console */
}
23 changes: 10 additions & 13 deletions src/tslint-cli.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ interface Argv {
formattersDir: string;
format?: string;
typeCheck?: boolean;
test?: string;
test?: boolean;
version?: boolean;
}

Expand Down Expand Up @@ -244,16 +244,9 @@ if (argv.typeCheck) {
}
}

let log: (message: string) => void;
if (argv.out != undefined) {
const outputStream = fs.createWriteStream(argv.out, {
flags: "w+",
mode: 420,
});
log = (message) => outputStream.write(`${message}\n`);
} else {
log = console.log;
}
const outputStream: NodeJS.WritableStream = argv.out === undefined
? process.stdout
: fs.createWriteStream(argv.out, {flags: "w+", mode: 420});

run(
{
Expand All @@ -273,8 +266,12 @@ run(
typeCheck: argv.typeCheck,
},
{
log,
error: (m) => console.error(m),
log(m) {
outputStream.write(m);
},
error(m) {
process.stdout.write(m);
},
})
.then((rc) => {
process.exitCode = rc;
Expand Down
Loading

0 comments on commit 0b83cbe

Please sign in to comment.