Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: don't log stacktrace if cli succeeds (regression) #182

Merged
merged 8 commits into from
Mar 15, 2023
Merged

fix: don't log stacktrace if cli succeeds (regression) #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5586c25
fix: don't log stacktrace if cli succeeds
tommy-mitchell Mar 14, 2023
0a92e2e
fix: new line on failure
tommy-mitchell Mar 14, 2023
4e1dc85
tests: add `verifyCli` utility to check results
tommy-mitchell Mar 14, 2023
dafe675
fix(`test/cli.ts`): don’t test exact path in error stack
tommy-mitchell Mar 14, 2023
c7ec3a3
fix: cli success detection on Node 14
tommy-mitchell Mar 14, 2023
ee0bc03
fix: only throw on failure or if tsd finds errors
tommy-mitchell Mar 14, 2023
9e6a4ee
fix(`verifyCli`): exact matching
tommy-mitchell Mar 14, 2023
1a9c809
fix: stacktrace resolution for ci
tommy-mitchell Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
33 changes: 23 additions & 10 deletions source/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
})();
64 changes: 53 additions & 11 deletions source/test/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 => {
Expand All @@ -32,7 +38,11 @@ test('provide a path', async t => {
const {exitCode, stderr} = await t.throwsAsync<ExecaError>(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 => {
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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 => {
Expand All @@ -94,17 +116,29 @@ test('cli typings and files flags', async t => {
const {exitCode, stderr} = t.throws<ExecaError>(() => 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<ExecaError>(execa('../../../cli.js', {
const {exitCode, stderr} = await t.throwsAsync<ExecaError>(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 => {
Expand All @@ -114,10 +148,18 @@ test('exported formatter matches cli results', async t => {

const {stderr: cliResults} = await t.throwsAsync<ExecaError>(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',
]);
});
15 changes: 3 additions & 12 deletions source/test/diff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -41,8 +41,7 @@ test('diff cli', async t => {
const {exitCode, stderr} = await t.throwsAsync<ExecaError>(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; }',
Expand All @@ -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);
]);
});
21 changes: 20 additions & 1 deletion source/test/fixtures/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand All @@ -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!');
};