Skip to content

Commit

Permalink
fix: improve error messages (#1536)
Browse files Browse the repository at this point in the history
  • Loading branch information
tatomyr authored Apr 25, 2024
1 parent 4cf08db commit 27ec10a
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 37 deletions.
64 changes: 36 additions & 28 deletions __tests__/commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import { readdirSync, statSync, existsSync } from 'fs';
import { join, relative } from 'path';
//@ts-ignore
import { toMatchSpecificSnapshot } from './specific-snapshot';
import { getCommandOutput, getEntrypoints, callSerializer, getParams } from './helpers';
import {
getCommandOutput,
getEntrypoints,
callSerializer,
getParams,
cleanupOutput,
} from './helpers';
import * as fs from 'fs';
import { spawnSync } from 'child_process';

Expand All @@ -27,7 +33,7 @@ describe('E2E', () => {

it(file, () => {
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
}
});
Expand All @@ -41,7 +47,7 @@ describe('E2E', () => {
join(testPath, './openapi.yaml'),
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

it('no-default-recommended-fallback', () => {
Expand All @@ -50,7 +56,7 @@ describe('E2E', () => {
join(testPath, './openapi.yaml'),
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
});

Expand Down Expand Up @@ -156,7 +162,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});

test('swagger', () => {
Expand All @@ -168,7 +174,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});

test('openapi with no errors', () => {
Expand All @@ -181,7 +187,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});

test('with separator: /', () => {
Expand All @@ -195,7 +201,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});

test('openapi json file', () => {
Expand All @@ -208,7 +214,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});

test('openapi json file refs validation', () => {
Expand Down Expand Up @@ -257,7 +263,7 @@ describe('E2E', () => {
});

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});
});

Expand All @@ -281,7 +287,7 @@ describe('E2E', () => {
const testPath = join(__dirname, `join/${dir}`);
const args = getParams('../../../packages/cli/src/index.ts', 'join', entrypoints);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
});

Expand All @@ -298,7 +304,7 @@ describe('E2E', () => {
const argsWithOptions = [...entrypoints, ...[`--${option.name}=${option.value}`]];
const args = getParams('../../../packages/cli/src/index.ts', 'join', argsWithOptions);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
});

Expand All @@ -309,7 +315,7 @@ describe('E2E', () => {
'pet.yaml',
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

describe('files with different extensions', () => {
Expand Down Expand Up @@ -348,7 +354,9 @@ describe('E2E', () => {
: parameters.entrypoints;
const args = getParams('../../../packages/cli/src/index.ts', 'join', argsWithOption);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, parameters.snapshot));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(
join(testPath, parameters.snapshot)
);
});
});
});
Expand All @@ -373,7 +381,7 @@ describe('E2E', () => {

it(file, () => {
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
}
});
Expand All @@ -389,7 +397,7 @@ describe('E2E', () => {
...entryPoints,
];
const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(
join(folderPath, 'remove-unused-components-snapshot.js')
);
});
Expand All @@ -404,7 +412,7 @@ describe('E2E', () => {
const entryPoints = getEntrypoints(folderPath);
const args = ['../../../../packages/cli/src/index.ts', 'bundle', ...entryPoints];
const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(
join(folderPath, 'remove-unused-components-snapshot.js')
);
});
Expand All @@ -419,7 +427,7 @@ describe('E2E', () => {
const entryPoints = getEntrypoints(folderPath);
const args = ['../../../../packages/cli/src/index.ts', 'bundle', ...entryPoints];
const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(
join(folderPath, 'without-remove-unused-components-snapshot.js')
);
}
Expand All @@ -435,7 +443,7 @@ describe('E2E', () => {
]);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});
});

Expand All @@ -445,7 +453,7 @@ describe('E2E', () => {
const args = getParams('../../../packages/cli/src/index.ts', 'bundle', ['test.yaml']);

const result = getCommandOutput(args, folderPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(folderPath, 'snapshot.js'));
});
});

Expand All @@ -456,21 +464,21 @@ describe('E2E', () => {
const testPath = join(folderPath, 'resolve-refs-in-preprocessors');
const args = getParams('../../../packages/cli/src/index.ts', 'bundle', ['openapi.yaml']);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

test('lint should resolve $refs in preprocessors', () => {
const testPath = join(folderPath, 'resolve-refs-in-preprocessors');
const args = getParams('../../../packages/cli/src/index.ts', 'lint', ['openapi.yaml']);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

test('stat should print the correct summary with $refs in preprocessors', () => {
const testPath = join(folderPath, 'resolve-refs-in-preprocessors');
const args = getParams('../../../packages/cli/src/index.ts', 'stats', ['openapi.yaml']);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
});

Expand All @@ -481,7 +489,7 @@ describe('E2E', () => {
const testPath = join(folderPath, 'simple-build-docs');
const args = getParams('../../../packages/cli/src/index.ts', 'build-docs', ['pets.yaml']);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));

expect(fs.existsSync(join(testPath, 'redoc-static.html'))).toEqual(true);
});
Expand All @@ -494,7 +502,7 @@ describe('E2E', () => {
'-o=nested/redoc-static.html',
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));

expect(fs.existsSync(join(testPath, 'nested/redoc-static.html'))).toEqual(true);
expect(fs.statSync(join(testPath, 'nested/redoc-static.html')).size).toEqual(33016);
Expand All @@ -508,7 +516,7 @@ describe('E2E', () => {
const testPath = join(folderPath, 'stats-stylish');
const args = getParams('../../../packages/cli/src/index.ts', 'stats', ['museum.yaml']);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

test('stats should produce correct JSON output', () => {
Expand All @@ -518,7 +526,7 @@ describe('E2E', () => {
'--format=json',
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});

test('stats should produce correct Markdown format', () => {
Expand All @@ -528,7 +536,7 @@ describe('E2E', () => {
'--format=markdown',
]);
const result = getCommandOutput(args, testPath);
(<any>expect(result)).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
(<any>expect(cleanupOutput(result))).toMatchSpecificSnapshot(join(testPath, 'snapshot.js'));
});
});
});
5 changes: 5 additions & 0 deletions __tests__/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,8 @@ export function callSerializer() {
print: (v: any) => cleanUpVersion(v),
});
}

