Merge pull request #95 from chafnan/master

fix #65: Added message back to user when the path given does not exist
nathanboktae · Sep 26, 2013 · 08f57df · 08f57df
2 parents b71c3a6 + 065b6a0
commit 08f57df
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 22 deletions.
diff --git a/bin/mocha-phantomjs b/bin/mocha-phantomjs
@@ -87,38 +87,78 @@ var config = JSON.stringify({
   viewportSize: program.view,
   useColors: program.color
 });
-var spawnArgs = [script, page, reporter, config];
 
-var phantomjs;
-if (program.path !== undefined) {
-  phantomjs = spawn(program.path, spawnArgs);
+function startPhantomJS(cmdPath, args, callback) {
+  var cmd = getCmdFromPath(cmdPath)
+  if (cmd != null) {
+    return spawnPhantomJS(cmd, args)
+  }
+
+  getCmdByEnvironmentPath(function (err, cmdPath) {
+    if(err || !cmdPath) {
+      getCmdByModulePath(function(err, cmdPath) {
+        if (err) {
+          console.error('PhantomJS was not found.');
+          process.exit(1);
+        }
+        spawnPhantomJS(cmdPath, args)
+      });
+    } else {
+      spawnPhantomJS(cmdPath, args);
+    }
+  })
 }
-if (phantomjs === undefined) {
-  cmd = which.sync('phantomjs');
-  if (cmd) {
-    phantomjs = spawn(cmd, spawnArgs);
+
+function spawnPhantomJS(cmdPath, args) {
+  var phantomjs = spawn(cmdPath, args);
+
+  phantomjs.stdout.pipe(process.stdout);
+  phantomjs.stderr.pipe(process.stdout);
+
+  phantomjs.on('exit', function(code){
+    if (code === 127) { print("Perhaps phantomjs is not installed?\n"); }
+    process.exit(code);
+  });
+}
+
+function getCmdFromPath(cmdPath, callback) {
+  if (cmdPath === undefined) { return };
+  var result = null,
+      errorMessage = null,
+      tempPath = path.resolve(cmdPath);
+
+  if (exists(tempPath)) {
+    result = tempPath;
+  } else {
+    errorMessage = "PhantomJS does not exist at '" + cmdPath + "'. Looking for PhantomJS in the PATH."
+    if (!callback) {
+      console.error(errorMessage);
+    }
+  }
+
+  if (callback) {
+    callback(errorMessage, result);
+  } else {
+    return result;
   }
 }
-if (phantomjs === undefined) {
+
+function getCmdByEnvironmentPath(callback) {
+  which('phantomjs', callback);
+}
+
+function getCmdByModulePath(callback) {
   for (var i=0; i < module.paths.length; i++) {
     var bin = path.join(module.paths[i], '.bin/phantomjs');
     if (process.platform === 'win32') {
       bin += '.cmd';
     }
     if (exists(bin)) {
-      phantomjs = spawn(bin, spawnArgs);
-      break;
+      return callback(null, bin);
     }
   }
-}
-if (phantomjs === undefined) { phantomjs = spawn('phantomjs', spawnArgs); }
+  return callback('PhantomJS not found.', null)
 
-phantomjs.stdout.on('data', function(data){
-  print(data.toString());
-})
-
-phantomjs.on('exit', function(code){
-  if (code === 127) { print("Perhaps phantomjs is not installed?\n"); }
-  process.exit(code);
-})
+}
 
+startPhantomJS(program.path, [script, page, reporter, config]);
diff --git a/test/lib/mocha-phantomjs.js b/test/lib/mocha-phantomjs.js
diff --git a/test/src/mocha-phantomjs.coffee b/test/src/mocha-phantomjs.coffee
@@ -278,6 +278,14 @@ describe 'mocha-phantomjs', ->
         @runner done, ['-C', fileURL('mixed')], (code, stdout, stderr) ->
           expect(stdout).to.not.match /\u001b\[\d\dm/
 
+    describe 'path', ->
+      it 'has used custom path', (done) ->
+        # test for issue #65
+        # assumption here is that phantomjs is working correctly, so giving the wrong path
+        # will cause the runner to fail, there should be an output to explain this to the user
+        @runner done, ['-p', 'fake/path/to/phantomjs', fileURL('passing')], (code, stdout, stderr) ->
+          expect(stderr).to.contain "PhantomJS does not exist at 'fake/path/to/phantomjs'. Looking for PhantomJS in the PATH."
+
   describe 'env', ->
     it 'has passed environment variables', (done) ->
       process.env.FOO = 'bar'