BUG: Refs #212. Additional testing fixes, including move to Mock Objects.

Removed many redundant and unneeded testing files.
Changed the batchmake module tests around to use Mock Objects that simulate
BatchMake executable and condor_submit_dag executable, so these tests
can be run on systems without BatchMake and condor installed.
Adding in the Mock Objects also allowed more complete testing of the
KWBatchmakeComponent.

Author: Michael Grauer

modules/batchmake/controllers/components/KWBatchmakeComponent.php

@@ -30,7 +30,7 @@ class Batchmake_KWBatchmakeComponent extends AppComponent
    * accessor functin to return the names of the config propeties, and
    * their filesystem requirements;
    */
-  public function getConfigPropertiesRequirements()
+  public static function getConfigPropertiesRequirements()
     {
     return self::$configPropertiesRequirements;
     }
@@ -42,6 +42,14 @@ public function getConfigPropertiesRequirements()
   MIDAS_BATCHMAKE_CONDOR_SUBMIT_DAG => MIDAS_BATCHMAKE_CONDOR_BIN_DIR_PROPERTY,
   MIDAS_BATCHMAKE_EXE => MIDAS_BATCHMAKE_BIN_DIR_PROPERTY);
 
+  /**
+   * accessor functin to return the application names, along with their
+   * expected location property.
+   */
+  public static function getApplicationsPaths()
+    {
+    return self::$applicationsPaths;
+    }
 
 
   // component configuration settings
@@ -54,14 +62,26 @@ public function getConfigPropertiesRequirements()
   protected $configDataDir;
   protected $configCondorBinDir;
 
+
+  protected $executor;
+
   /**
    * Constructor, loads ini from standard config location, unless a
    * supplied alternateConfig.
    * @param string $alternateConfig path to alternative config ini file
    */
-  public function __construct($alternateConfig = null)
+  public function __construct($alternateConfig = null, $executor = null)
     {
     $this->loadConfigProperties($alternateConfig);
+    if(!isset($executor))
+      {
+      require_once BASE_PATH.'/modules/batchmake/library/Executor.php';
+      $this->executor = new Batchmake_Executor();
+      }
+    else
+      {
+      $this->executor = $executor;
+      }
     } // end __construct($alternateConfig)
 
 
@@ -521,11 +541,11 @@ public function compileBatchMakeScript($workDir, $bmScript)
       }
 
     // Run command
-    KWUtils::exec($cmd, $output, $workDir, $returnVal);
+    $this->executor->exec($cmd, $output, $workDir, $returnVal);
 
     if($returnVal !== 0)
       {
-      throw new Zend_Exception("Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
+      throw new Zend_Exception("compileBatchMakeScript: Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
       }
 
     // if BatchMake reports errors, throw an exception
@@ -541,12 +561,12 @@ public function compileBatchMakeScript($workDir, $bmScript)
           }
         else
           {
-          throw new Zend_Exception("Compiling script [".$bmScript."] yielded output: [".implode(",", $output )."]");
+          throw new Zend_Exception("compileBatchMakeScript: Compiling script [".$bmScript."] yielded output: [".implode(",", $output )."]");
           }
         }
       }
 
-    throw new Zend_Exception("Error in BatchMake script, the compile step didn't report errors, output: [".implode(",", $output )."]");
+    throw new Zend_Exception("compileBatchMakeScript: Error in BatchMake script, the compile step didn't report errors, output: [".implode(",", $output )."]");
     }
 
 
