Issue #3317988 by phenaproxima: Make ComposerExecutableValidatorTest easier to understand

package_manager/src/Validator/ComposerExecutableValidator.php

@@ -23,7 +23,7 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
  *   at any time without warning. External code should not interact with this
  *   class.
  */
-final class ComposerExecutableValidator implements EventSubscriberInterface, ProcessOutputCallbackInterface {
+class ComposerExecutableValidator implements EventSubscriberInterface {
 
   use StringTranslationTrait;
 
@@ -48,13 +48,6 @@ final class ComposerExecutableValidator implements EventSubscriberInterface, Pro
    */
   protected $moduleHandler;
 
-  /**
-   * The detected version of Composer.
-   *
-   * @var string
-   */
-  protected $version;
-
   /**
    * Constructs a ComposerExecutableValidator object.
    *
@@ -76,18 +69,24 @@ final class ComposerExecutableValidator implements EventSubscriberInterface, Pro
    */
   public function validateStagePreOperation(PreOperationStageEvent $event): void {
     try {
-      $this->composer->run(['--version'], $this);
+      $output = $this->runCommand();
     }
     catch (ExceptionInterface $e) {
       $this->setError($e->getMessage(), $event);
       return;
     }
 
-    if ($this->version) {
-      if (!Semver::satisfies($this->version, static::MINIMUM_COMPOSER_VERSION_CONSTRAINT)) {
+    $matched = [];
+    // Search for a semantic version number and optional stability flag.
+    if (preg_match('/([0-9]+\.?){3}-?((alpha|beta|rc)[0-9]*)?/i', $output, $matched)) {
+      $version = $matched[0];
+    }
+
+    if (isset($version)) {
+      if (!Semver::satisfies($version, static::MINIMUM_COMPOSER_VERSION_CONSTRAINT)) {
         $message = $this->t('A Composer version which satisfies <code>@minimum_version</code> is required, but version @detected_version was detected.', [
           '@minimum_version' => static::MINIMUM_COMPOSER_VERSION_CONSTRAINT,
-          '@detected_version' => $this->version,
+          '@detected_version' => $version,
         ]);
         $this->setError($message, $event);
       }
@@ -133,14 +132,35 @@ final class ComposerExecutableValidator implements EventSubscriberInterface, Pro
   }
 
   /**
-   * {@inheritdoc}
+   * Runs `composer --version` and returns its output.
+   *
+   * @return string
+   *   The output of `composer --version`.
    */
-  public function __invoke(string $type, string $buffer): void {
-    $matched = [];
-    // Search for a semantic version number and optional stability flag.
-    if (preg_match('/([0-9]+\.?){3}-?((alpha|beta|rc)[0-9]*)?/i', $buffer, $matched)) {
-      $this->version = $matched[0];
-    }
+  protected function runCommand(): string {
+    // For whatever reason, PHPCS thinks that $output is not used, even though
+    // it very clearly *is*. So, shut PHPCS up for the duration of this method.
+    // phpcs:disable
+    $callback = new class () implements ProcessOutputCallbackInterface {
+
+      /**
+       * The command output.
+       *
+       * @var string
+       */
+      public string $output = '';
+
+      /**
+       * {@inheritdoc}
+       */
+      public function __invoke(string $type, string $buffer): void {
+        $this->output .= $buffer;
+      }
+
+    };
+    $this->composer->run(['--version'], $callback);
+    return $callback->output;
+    // phpcs:enable
   }
 
 }

package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php

@@ -8,8 +8,7 @@ use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Validator\ComposerExecutableValidator;
 use Drupal\package_manager\ValidationResult;
 use PhpTuf\ComposerStager\Domain\Exception\IOException;
-use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface;
-use Prophecy\Argument;
+use PHPUnit\Framework\Assert;
 
 /**
  * @covers \Drupal\package_manager\Validator\ComposerExecutableValidator
@@ -18,21 +17,6 @@ use Prophecy\Argument;
  */
 class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
 
-  /**
-   * The mocked Composer runner.
-   *
-   * @var \Prophecy\Prophecy\ObjectProphecy|\PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface
-   */
-  private $composerRunner;
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function setUp(): void {
-    $this->composerRunner = $this->prophesize(ComposerRunnerInterface::class);
-    parent::setUp();
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -40,7 +24,7 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
     parent::register($container);
 
     $container->getDefinition('package_manager.validator.composer_executable')
-      ->setArgument('$composer', $this->composerRunner->reveal());
+      ->setClass(TestComposerExecutableValidator::class);
   }
 
   /**
@@ -48,12 +32,7 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
    */
   public function testErrorIfComposerNotFound(): void {
     $exception = new IOException("This is your regularly scheduled error.");
-
-    // If the Composer executable isn't found, the executable finder will throw
-    // an exception, which will not be caught by the Composer runner.
-    $this->composerRunner->run(Argument::cetera())
-      ->willThrow($exception)
-      ->shouldBeCalled();
+    TestComposerExecutableValidator::setCommandOutput($exception);
 
     // The validator should translate that exception into an error.
     $error = ValidationResult::createError([
@@ -163,21 +142,7 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
    * @dataProvider providerComposerVersionValidation
    */
   public function testComposerVersionValidation(string $reported_version, array $expected_results): void {
-    // Mock the output of `composer --version`, will be passed to the validator,
-    // which is itself a callback function that gets called repeatedly as
-    // Composer produces output.
-    $this->composerRunner->run(['--version'], Argument::type(ComposerExecutableValidator::class))
-      // Whatever is passed to ::run() will be passed to this mock callback in
-      // $arguments, and we know exactly what that will contain: an array of
-      // command arguments for Composer, and the validator object.
-      ->will(function (array $arguments) use ($reported_version) {
-        /** @var \Drupal\package_manager\Validator\ComposerExecutableValidator $validator */
-        $validator = $arguments[1];
-        // Invoke the validator (which, as mentioned, is a callback function),
-        // with fake output from `composer --version`. It should try to tease a
-        // recognized, supported version number out of this output.
-        $validator($validator::OUT, "Composer version $reported_version");
-      });
+    TestComposerExecutableValidator::setCommandOutput("Composer version $reported_version");
 
     // If the validator can't find a recognized, supported version of Composer,
     // it should produce errors.
@@ -220,3 +185,32 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase {
   }
 
 }
+
+/**
+ * A test-only version of ComposerExecutableValidator that returns set output.
+ */
+class TestComposerExecutableValidator extends ComposerExecutableValidator {
+
+  /**
+   * Sets the output of `composer --version`.
+   *
+   * @param string|\Throwable $output
+   *   The output of the command, or an exception to throw.
+   */
+  public static function setCommandOutput($output): void {
+    \Drupal::state()->set(static::class, $output);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function runCommand(): string {
+    $output = \Drupal::state()->get(static::class);
+    Assert::assertNotNull($output, __CLASS__ . '::setCommandOutput() should have been called first 💩');
+    if ($output instanceof \Throwable) {
+      throw $output;
+    }
+    return $output;
+  }
+
+}