Skip to content

Commit

Permalink
fix(config): Separate ENOENT error handler from others
Browse files Browse the repository at this point in the history
This prevents karma from incorrectly reporting a config file not found error in the case where the code inside the config file throws an ENOENT error for other reasons

style(config): indentation, error messages

Fix indentation mistake, improve error message, add process.exit(1)

test(config): add missing test

to ensure proper error reporting when a config file throws an ENOENT error of its own

style(config): whitespace and var scope

Removed trailing whitespace and moved configSrc declaration outside of the try/catch block
  • Loading branch information
ashaffer committed Apr 8, 2013
1 parent ac1e227 commit e49dabe
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
15 changes: 12 additions & 3 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,20 @@ var readConfigFile = function(filepath) {
}});
});

var configSrc;
try {
var configSrc = fs.readFileSync(filepath);
configSrc = fs.readFileSync(filepath);
} catch(e) {
if (e.code === 'ENOENT' || e.code === 'EISDIR') {
log.error('Config file does not exist!');
} else {
log.error('Unexpected error opening config file!\n', e);
}

process.exit(1);
}

try {
// if the configuration file is coffeescript compile it
if (path.extname(filepath) === '.coffee') {
configSrc = coffee.compile(configSrc.toString(), {bare: true});
Expand All @@ -173,8 +184,6 @@ var readConfigFile = function(filepath) {
} catch(e) {
if (e.name === 'SyntaxError') {
log.error('Syntax error in config file!\n' + e.message);
} else if (e.code === 'ENOENT' || e.code === 'EISDIR') {
log.error('Config file does not exist!');
} else {
log.error('Invalid config file!\n', e);
}
Expand Down
10 changes: 10 additions & 0 deletions test/unit/config.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe 'config', ->
'config5.js': fsMock.file 0, 'port = {f: __filename, d: __dirname}' # piggyback on port prop
'config6.js': fsMock.file 0, 'reporters = "junit";'
'config7.js': fsMock.file 0, 'browsers = ["Chrome", "Firefox"];'
'config8.js': fsMock.file 0, 'require("fs").readFileSync("/not/a/real/file/path")'
conf:
'invalid.js': fsMock.file 0, '={function'
'exclude.js': fsMock.file 0, 'exclude = ["one.js", "sub/two.js"];'
Expand Down Expand Up @@ -118,6 +119,15 @@ describe 'config', ->
expect(event.data).to.be.deep.equal ['Config file does not exist!']
expect(mocks.process.exit).to.have.been.calledWith 1

it 'should not log config file does not exist if config file throws an ENOENT', ->
e.parseConfig '/home/config8.js', {}

expect(logSpy).to.have.been.called
event = logSpy.lastCall.args[0]
expect(event.level.toString()).to.be.equal 'ERROR'
expect(event.data).to.be.not.deep.equal ['Config file does not exist!']
expect(mocks.process.exit).to.have.been.calledWith 1


it 'should log error and exit if it is a directory', ->
e.parseConfig '/conf', {}
Expand Down

0 comments on commit e49dabe

Please sign in to comment.