@@ -559,7 +579,15 @@ public function compileBatchMakeScript($workDir, $bmScript)
    */
   public function generateCondorDag($workDir, $bmScript)
     {
-    $dagName      = $bmScript.'.dagjob';
+    // remove .bms if it exists, for the creation of the dagjob
+    if(substr($bmScript, strlen($bmScript) - strlen('.bms') === '.bms'))
+      {
+      $dagName = substr($bmScript, 0, strlen($bmScript)-4) . '.dagjob';
+      }
+    else
+      {
+      $dagName      = $bmScript.'.dagjob';
+      }
 
     // Prepare command
     $params = array(
@@ -571,11 +599,11 @@ public function generateCondorDag($workDir, $bmScript)
     $cmd = KWUtils::prepareExecCommand($this->configBinDir . '/'. MIDAS_BATCHMAKE_EXE, $params);
 
     // Run command
-    KWUtils::exec($cmd, $output, $workDir, $returnVal);
+    $this->executor->exec($cmd, $output, $workDir, $returnVal);
 
     if($returnVal !== 0)
       {
-      throw new Zend_Exception("Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
+      throw new Zend_Exception("generateCondorDag: Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
       }
     return $dagName;
     }
@@ -594,11 +622,11 @@ public function condorSubmitDag($workDir, $dagScript)
     $cmd = KWUtils::prepareExecCommand($this->configCondorBinDir . '/'. MIDAS_BATCHMAKE_CONDOR_SUBMIT_DAG, $params);
 
     // Run command
-    KWUtils::exec($cmd, $output, $workDir, $returnVal);
+    $this->executor->exec($cmd, $output, $workDir, $returnVal);
 
     if($returnVal !== 0)
       {
-      throw new Zend_Exception("Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
+      throw new Zend_Exception("condorSubmitDag: Failed to run: [".$cmd."], output: [".implode(",", $output )."]");
       }
     }
 

modules/batchmake/library/Executor.php

@@ -0,0 +1,22 @@
+<?php
+/*=========================================================================
+MIDAS Server
+Copyright (c) Kitware SAS. 20 rue de la Villette. All rights reserved.
+69328 Lyon, FRANCE.
+
+See Copyright.txt for details.
+This software is distributed WITHOUT ANY WARRANTY; without even
+the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE.  See the above copyright notices for more information.
+=========================================================================*/
+include_once BASE_PATH . '/library/KWUtils.php';
+class Batchmake_Executor
+{
+  public function exec($command, &$output = null, $chdir = "", &$return_val = null)
+    {
+    KWUtils::exec($command, $output, $chdir, $return_val);  
+    }
+} // end class
+
+    
+    

modules/batchmake/tests/CMakeLists.txt

@@ -2,5 +2,6 @@ option( MIDAS_RUN_BATCHMAKE_TESTS "Should MIDAS run BatchMake tests?  Specific c
 if(MIDAS_RUN_BATCHMAKE_TESTS)
   add_subdirectory( controllers )
   add_midas_style_test( StyleTestsBatchmakeControllers ${CMAKE_SOURCE_DIR}/modules/batchmake/tests/controllers )
+  add_midas_style_test( StyleTestsBatchmakeLibrary ${CMAKE_SOURCE_DIR}/modules/batchmake/tests/library )
 endif()
 

modules/batchmake/tests/controllers/BatchmakeControllerTest.php

@@ -12,6 +12,7 @@
 
 // need to include the module constant for this test
 require_once BASE_PATH.'/modules/batchmake/constant/module.php';
+require_once BASE_PATH.'/modules/batchmake/controllers/components/KWBatchmakeComponent.php';
 require_once BASE_PATH.'/library/KWUtils.php';
 
 /** helper class used for testing batchmake module */
@@ -48,7 +49,7 @@ public function setupAndGetConfig()
     MIDAS_BATCHMAKE_SCRIPT_DIR_PROPERTY => $tmpDir.'/batchmake/tests/script',
     MIDAS_BATCHMAKE_APP_DIR_PROPERTY => $tmpDir.'/batchmake/tests/bin',
     MIDAS_BATCHMAKE_DATA_DIR_PROPERTY => $tmpDir.'/batchmake/tests/data',
-    MIDAS_BATCHMAKE_CONDOR_BIN_DIR_PROPERTY => '/home/condor/bin');//change this with mocks??
+    MIDAS_BATCHMAKE_CONDOR_BIN_DIR_PROPERTY => $tmpDir.'/batchmake/tests/condorbin');//change this with mocks??
     // now make sure these dirs exist
     // later can actually add some stuff to these dirs
     foreach($configProps as $prop => $dir)
@@ -71,6 +72,36 @@ public function setupAndGetConfig()
     $extension = '.bmm';
     $this->symlinkFileset($srcDir, $targetDir, $extension);
 
+
+    // the mock object strategy requires both an interface and for
+    // executable files to exist on disk in a particular location,
+    // so here we will create symlinks to a known executable
+    // ls
+    // which should be on most systems
+
+    $params = array('ls');
+    $cmd = KWUtils::prepareExeccommand('which', $params);
+    // dir doesn't matter, just send in srcDir as it is convenient
+    KWUtils::exec($cmd, $output, $srcDir, $returnVal);
+    if($returnVal !== 0 || !isset($output) || !isset($output[0]))
+      {
+      throw new Zend_Exception('Problem finding ls on your system, used for testing');
+      }
+    $pathToLs = $output[0];
+
+    // get the applications and their path properties from the component that
+    // expects them
+    $applicationsPaths = Batchmake_KWBatchmakeComponent::getApplicationsPaths();
+    foreach($applicationsPaths as $application => $pathProperty)
+      {
+      // now in the place of each executable, symlink the ls exe
+      $link = $configProps[$pathProperty] . '/' . $application;
+      if(!file_exists($link) && !symlink($pathToLs, $link))
+        {
+        throw new Zend_Exception($pathToLs . ' could not be sym-linked to ' . $link);
+        }
+      }
+
     return $configProps;
     }
 

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

@@ -34,7 +34,10 @@ public function setUp()
     if(!isset($this->kwBatchmakeComponent))
       {
       require_once BASE_PATH.'/modules/batchmake/controllers/components/KWBatchmakeComponent.php';
-      $this->kwBatchmakeComponent = new Batchmake_KWBatchmakeComponent($this->setupAndGetConfig());
+      require_once BASE_PATH.'/modules/batchmake/tests/library/ExecutorMock.php';
+      $executor = new Batchmake_ExecutorMock();
+      $this->kwBatchmakeComponent = new Batchmake_KWBatchmakeComponent($this->setupAndGetConfig(), $executor);
+//      $this->kwBatchmakeComponent = new Batchmake_KWBatchmakeComponent($this->setupAndGetConfig());
       }
     }
 
