BUG: refs #68. better handling in exec function for chdir, plus testing.

Author: Michael Grauer

library/KWUtils.php

@@ -141,19 +141,35 @@ public static function appendStringIfNot($subject, $ext)
    */
   public static function exec($command, &$output = null, $chdir = "", &$return_val = null)
     {
-    $currCwd = getcwd();  
-    if(!empty($chdir) && is_dir($chdir))
+    $changed = false;
+    if(!empty($chdir))
       {
-      if(!chdir($chdir))
+      if(is_dir($chdir))
         {
-        throw new Zend_Exception("Failed to change directory: [".$chdir."]");
+        if(!getcwd())
+          {
+          throw new Exception ('getcwd failed');
+          }
+        $currCwd = getcwd();
+
+        if(!chdir($chdir))
+          {
+          throw new Zend_Exception("Failed to change directory: [".$chdir."]");
+          }
+        $changed = true;
+        }
+      else
+        {
+        throw new Zend_Exception("passed in chdir is not a directory: [".$chdir."]");
         }
       }
+
     // on Linux need to add redirection to handle stderr
     $redirect_error = KWUtils::isLinux() ? " 2>&1" : "";
     exec(KWUtils::escapeCommand($command) . $redirect_error, $output, $return_val);
-    // change back to original directory
-    if(!chdir($currCwd))
+
+    // change back to original directory if necessary
+    if($changed && !chdir($currCwd))
       {
       throw new Zend_Exception("Failed to change back to original directory: [".$currCwd."]");
       }

modules/batchmake/tests/controllers/components/KWBatchmakeComponentTest.php

@@ -23,12 +23,16 @@ class KWBatchmakeComponentTest extends BatchmakeControllerTest
   protected $kwBatchmakeComponent;
   protected $applicationConfig;
 
+
+  protected $cwd;
+
   /** set up tests*/
   public function setUp()
     {
     $this->setupDatabase(array('default'));
     $this->_models = array('User');
     $this->enabledModules = array('batchmake');
+    $this->cwd = getcwd();
     parent::setUp();
     if(!isset($this->kwBatchmakeComponent))
       {
@@ -46,6 +50,8 @@ public function tearDown()
     $testTmpDir = $this->getTempDirectory() . '/batchmake/tests';
 
     KWUtils::recursiveRemoveDirectory($testTmpDir);
+    // change the current dir back to the saved cwd after each test
+    chdir($this->cwd);
     }
 
 

tests/library/KWUtilsTest.php

@@ -67,6 +67,10 @@ public function testExec()
     // not sure how to test this exactly, for now create a tmp dir, check
     // the value of pwd in it
 
+    $initialCwd = getcwd();
+    $output = null;
+    $returnVal = null;
+
     // create a tmp dir for this test
     $execDir = KWUtils::getTempDirectory() . '/KWUtilsTest';
     mkdir($execDir);
@@ -75,6 +79,12 @@ public function testExec()
     KWUtils::exec($cmd, $output, $chdir, $returnVal);
     // $output should have one value, the same as execDir
 
+    $postCwd = getcwd();
+    // check that we are back to the original directory after the exec
+    // we are already checking with this test that we chdir correctly because
+    // the test changes into a dir and then performs pwd there
+    $this->assertEquals($initialCwd, $postCwd, "exec didn't correctly return to the origin directory");//
+
     // yuck, need to do a bit of munging to get around tests/.. in BASE_PATH
     $execDir = str_replace('tests/../', '', $execDir);
     // and now replace any // with /,
@@ -87,6 +97,37 @@ public function testExec()
     $this->assertEquals($returnVal, 0);
     // now clean up the tmp dir
     rmdir($execDir);
+
+    // pass in a bad cwd, be sure that we get an exception
+    $chdir = "/this/dir/probably/will/not/exist/anywhere";
+    $output = null;
+    $returnVal = null;
+    try
+      {
+      KWUtils::exec($cmd, $output, $chdir, $returnVal);
+      $this->fail();
+      }
+    catch(Zend_Exception $ze)
+      {
+      // this is the correct behavior
+      $this->assertTrue(true);
+      }
+    $postCwd = getcwd();
+    // ensure we are still in the same directory
+    $this->assertEquals($initialCwd, $postCwd, "exec didn't correctly return to the origin directory");//
+
+    // now try to run pwd passing in a null chdir
+    // should return an output the same as the initialCwd
+    $chdir = null;
+    $output = null;
+    $returnVal = null;
+    KWUtils::exec($cmd, $output, $chdir, $returnVal);
+    // ensure that it ran in the initialCwd
+    $this->assertEquals($initialCwd, $output[0]);
+
+    // check that we are still in the output dir
+    $postCwd = getcwd();
+    $this->assertEquals($initialCwd, $postCwd);
     }
 
   /** tests appendStringIfNot function */