Skip to content

Commit

Permalink
Gracefully handle syntax error when importing spec file
Browse files Browse the repository at this point in the history
Avoid bailing out when using the importer to read in a spec file, if it
has a syntax error. Instead, add a suite and an always-failing spec that
highlights the location of the syntax error. (Previously, an unhelpful
error message would be printed, showing the stack trace of where the
spec was imported, not the location of the syntax error within the
spec.)

Closes: #13
  • Loading branch information
ptomato committed Aug 25, 2020
1 parent 4723ba9 commit e8f10a8
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 2 deletions.
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Copied from Jasmine, use the original project's coding style
lib/jasmine.js
test/jasmineIntegrationTest.js

# Deliberate syntax error
test/fixtures/syntaxErrorSpec.js
15 changes: 14 additions & 1 deletion src/jasmineBoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,20 @@ var Jasmine = class Jasmine {
// directories
imports.searchPath.unshift(modulePath);
specImporter.searchPath.unshift(modulePath);
void specImporter[moduleName];
try {
void specImporter[moduleName];
} catch (err) {
if (!(err instanceof SyntaxError))
throw err;
// Fake failing suite, to log a failure but continue on with
// other specs
globalThis.describe(file, function () {
globalThis.it('did not import correctly', function () {
const {fileName, lineNumber, columnNumber, message} = err;
globalThis.fail(`${fileName}:${lineNumber}:${columnNumber}: ${message}`);
});
});
}
imports.searchPath = oldSearchPath;

// Make a new copy of the importer in case we need to import another
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/syntaxErrorSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
lett foo = 4;
describe('SyntaxError suite', function () {});
8 changes: 7 additions & 1 deletion test/jasmineBootSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ describe('Jasmine boot', function () {
`${SRCDIR}test/fixtures/path1/test.js`,
`${SRCDIR}test/fixtures/path2/test.js`,
`${SRCDIR}test/fixtures/someSpec.js`,
`${SRCDIR}test/fixtures/syntaxErrorSpec.js`,
]);
expect(testJasmine.specFiles.every(path => path.indexOf('notASpec.txt') === -1)).toBe(true);
});
Expand All @@ -133,7 +134,7 @@ describe('Jasmine boot', function () {
});

it('respects excluded files', function () {
testJasmine.exclusions = ['otherSpec.js'];
testJasmine.exclusions = ['otherSpec.js', 'syntaxErrorSpec.js'];
testJasmine.addSpecFiles([`${SRCDIR}test/fixtures`]);
expect(testJasmine.specFiles).toMatchAllFiles([
`${SRCDIR}test/fixtures/someSpec.js`,
Expand Down Expand Up @@ -171,4 +172,9 @@ describe('Jasmine boot', function () {
expect(() => testJasmine.loadSpecs()).toThrowError(Error,
'Catch this error to ensure this file is loaded');
});

it('does not bail out altogether if one of the specs has a syntax error', function () {
testJasmine.addSpecFiles([`${SRCDIR}test/fixtures/syntaxErrorSpec.js`]);
expect(() => testJasmine.loadSpecs()).not.toThrow();
});
});

0 comments on commit e8f10a8

Please sign in to comment.