@@ -61,7 +64,7 @@ public function testGetBatchmakeScripts()
     {
     $foundTestScripts = $this->kwBatchmakeComponent->getBatchmakeScripts();
     sort($foundTestScripts);
-    $expectedTestScripts = array("Compiles.bms", "Myscript2.bms", "noscripts.bms",
+    $expectedTestScripts = array("CompileEmptyOutput.bms", "CompileReturnNonzero.bms", "Compiles.bms", "Myscript2.bms", "noscripts.bms",
         "anotherscript.bms", "anotherscriptwitherrors.bms", "bmmswitherrors.bms",
         "cycle1.bms", "cycle31.bms", "cycle32.bms", "cycle33.bms", "nocycle1.bms",
         "nocycle2.bms", "nocycle3.bms", "myscript.bms", "PixelCounter.bms",
@@ -232,9 +235,6 @@ public function testPreparePipelineBmms()
 
 
 
-
-
-
   /** tests testCompileBatchMakeScript */
   public function testCompileBatchMakeScript()
     {
@@ -250,7 +250,7 @@ public function testCompileBatchMakeScript()
     // this one should work
     $this->kwBatchmakeComponent->compileBatchMakeScript($workDir, $scriptName);
 
-    // now try a script that doesn't compile
+    // now try a script that doesn't compile but returns errors
     $scriptName = 'CompileErrors.bms';
     $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
     $bmms = $this->kwBatchmakeComponent->preparePipelineBmms($workDir, $bmScripts);
@@ -262,7 +262,41 @@ public function testCompileBatchMakeScript()
     catch(Zend_Exception $ze)
       {
       // if we got here, this is the correct behavior
-      $this->assertTrue(true);
+      // check that the exception came from the right place
+      $this->assertEquals(1, preg_match('/compileBatchMakeScript/', $ze->getMessage()));
+      }
+
+    // now try a script that returns a non-zero value from the compile step
+    $scriptName = 'CompileReturnNonzero.bms';
+    $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
+    $bmms = $this->kwBatchmakeComponent->preparePipelineBmms($workDir, $bmScripts);
+    try
+      {
+      $this->kwBatchmakeComponent->compileBatchMakeScript($workDir, $scriptName);
+      $this->fail('Should have had a compile error but did not, testCompileBatchMakeScript');
+      }
+    catch(Zend_Exception $ze)
+      {
+      // if we got here, this is the correct behavior
+      // check that the exception came from the right place
+      $this->assertEquals(1, preg_match('/compileBatchMakeScript/', $ze->getMessage()));
+      }
+
+
+    //a script that returns a zero value but an empty output from the compile step
+    $scriptName = 'CompileEmptyOutput.bms';
+    $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
+    $bmms = $this->kwBatchmakeComponent->preparePipelineBmms($workDir, $bmScripts);
+    try
+      {
+      $this->kwBatchmakeComponent->compileBatchMakeScript($workDir, $scriptName);
+      $this->fail('Should have had a compile error but did not, testCompileBatchMakeScript');
+      }
+    catch(Zend_Exception $ze)
+      {
+      // if we got here, this is the correct behavior
+      // check that the exception came from the right place
+      $this->assertEquals(1, preg_match('/compileBatchMakeScript/', $ze->getMessage()));
       }
 
     }
@@ -274,27 +308,36 @@ public function testGenerateCondorDag()
     $userDao = $this->User->load($usersFile[0]->getKey());
     $workDir = $this->kwBatchmakeComponent->createTask($userDao);
 
+    // try a script that returns a non-zero value
+    $scriptName = 'CompileReturnNonzero.bms';
+    $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
+    $bmms = $this->kwBatchmakeComponent->preparePipelineBmms($workDir, $bmScripts);
+    try
+      {
+      $dagJobFile = $this->kwBatchmakeComponent->generateCondorDag($workDir, $scriptName);
+      $this->fail('Should have had an error but did not, testGenerateCondorDag');
+      }
+    catch(Zend_Exception $ze)
+      {
+      // if we got here, this is the correct behavior
+      // check that the exception came from the right place
+      $this->assertEquals(1, preg_match('/generateCondorDag/', $ze->getMessage()));
+      }
+
     // a script that compiles
     $scriptName = 'Compiles.bms';
     $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
     $bmms = $this->kwBatchmakeComponent->preparePipelineBmms($workDir, $bmScripts);
 
     // try to generate the Condor script
     $dagJobFile = $this->kwBatchmakeComponent->generateCondorDag($workDir, $scriptName);
-    $this->assertEquals($dagJobFile, 'Compiles.bms.dagjob');
+    $this->assertEquals($dagJobFile, 'Compiles.dagjob');
     // check that dag files and condor job files were created
-    $condorFiles = array($dagJobFile, 'Compiles.1.bms.dagjob', 'Compiles.3.bms.dagjob', 'Compiles.5.bms.dagjob');
+    $condorFiles = array($dagJobFile, 'Compiles.1.dagjob', 'Compiles.3.dagjob', 'Compiles.5.dagjob');
     foreach($condorFiles as $condorFile)
       {
       $this->assertFileExists($workDir.'/'.$condorFile);
       }
-    // now look for some specific strings
-    $contents = file_get_contents($workDir.'/'. 'Compiles.bms.dagjob');
-    $dagjobStrings = array('Job job3', 'Job job5', 'PARENT job1 CHILD job3', 'PARENT job3 CHILD job5');
-    foreach($dagjobStrings as $string)
-      {
-      $this->assertTrue(preg_match("/".$string."/", $contents, $matches) === 1);
-      }
     }
 
 
@@ -305,6 +348,20 @@ public function testCondorSubmitDag()
     $userDao = $this->User->load($usersFile[0]->getKey());
     $workDir = $this->kwBatchmakeComponent->createTask($userDao);
 
+    // try a script that returns a non-zero value
+    $scriptName = 'CompileReturnNonzero.dagjob';
+    try
+      {
+      $this->kwBatchmakeComponent->condorSubmitDag($workDir, $scriptName);
+      $this->fail('Should have had an error but did not, testCondorSubmitDag');
+      }
+    catch(Zend_Exception $ze)
+      {
+      // if we got here, this is the correct behavior
+      // check that the exception came from the right place
+      $this->assertEquals(1, preg_match('/condorSubmitDag/', $ze->getMessage()));
+      }
+
     // a script that compiles
     $scriptName = 'Compiles.bms';
     $bmScripts = $this->kwBatchmakeComponent->preparePipelineScripts($workDir, $scriptName);
@@ -313,6 +370,9 @@ public function testCondorSubmitDag()
     $dagScript = $this->kwBatchmakeComponent->generateCondorDag($workDir, $scriptName);
     $this->kwBatchmakeComponent->condorSubmitDag($workDir, $dagScript);
     // how to check this now?
+    // TODO think about this some more
+    // perhaps send something back through the web-api
+    // what would that be testing exactly?
     }