From 2dc53582c969bd9e2abe24f577712c336e004a1c Mon Sep 17 00:00:00 2001 From: Tommy Date: Wed, 15 Mar 2023 12:54:07 -0500 Subject: [PATCH] Don't log stacktrace if CLI succeeds (#182) --- package.json | 1 + source/cli.ts | 33 ++++++++++++------ source/test/cli.ts | 64 +++++++++++++++++++++++++++++------ source/test/diff.ts | 15 ++------ source/test/fixtures/utils.ts | 21 +++++++++++- 5 files changed, 100 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 1bc87b81..52b9084a 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ "eslint-config-xo-typescript": "^0.41.1", "execa": "^5.0.0", "react": "^16.9.0", + "resolve-from": "^5.0.0", "rxjs": "^6.5.3", "typescript": "~4.9.5" }, diff --git a/source/cli.ts b/source/cli.ts index c6dbed5f..89ac8e6c 100644 --- a/source/cli.ts +++ b/source/cli.ts @@ -44,26 +44,39 @@ const cli = meow(` }, }); +/** + * Displays a message and exits, conditionally erroring. + * + * @param message The message to display. + * @param isError Whether or not to fail on exit. + */ +const exit = (message: string, {isError = true}: {isError?: boolean} = {}) => { + if (isError) { + console.error(message); + process.exit(1); + } else { + console.log(message); + process.exit(0); + } +}; + (async () => { try { const cwd = cli.input.length > 0 ? cli.input[0] : process.cwd(); const {typings: typingsFile, files: testFiles, showDiff} = cli.flags; - const options = {cwd, typingsFile, testFiles}; - - const diagnostics = await tsd(options); + const diagnostics = await tsd({cwd, typingsFile, testFiles}); if (diagnostics.length > 0) { - throw new Error(formatter(diagnostics, showDiff)); + const hasErrors = diagnostics.some(diagnostic => diagnostic.severity === 'error'); + const formattedDiagnostics = formatter(diagnostics, showDiff); + + exit(formattedDiagnostics, {isError: hasErrors}); } } catch (error: unknown) { const potentialError = error as Error | undefined; - const errorMessage = potentialError?.stack ?? potentialError?.message; - - if (errorMessage) { - console.error(`Error running tsd: ${errorMessage}`); - } + const errorMessage = potentialError?.stack ?? potentialError?.message ?? 'tsd unexpectedly crashed.'; - process.exit(1); + exit(`Error running tsd:\n${errorMessage}`); } })(); diff --git a/source/test/cli.ts b/source/test/cli.ts index a549d98b..1f5d510a 100644 --- a/source/test/cli.ts +++ b/source/test/cli.ts @@ -3,6 +3,8 @@ import test from 'ava'; import execa from 'execa'; import readPkgUp from 'read-pkg-up'; import tsd, {formatter} from '..'; +import {verifyCli} from './fixtures/utils'; +import resolveFrom from 'resolve-from'; interface ExecaError extends Error { readonly exitCode: number; @@ -15,7 +17,11 @@ test('fail if errors are found', async t => { })); t.is(exitCode, 1); - t.regex(stderr, /5:19[ ]{2}Argument of type number is not assignable to parameter of type string./); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }); test('succeed if no errors are found', async t => { @@ -32,7 +38,11 @@ test('provide a path', async t => { const {exitCode, stderr} = await t.throwsAsync(execa('dist/cli.js', [file])); t.is(exitCode, 1); - t.regex(stderr, /5:19[ ]{2}Argument of type number is not assignable to parameter of type string./); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }); test('cli help flag', async t => { @@ -57,7 +67,11 @@ test('cli typings flag', async t => { })); t.is(exitCode, 1); - t.true(stderr.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }; await runTest('--typings'); @@ -71,7 +85,11 @@ test('cli files flag', async t => { })); t.is(exitCode, 1); - t.true(stderr.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }; await runTest('--files'); @@ -84,7 +102,11 @@ test('cli files flag array', async t => { })); t.is(exitCode, 1); - t.true(stderr.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }); test('cli typings and files flags', async t => { @@ -94,17 +116,29 @@ test('cli typings and files flags', async t => { const {exitCode, stderr} = t.throws(() => execa.commandSync(`dist/cli.js -t ${typingsFile} -f ${testFile}`)); t.is(exitCode, 1); - t.true(stderr.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, stderr, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }); test('tsd logs stacktrace on failure', async t => { - const {exitCode, stderr, stack} = await t.throwsAsync(execa('../../../cli.js', { + const {exitCode, stderr} = await t.throwsAsync(execa('../../../cli.js', { cwd: path.join(__dirname, 'fixtures/empty-package-json') })); + const nodeModulesPath = path.resolve('node_modules'); + const parseJsonPath = resolveFrom.silent(`${nodeModulesPath}/read-pkg`, 'parse-json') ?? `${nodeModulesPath}/index.js`; + t.is(exitCode, 1); - t.true(stderr.includes('Error running tsd: JSONError: Unexpected end of JSON input while parsing empty string')); - t.truthy(stack); + verifyCli(t, stderr, [ + 'Error running tsd:', + 'JSONError: Unexpected end of JSON input while parsing empty string', + `at parseJson (${parseJsonPath}:29:21)`, + `at module.exports (${nodeModulesPath}/read-pkg/index.js:17:15)`, + `at async module.exports (${nodeModulesPath}/read-pkg-up/index.js:14:16)`, + ], {startLine: 0}); }); test('exported formatter matches cli results', async t => { @@ -114,10 +148,18 @@ test('exported formatter matches cli results', async t => { const {stderr: cliResults} = await t.throwsAsync(execa('../../../cli.js', options)); - t.true(cliResults.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, cliResults, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); const tsdResults = await tsd(options); const formattedResults = formatter(tsdResults); - t.true(formattedResults.includes('✖ 5:19 Argument of type number is not assignable to parameter of type string.')); + verifyCli(t, formattedResults, [ + '✖ 5:19 Argument of type number is not assignable to parameter of type string.', + '', + '1 error', + ]); }); diff --git a/source/test/diff.ts b/source/test/diff.ts index e2a26a1a..b8867618 100644 --- a/source/test/diff.ts +++ b/source/test/diff.ts @@ -1,4 +1,4 @@ -import {verifyWithDiff} from './fixtures/utils'; +import {verifyWithDiff, verifyCli} from './fixtures/utils'; import execa, {ExecaError} from 'execa'; import path from 'path'; import test from 'ava'; @@ -41,8 +41,7 @@ test('diff cli', async t => { const {exitCode, stderr} = await t.throwsAsync(execa('dist/cli.js', [file, '--show-diff'])); t.is(exitCode, 1); - - const expectedLines = [ + verifyCli(t, stderr, [ '✖ 8:0 Parameter type { life?: number | undefined; } is declared too wide for argument type { life: number; }.', '', '- { life?: number | undefined; }', @@ -69,13 +68,5 @@ test('diff cli', async t => { '+ This is a comment.', '', '6 errors' - ]; - - // NOTE: If lines are added to the output in the future startLine and endLine should be adjusted. - const startLine = 2; // Skip tsd error message and file location. - const endLine = startLine + expectedLines.length; // Grab diff output only and skip stack trace. - - const receivedLines = stderr.trim().split('\n').slice(startLine, endLine).map(line => line.trim()); - - t.deepEqual(receivedLines, expectedLines); + ]); }); diff --git a/source/test/fixtures/utils.ts b/source/test/fixtures/utils.ts index 4cbea202..a65fdfd5 100644 --- a/source/test/fixtures/utils.ts +++ b/source/test/fixtures/utils.ts @@ -94,7 +94,7 @@ export const verifyWithFileName = ( * @param diagnostics - List of diagnostics to verify. * @param expectations - Expected diagnostics. */ - export const verifyWithDiff = ( +export const verifyWithDiff = ( t: ExecutionContext, diagnostics: Diagnostic[], expectations: ExpectationWithDiff[] @@ -117,3 +117,22 @@ export const verifyWithFileName = ( t.deepEqual(diagnosticObjs, expectationObjs, 'Received diagnostics that are different from expectations!'); }; + +/** + * Verify a list of diagnostics reported from the CLI. + * + * @param t - The AVA execution context. + * @param diagnostics - List of diagnostics to verify. + * @param expectations - Expected diagnostics. + * @param startLine - Optionally specify how many lines to skip from start. + */ +export const verifyCli = ( + t: ExecutionContext, + diagnostics: string, + expectedLines: string[], + {startLine}: {startLine: number} = {startLine: 1} // Skip file location. +) => { + const receivedLines = diagnostics.trim().split('\n').slice(startLine).map(line => line.trim()); + + t.deepEqual(receivedLines, expectedLines, 'Received diagnostics that are different from expectations!'); +};