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 5 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
9 changes: 6 additions & 3 deletions source/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const cli = meow(`
});

(async () => {
let success = false;
tommy-mitchell marked this conversation as resolved.
Show resolved Hide resolved
try {
const cwd = cli.input.length > 0 ? cli.input[0] : process.cwd();
const {typings: typingsFile, files: testFiles, showDiff} = cli.flags;
Expand All @@ -54,14 +55,16 @@ const cli = meow(`
const diagnostics = await tsd(options);

if (diagnostics.length > 0) {
success = true;
throw new Error(formatter(diagnostics, showDiff));
}
tommy-mitchell marked this conversation as resolved.
Show resolved Hide resolved
} catch (error: unknown) {
const potentialError = error as Error | undefined;
const errorMessage = potentialError?.stack ?? potentialError?.message;

if (errorMessage) {
console.error(`Error running tsd: ${errorMessage}`);
if (success && potentialError?.message) {
console.error(potentialError?.message);
} else if (potentialError?.stack) {
console.error(`Error running tsd:\n${potentialError?.stack}`);
}

process.exit(1);
Expand Down
54 changes: 44 additions & 10 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,7 @@ import test from 'ava';
import execa from 'execa';
import readPkgUp from 'read-pkg-up';
import tsd, {formatter} from '..';
import {verifyCli} from './fixtures/utils';

interface ExecaError extends Error {
readonly exitCode: number;
Expand All @@ -15,7 +16,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 +37,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 +66,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 +84,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 +101,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 +115,26 @@ 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')
}));

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',
// TODO: check that stack matches without checking for exact filepath
// would have to match in CI and locally
], {startLine: 0});
tommy-mitchell marked this conversation as resolved.
Show resolved Hide resolved
});

test('exported formatter matches cli results', async t => {
Expand All @@ -114,7 +144,11 @@ 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);
Expand Down
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);
]);
});
24 changes: 23 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,25 @@ 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.
) => {
// NOTE: If lines are added to the output in the future `startLine` and `endLine` should be adjusted.
const endLine = startLine + expectedLines.length; // Grab diff output only and skip stack trace.

const receivedLines = diagnostics.trim().split('\n').slice(startLine, endLine).map(line => line.trim());

t.deepEqual(receivedLines, expectedLines, 'Received diagnostics that are different from expectations!');
};