export function cleanupOutput(message: string) {
const cwdRegexp = new RegExp(process.cwd(), 'g');
return message.replace(cwdRegexp, '.');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openapi: 3.1.0
components:
schemas:
Test:
type: string
12 changes: 12 additions & 0 deletions __tests__/lint/assertions-property-key-not-allowed/redocly.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apis:
main:
root: ./openapi.yaml

rules:
rule/not-allowed-property:
subject:
type: Schema
property: type # this is not allowed with the `disallowed` assertion
assertions:
disallowed:
- string
11 changes: 11 additions & 0 deletions __tests__/lint/assertions-property-key-not-allowed/snapshot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`E2E lint assertions-property-key-not-allowed 1`] = `
validating /openapi.yaml...
Something went wrong when processing ./__tests__/lint/assertions-property-key-not-allowed/openapi.yaml:
- The 'disallowed' assertion can't be used on properties. Please remove the 'property' key.
`;
5 changes: 5 additions & 0 deletions __tests__/lint/assertions-property-key-required/openapi.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
openapi: 3.1.0
components:
schemas:
Test:
type: string
11 changes: 11 additions & 0 deletions __tests__/lint/assertions-property-key-required/redocly.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apis:
main:
root: ./openapi.yaml

rules:
rule/required-property:
subject:
type: Schema
# Missing the 'property' key
assertions:
nonEmpty: true
11 changes: 11 additions & 0 deletions __tests__/lint/assertions-property-key-required/snapshot.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`E2E lint assertions-property-key-required 1`] = `
validating /openapi.yaml...
Something went wrong when processing ./__tests__/lint/assertions-property-key-required/openapi.yaml:
- The 'nonEmpty' assertion can't be used on all keys. Please provide a single property.
`;
6 changes: 3 additions & 3 deletions packages/cli/src/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ describe('handleErrors', () => {
});

it('should handle ResolveError', () => {
const resolveError = new ResolveError(new Error('File not found'));
const resolveError = new ResolveError(new Error('File not found.'));
expect(() => handleError(resolveError, ref)).toThrowError(HandledError);
expect(redColoretteMocks).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenCalledWith(
Expand All @@ -424,7 +424,7 @@ describe('handleErrors', () => {
});

it('should handle YamlParseError', () => {
const yamlParseError = new YamlParseError(new Error('Invalid yaml'), {} as any);
const yamlParseError = new YamlParseError(new Error('Invalid yaml.'), {} as any);
expect(() => handleError(yamlParseError, ref)).toThrowError(HandledError);
expect(redColoretteMocks).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenCalledWith(
Expand All @@ -451,7 +451,7 @@ describe('handleErrors', () => {
});

it('should throw unknown error', () => {
const testError = new Error('Test error');
const testError = new Error('Test error.');
expect(() => handleError(testError, ref)).toThrowError(HandledError);
expect(process.stderr.write).toHaveBeenCalledWith(
`Something went wrong when processing openapi/test.yaml:\n\n - Test error.\n\n`
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/utils/miscellaneous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ export function handleError(e: Error, ref: string) {
throw e;
}
case ResolveError:
return exitWithError(`Failed to resolve API description at ${ref}:\n\n - ${e.message}.`);
return exitWithError(`Failed to resolve API description at ${ref}:\n\n - ${e.message}`);
case YamlParseError:
return exitWithError(`Failed to parse API description at ${ref}:\n\n - ${e.message}.`);
return exitWithError(`Failed to parse API description at ${ref}:\n\n - ${e.message}`);
case CircularJSONNotSupportedError: {
return exitWithError(
`Detected circular reference which can't be converted to JSON.\n` +
Expand All @@ -307,7 +307,7 @@ export function handleError(e: Error, ref: string) {
case ConfigValidationError:
return exitWithError(e.message);
default: {
exitWithError(`Something went wrong when processing ${ref}:\n\n - ${e.message}.`);
exitWithError(`Something went wrong when processing ${ref}:\n\n - ${e.message}`);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/config/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export function mergeExtends(rulesConfList: ResolvedStyleguideConfig[]) {
for (const rulesConf of rulesConfList) {
if (rulesConf.extends) {
throw new Error(
`'extends' is not supported in shared configs yet: ${JSON.stringify(rulesConf, null, 2)}.`
`'extends' is not supported in shared configs yet:\n${JSON.stringify(rulesConf, null, 2)}`
);
}

Expand Down
Loading

1 comment on commit 27ec10a

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.05% 4451/5777
🟡 Branches 67.48% 2447/3626
🟡 Functions 70.73% 742/1049
🟡 Lines 77.24% 4185/5418

Test suite run success

734 tests passing in 102 suites.

Report generated by 🧪jest coverage report action from 27ec10a

Please sign in to comment.