fix(config): Separate ENOENT error handler from others

ashaffer · ashaffer · commit e49dabe783d6 · 2013-04-08T00:07:13.000-07:00
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
diff --git a/lib/config.js b/lib/config.js
@@ -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});
@@ -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);
     }
diff --git a/test/unit/config.spec.coffee b/test/unit/config.spec.coffee
@@ -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"];'
@@ -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', {}
