Fixes acquia#4513 to refactor due to symfony/process update. (acquia#…

…4514) * Fixes acquia#4513: updating other inspector uses of drush. * refactoring test use of drush commands. * Adding backwards compatibility string to array conversion for drush and execution. Co-authored-by: Mike Madison <>
mikemadison13 · May 6, 2022 · 57cf3d7 · 57cf3d7
1 parent b4dece2
commit 57cf3d7
Show file tree

Hide file tree

Showing 13 changed files with 172 additions and 48 deletions.
diff --git a/src/Robo/Commands/Tests/ServerCommand.php b/src/Robo/Commands/Tests/ServerCommand.php
@@ -54,7 +54,7 @@ public function launchWebServer() {
     /** @var \Acquia\Blt\Robo\Common\Executor $executor */
     $executor = $this->getContainer()->get('executor');
     $result = $executor
-      ->drush("runserver --quiet $this->serverUrl > $log_file 2>&1")
+      ->drush(["runserver", "--quiet", "$this->serverUrl > $log_file 2>&1"])
       ->background(TRUE)
       ->run();
 

diff --git a/src/Robo/Common/Executor.php b/src/Robo/Common/Executor.php
@@ -56,7 +56,8 @@ public function getBuilder() {
    * Wrapper for taskExec().
    *
    * @param string $command
-   *   The command to execute.
+   *   The command string|array.
+   *   Warning: symfony/process 5.x expects an array.
    *
    * @return \Robo\Task\Base\Exec
    *   The task. You must call run() on this to execute it!
@@ -68,26 +69,33 @@ public function taskExec($command) {
   /**
    * Executes a drush command.
    *
-   * @param string $command
+   * @param mixed $command
    *   The command to execute, without "drush" prefix.
    *
    * @return \Robo\Common\ProcessExecutor
    *   The unexecuted process.
    */
   public function drush($command) {
+    $drush_array = [];
+
+    // Backwards compatibility check for legacy commands.
+    if (!is_array($command)) {
+      $this->say(StringManipulator::stringToArrayMsg());
+      $command = StringManipulator::commandConvert($command);
+    }
+
     // @todo Set to silent if verbosity is less than very verbose.
-    $bin = $this->getConfigValue('composer.bin');
-    /** @var \Robo\Common\ProcessExecutor $process_executor */
-    $drush_alias = $this->getConfigValue('drush.alias');
-    $command_string = $bin . DIRECTORY_SEPARATOR . "drush @$drush_alias $command";
+    $drush_array[] = $this->getConfigValue('composer.bin') . DIRECTORY_SEPARATOR . "drush";
+    $drush_array[] = "@" . $this->getConfigValue('drush.alias');
 
     // URIs do not work on remote drush aliases in Drush 9. Instead, it is
     // expected that the alias define the uri in its configuration.
-    if ($drush_alias != 'self') {
-      $command_string .= ' --uri=' . $this->getConfigValue('site');
+    if ($this->getConfigValue('drush.alias') != 'self') {
+      $drush_array[] = ' --uri=' . $this->getConfigValue('site');
     }
+    $command_array = array_merge($drush_array, $command);
 
-    $process_executor = Robo::process(new Process($command_string));
+    $process_executor = Robo::process(new Process($command_array));
 
     return $process_executor->dir($this->getConfigValue('docroot'))
       ->interactive(FALSE)
@@ -99,13 +107,19 @@ public function drush($command) {
   /**
    * Executes a command.
    *
-   * @param string $command
-   *   The command.
+   * @param mixed $command
+   *   The command string|array.
+   *   Warning: symfony/process 5.x expects an array.
    *
    * @return \Robo\Common\ProcessExecutor
    *   The unexecuted command.
    */
   public function execute($command) {
+    // Backwards compatibility check for legacy commands.
+    if (!is_array($command)) {
+      $this->say(StringManipulator::stringToArrayMsg());
+      $command = StringManipulator::commandConvert($command);
+    }
     /** @var \Robo\Common\ProcessExecutor $process_executor */
     $process_executor = Robo::process(new Process($command));
     return $process_executor->dir($this->getConfigValue('repo.root'))

diff --git a/src/Robo/Common/StringManipulator.php b/src/Robo/Common/StringManipulator.php
@@ -95,4 +95,27 @@ public static function convertStringToPrefix($string) {
     return strtoupper($prefix);
   }
 
+  /**
+   * Converts a command string to command array.
+   *
+   * @param string $command
+   *   The command string to conver to array.
+   *
+   * @return array
+   *   Command array.
+   */
+  public static function commandConvert($command) {
+    return explode(" ", $command);
+  }
+
+  /**
+   * Provides a common warning for command string / array use.
+   *
+   * @return string
+   *   The deprecation warning.
+   */
+  public static function stringToArrayMsg() {
+    return "Deprecation Warning: this command is passing a command string and should pass a command arary.";
+  }
+
 }
diff --git a/src/Robo/Doctor/WebUriCheck.php b/src/Robo/Doctor/WebUriCheck.php
@@ -49,7 +49,12 @@ protected function checkUri() {
    * Checks that configured URI responds to requests.
    */
   protected function checkUriResponse() {
-    $site_available = $this->getExecutor()->execute("curl -I --insecure " . $this->drushStatus['uri'])->run()->wasSuccessful();
+    $site_available = $this->getExecutor()->execute([
+      "curl",
+      "-I",
+      "--insecure",
+      $this->drushStatus['uri'],
+    ])->run()->wasSuccessful();
     if (!$site_available) {
       $this->logProblem(__FUNCTION__, [
         "Did not get a response from {$this->drushStatus['uri']}",
@@ -66,7 +71,11 @@ protected function checkUriResponse() {
    */
   protected function checkHttps() {
     if (strstr($this->drushStatus['uri'], 'https')) {
-      if (!$this->getExecutor()->execute('curl -cacert ' . $this->drushStatus['uri'])->run()->wasSuccessful()) {
+      if (!$this->getExecutor()->execute([
+        'curl',
+        '-cacert',
+        $this->drushStatus['uri'],
+      ])->run()->wasSuccessful()) {
         $this->logProblem(__FUNCTION__, [
           "The SSL certificate for your local site appears to be invalid for {$this->drushStatus['uri']}.",
         ], 'error');

diff --git a/src/Robo/Inspector/Inspector.php b/src/Robo/Inspector/Inspector.php
@@ -190,7 +190,7 @@ public function isDrupalSettingsFileValid() {
   public function isDrupalInstalled() {
     $this->logger->debug("Verifying that Drupal is installed...");
     $uri = $this->getConfigValue('drush.uri');
-    $result = $this->executor->drush("--uri=$uri status bootstrap")->run();
+    $result = $this->executor->drush(["--uri=$uri", "status", "bootstrap"])->run();
     $output = trim($result->getMessage());
     $installed = $result->wasSuccessful() && strpos($output, 'Drupal bootstrap : Successful') !== FALSE;
     $this->logger->debug("Drupal bootstrap results: $output");
@@ -205,7 +205,13 @@ public function isDrupalInstalled() {
    *   The result of `drush status`.
    */
   public function getDrushStatus() {
-    $status_info = (array) json_decode($this->executor->drush('status --format=json --fields=*')->run()->getMessage(), TRUE);
+    $docroot = $this->getConfigValue('docroot');
+    $status_info = (array) json_decode($this->executor->drush([
+      'status',
+      '--format=json',
+      '--fields=*',
+      "--root=$docroot",
+    ])->run()->getMessage(), TRUE);
 
     return $status_info;
   }
@@ -253,9 +259,11 @@ public function getStatus() {
    *   TRUE if alias is valid.
    */
   public function isDrushAliasValid($alias) {
-    return $this->executor->drush("site:alias @$alias --format=json")
-      ->run()
-      ->wasSuccessful();
+    return $this->executor->drush([
+      "site:alias",
+      "@$alias",
+      "--format=json",
+    ])->run()->wasSuccessful();
   }
 
   /**
@@ -308,7 +316,7 @@ public function isMySqlAvailable() {
   public function getMySqlAvailable() {
     $this->logger->debug("Verifying that MySQL is available...");
     /** @var \Robo\Result $result */
-    $result = $this->executor->drush("sqlq \"SHOW DATABASES\"")
+    $result = $this->executor->drush(["sqlq", "SHOW DATABASES"])
       ->run();
 
     return $result->wasSuccessful();
@@ -341,7 +349,7 @@ public function isPostgreSqlAvailable() {
   public function getPostgreSqlAvailable() {
     $this->logger->debug("Verifying that PostgreSQL is available...");
     /** @var \Robo\Result $result */
-    $result = $this->executor->drush("sqlq \"SHOW DATABASES\"")
+    $result = $this->executor->drush(["sqlq \"SHOW DATABASES\""])
       ->run();
 
     return $result->wasSuccessful();
@@ -374,7 +382,7 @@ public function isSqliteAvailable() {
   public function getSqliteAvailable() {
     $this->logger->debug("Verifying that Sqlite is available...");
     /** @var \Robo\Result $result */
-    $result = $this->executor->drush("sqlq \".tables\"")
+    $result = $this->executor->drush(["sqlq", ".tables"])
       ->run();
 
     return $result->wasSuccessful();
@@ -397,7 +405,7 @@ public function isLandoConfigPresent() {
    *   The version of Composer.
    */
   public function getComposerVersion() {
-    $version = $this->executor->execute("composer --version")
+    $version = $this->executor->execute(["composer", "--version"])
       ->interactive(FALSE)
       ->silent(TRUE)
       ->run()
@@ -605,12 +613,16 @@ protected function warnIfXdebugLoaded() {
    *   TRUE if config is identical.
    */
   public function isActiveConfigIdentical() {
-    $uri = $this->getConfigValue('drush.uri');
-    $result = $this->executor->drush("config:status --uri=$uri 2>&1")->run();
+    $result = $this->executor->drush(["config:status"])->run();
     $message = trim($result->getMessage());
-    $identical = strstr($message, 'No differences between DB and sync directory') !== FALSE;
 
-    return $identical;
+    // A successful test here results in "no message" so check for null.
+    if ($message == NULL) {
+      return TRUE;
+    }
+    else {
+      return FALSE;
+    }
   }
 
 }
diff --git a/tests/phpunit/fixtures/.node-version b/tests/phpunit/fixtures/.node-version
diff --git a/tests/phpunit/src/BltProjectTestBase.php b/tests/phpunit/src/BltProjectTestBase.php
@@ -3,6 +3,7 @@
 namespace Acquia\Blt\Tests;
 
 use Acquia\Blt\Robo\Blt;
+use Acquia\Blt\Robo\Common\StringManipulator;
 use Acquia\Blt\Robo\Config\ConfigInitializer;
 use PHPUnit\Framework\TestCase;
 use Robo\Robo;
@@ -39,6 +40,11 @@ abstract class BltProjectTestBase extends TestCase {
    */
   protected $dbDump;
 
+  /**
+   * @var \Acquia\Blt\Robo\Common\Executor
+   */
+  protected $executor;
+
   /**
    * @var \Symfony\Component\Console\Output\ConsoleOutput
    */
@@ -64,7 +70,7 @@ public function setUp(): void {
     $this->printTestName();
     $this->bltDirectory = realpath(dirname(__FILE__) . '/../../../');
     $this->fs = new Filesystem();
-    $this->execute('./bin/orca fixture:reset -f', getenv('ORCA_ROOT'));
+    $this->execute(['./bin/orca', 'fixture:reset', '-f'], getenv('ORCA_ROOT'));
     $this->sandboxInstance = getenv('ORCA_FIXTURE_DIR');
 
     $this->fs->copy($this->bltDirectory . "/scripts/blt/ci/internal/ci.yml", $this->sandboxInstance . "/blt/ci.blt.yml", TRUE);
@@ -90,7 +96,8 @@ protected function reInitializeConfig($input) {
 
   /**
    * @param mixed $command
-   *   Command.
+   *   The command string|array.
+   *   Warning: symfony/process 5.x expects an array.
    * @param mixed $cwd
    *   CWD.
    * @param bool $stop_on_error
@@ -102,6 +109,10 @@ protected function reInitializeConfig($input) {
    * @throws \Exception
    */
   protected function execute($command, $cwd = NULL, $stop_on_error = TRUE) {
+    // Backwards compatibility check for legacy commands.
+    if (!is_array($command)) {
+      $command = StringManipulator::commandConvert($command);
+    }
     if (!$cwd) {
       $cwd = $this->sandboxInstance;
     }
@@ -110,8 +121,9 @@ protected function execute($command, $cwd = NULL, $stop_on_error = TRUE) {
     $process->setTimeout(NULL);
     $output = new ConsoleOutput();
     if (getenv('BLT_PRINT_COMMAND_OUTPUT')) {
+      $string = implode(" ", $command);
       $output->writeln("");
-      $output->writeln("Executing <comment>$command</comment>...");
+      $output->writeln("Executing <comment>$string</comment>...");
       if (!$stop_on_error) {
         $output->writeln("Command failure is permitted.");
       }
@@ -138,8 +150,9 @@ protected function execute($command, $cwd = NULL, $stop_on_error = TRUE) {
   /**
    * Drush.
    *
-   * @param string $command
-   *   Command.
+   * @param mixed $command
+   *   The command string|array.
+   *   Warning: symfony/process 5.x expects an array.
    * @param mixed $root
    *   Root.
    * @param bool $stop_on_error
@@ -151,12 +164,20 @@ protected function execute($command, $cwd = NULL, $stop_on_error = TRUE) {
    * @throws \Exception
    */
   protected function drush($command, $root = NULL, $stop_on_error = TRUE) {
+    // Backwards compatibility check for legacy commands.
+    if (!is_array($command)) {
+      $command = StringManipulator::commandConvert($command);
+    }
     if (!$root) {
       $root = $this->config->get('docroot');
     }
-    $drush_bin = $this->sandboxInstance . '/vendor/bin/drush';
-    $command_string = "$drush_bin $command --root=$root --no-interaction --ansi";
-    $process = $this->execute($command_string, $root, $stop_on_error);
+    $command_array[] = $this->sandboxInstance . '/vendor/bin/drush';
+    $drush_array = array_merge($command_array, $command);
+    $drush_array[] = "--root=$root";
+    $drush_array[] = "--no-interaction";
+    $drush_array[] = "--ansi";
+
+    $process = $this->execute($drush_array, $root, $stop_on_error);
     $output = $process->getOutput();
 
     return $output;
@@ -165,8 +186,9 @@ protected function drush($command, $root = NULL, $stop_on_error = TRUE) {
   /**
    * Drush JSON.
    *
-   * @param string $command
-   *   Command.
+   * @param mixed $command
+   *   The command string|array.
+   *   Warning: symfony/process 5.x expects an array.
    * @param mixed $root
    *   Root.
    * @param bool $stop_on_error
@@ -178,7 +200,12 @@ protected function drush($command, $root = NULL, $stop_on_error = TRUE) {
    * @throws \Exception
    */
   protected function drushJson($command, $root = NULL, $stop_on_error = TRUE) {
-    $output = $this->drush($command . " --format=json", $root, $stop_on_error);
+    // Backwards compatibility check for legacy commands.
+    if (!is_array($command)) {
+      $command = StringManipulator::commandConvert($command);
+    }
+    $command[] = "--format=json";
+    $output = $this->drush($command, $root, $stop_on_error);
     $array = json_decode($output, TRUE);
 
     return $array;

diff --git a/tests/phpunit/src/CommandArrayTest.php b/tests/phpunit/src/CommandArrayTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Acquia\Blt\Tests;
+
+use Acquia\Blt\Robo\Common\StringManipulator;
+
+/**
+ * Tests String to Array Conversion for Commands.
+ */
+class CommandArrayTest extends BltProjectTestBase {
+
+  public function testStringToArray() {
+    $string = "site-install minimal --existing-config --ansi -n";
+    $command = StringManipulator::commandConvert($string);
+    $this->assertIsArray($command);
+
+    $keys = $this->getKeys();
+    foreach ($keys as $key) {
+      $result = in_array($key, $command);
+      $this->assertTrue($result);
+    }
+  }
+
+  public function testStringDrush() {
+    $string = "status --yes";
+    $this->drushJson($string);
+  }
+
+  public function getKeys() {
+    return [
+      "site-install",
+      "minimal",
+      "--existing-config",
+      "--ansi",
+      "-n",
+    ];
+  }
+
+}