Issue #3325869 by Wim Leers, omkar.podey: Improve documentation for...

Issue #3325869 by Wim Leers, omkar.podey: Improve documentation for package_manager_bypass test module

automatic_updates_extensions/tests/src/Functional/UpdateErrorTest.php

@@ -7,7 +7,7 @@ namespace Drupal\Tests\automatic_updates_extensions\Functional;
 use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
 use Drupal\package_manager\Event\StatusCheckEvent;
 use Drupal\package_manager\ValidationResult;
-use Drupal\package_manager_bypass\Committer;
+use Drupal\package_manager_bypass\LoggingCommitter;
 use Drupal\package_manager_test_validation\EventSubscriber\TestSubscriber;
 
 /**
@@ -38,7 +38,7 @@ class UpdateErrorTest extends UpdaterFormTestBase {
     $this->checkForMetaRefresh();
     $this->assertUpdateStagedTimes(1);
     $assert_session->pageTextNotContains('The following dependencies will also be updated:');
-    Committer::setException(new \Exception('failed at committer'));
+    LoggingCommitter::setException(new \Exception('failed at committer'));
     $page->pressButton('Continue');
     $this->checkForMetaRefresh();
     $assert_session->pageTextContainsOnce('An error has occurred.');

package_manager/tests/modules/package_manager_bypass/package_manager_bypass.info.yml

 name: 'Package Manager Bypass'
-description: 'Mocks Package Manager services for functional testing.'
+description: 'Mocks PathLocator service, decorates Beginner & Committer services (adds logging) and by default bypasses the Stager service (to minimize I/O during tests).'
 type: module
 package: Testing
 dependencies:

package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml

 services:
   package_manager_bypass.beginner:
-    class: Drupal\package_manager_bypass\Beginner
+    class: Drupal\package_manager_bypass\LoggingBeginner
     arguments: ['@state', '@package_manager_bypass.beginner.inner' ]
     decorates: 'package_manager.beginner'
   package_manager_bypass.committer:
-    class: Drupal\package_manager_bypass\Committer
+    class: Drupal\package_manager_bypass\LoggingCommitter
     arguments: [ '@state', '@package_manager_bypass.committer.inner' ]
     decorates: 'package_manager.committer'

package_manager/tests/modules/package_manager_bypass/src/Beginner.php → package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php

@@ -12,9 +12,13 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface;
 
 /**
- * Defines a service that decorates the Composer Stager beginner service.
+ * A composer-stager Beginner decorator that adds logging.
+ *
+ * @internal
  */
-class Beginner extends BypassedStagerServiceBase implements BeginnerInterface {
+final class LoggingBeginner implements BeginnerInterface {
+
+  use LoggingDecoratorTrait;
 
   /**
    * The decorated service.

package_manager/tests/modules/package_manager_bypass/src/Committer.php → package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php

@@ -12,9 +12,13 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface;
 
 /**
- * Defines a service that decorates the Composer Stager committer service.
+ * A composer-stager Committer decorator that adds logging.
+ *
+ * @internal
  */
-class Committer extends BypassedStagerServiceBase implements CommitterInterface {
+final class LoggingCommitter implements CommitterInterface {
+
+  use LoggingDecoratorTrait;
 
   /**
    * The decorated service.

package_manager/tests/modules/package_manager_bypass/src/BypassedStagerServiceBase.php → package_manager/tests/modules/package_manager_bypass/src/LoggingDecoratorTrait.php

@@ -10,8 +10,10 @@ namespace Drupal\package_manager_bypass;
  * This can be used by functional tests to ensure that the bypassed Composer
  * Stager services were called as expected. Kernel and unit tests should use
  * regular mocks instead.
+ *
+ * @internal
  */
-abstract class BypassedStagerServiceBase {
+trait LoggingDecoratorTrait {
 
   /**
    * The state service.

package_manager/tests/modules/package_manager_bypass/src/PathLocator.php → package_manager/tests/modules/package_manager_bypass/src/MockPathLocator.php

@@ -9,9 +9,11 @@ use Drupal\package_manager\PathLocator as BasePathLocator;
 use Drupal\package_manager\Path;
 
 /**
- * Overrides the path locator to return pre-set values for testing purposes.
+ * Mock path locator: allows specifying paths instead of discovering paths.
+ *
+ * @internal
  */
-class PathLocator extends BasePathLocator {
+final class MockPathLocator extends BasePathLocator {
 
   /**
    * The state service.

package_manager/tests/modules/package_manager_bypass/src/Stager.php → package_manager/tests/modules/package_manager_bypass/src/NoOpStager.php

@@ -12,9 +12,22 @@ use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ProcessRunnerInterface;
 use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 
 /**
- * Defines an update stager which doesn't actually do anything.
+ * A composer-stager Stager implementation that does nothing, except logging.
+ *
+ * By default, it will modify composer.lock in the stage directory, to fool the
+ * \Drupal\package_manager\Validator\LockFileValidator into thinking that there
+ * are pending composer operations.
+ *
+ * Opt out of this by calling @code setLockFileShouldChange(FALSE) @endcode.
+ *
+ * @see ::setLockFileShouldChange()
+ * @see \Drupal\package_manager\Validator\LockFileValidator
+ *
+ * @internal
  */
-class Stager extends BypassedStagerServiceBase implements StagerInterface {
+final class NoOpStager implements StagerInterface {
+
+  use LoggingDecoratorTrait;
 
   /**
    * Constructs a Stager object.

package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php

@@ -11,8 +11,10 @@ use Symfony\Component\DependencyInjection\Reference;
 
 /**
  * Defines services to bypass Package Manager's core functionality.
+ *
+ * @internal
  */
-class PackageManagerBypassServiceProvider extends ServiceProviderBase {
+final class PackageManagerBypassServiceProvider extends ServiceProviderBase {
 
   /**
    * {@inheritdoc}
@@ -21,12 +23,13 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase {
     parent::alter($container);
 
     $state = new Reference('state');
+    // By default, \Drupal\package_manager_bypass\Stager
     if (Settings::get('package_manager_bypass_composer_stager', TRUE)) {
-      $container->getDefinition('package_manager.stager')->setClass(Stager::class)->setArguments([$state]);
+      $container->getDefinition('package_manager.stager')->setClass(NoOpStager::class)->setArguments([$state]);
     }
 
     $definition = $container->getDefinition('package_manager.path_locator')
-      ->setClass(PathLocator::class);
+      ->setClass(MockPathLocator::class);
     $arguments = $definition->getArguments();
     array_unshift($arguments, $state);
     $definition->setArguments($arguments);

package_manager/tests/src/Kernel/LockFileValidatorTest.php

@@ -9,7 +9,7 @@ use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Event\PreRequireEvent;
 use Drupal\package_manager\Validator\LockFileValidator;
 use Drupal\package_manager\ValidationResult;
-use Drupal\package_manager_bypass\Stager;
+use Drupal\package_manager_bypass\NoOpStager;
 
 /**
  * @coversDefaultClass \Drupal\package_manager\Validator\LockFileValidator
@@ -137,7 +137,7 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
    */
   public function testApplyWithNoChange(): void {
     // Leave the staged lock file alone.
-    Stager::setLockFileShouldChange(FALSE);
+    NoOpStager::setLockFileShouldChange(FALSE);
 
     $result = ValidationResult::createError([
       t('There are no pending Composer operations.'),

package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php

@@ -267,7 +267,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
 
     // Ensure the path locator points to the test project. We assume that is its
     // own web root and the vendor directory is at its top level.
-    /** @var \Drupal\package_manager_bypass\PathLocator $path_locator */
+    /** @var \Drupal\package_manager_bypass\MockPathLocator $path_locator */
     $path_locator = $this->container->get('package_manager.path_locator');
     $path_locator->setPaths($active_dir, $active_dir . '/vendor', '', $staging_root);
 

package_manager/tests/src/Kernel/StageTest.php

@@ -18,7 +18,7 @@ use Drupal\package_manager\Event\StageEvent;
 use Drupal\package_manager\Exception\ApplyFailedException;
 use Drupal\package_manager\Exception\StageException;
 use Drupal\package_manager\Stage;
-use Drupal\package_manager_bypass\Committer;
+use Drupal\package_manager_bypass\LoggingCommitter;
 use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
 use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
 use PhpTuf\ComposerStager\Domain\Service\Precondition\PreconditionInterface;
@@ -63,7 +63,7 @@ class StageTest extends PackageManagerKernelTestBase {
     $validator = $this->container->get('package_manager.validator.file_system');
     $this->container->get('event_dispatcher')->removeSubscriber($validator);
 
-    /** @var \Drupal\package_manager_bypass\PathLocator $path_locator */
+    /** @var \Drupal\package_manager_bypass\MockPathLocator $path_locator */
     $path_locator = $this->container->get('package_manager.path_locator');
 
     $stage = $this->createStage();
@@ -188,7 +188,7 @@ class StageTest extends PackageManagerKernelTestBase {
       // testing purposes.
       $event->getStage()->destroy($force);
       // @see \PhpTuf\ComposerStager\Infrastructure\Service\Precondition\StagingDirDoesNotExist
-      Committer::setException(
+      LoggingCommitter::setException(
         new PreconditionException(
           $this->prophesize(PreconditionInterface::class)->reveal(),
           'Stage directory does not exist',
@@ -327,7 +327,7 @@ class StageTest extends PackageManagerKernelTestBase {
     else {
       $throwable = new $thrown_class($thrown_message, 123);
     }
-    Committer::setException($throwable);
+    LoggingCommitter::setException($throwable);
 
     try {
       $stage->apply();
@@ -360,7 +360,7 @@ class StageTest extends PackageManagerKernelTestBase {
     // Make the committer throw an exception, which should cause the failure
     // marker to be present.
     $thrown = new \Exception('Disastrous catastrophe!');
-    Committer::setException($thrown);
+    LoggingCommitter::setException($thrown);
     try {
       $stage->apply();
       $this->fail('Expected an exception.');

tests/src/Functional/UpdateFailedTest.php

@@ -4,7 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\Tests\automatic_updates\Functional;
 
-use Drupal\package_manager_bypass\Committer;
+use Drupal\package_manager_bypass\LoggingCommitter;
 
 /**
  * @covers \Drupal\automatic_updates\Form\UpdaterForm
@@ -31,7 +31,7 @@ class UpdateFailedTest extends UpdaterFormTestBase {
     $this->checkForMetaRefresh();
     $this->assertUpdateStagedTimes(1);
 
-    Committer::setException(new \Exception('failed at committer'));
+    LoggingCommitter::setException(new \Exception('failed at committer'));
     $page->pressButton('Continue');
     $this->checkForMetaRefresh();
     $assert_session->pageTextContainsOnce('An error has occurred.');

tests/src/Kernel/CronUpdaterTest.php

@@ -20,7 +20,7 @@ use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Exception\StageOwnershipException;
 use Drupal\package_manager\Exception\StageValidationException;
 use Drupal\package_manager\ValidationResult;
-use Drupal\package_manager_bypass\Committer;
+use Drupal\package_manager_bypass\LoggingCommitter;
 use Drupal\Tests\automatic_updates\Traits\EmailNotificationsTestTrait;
 use Drupal\Tests\package_manager\Kernel\TestStage;
 use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
@@ -577,7 +577,7 @@ END;
   public function testApplyFailureEmail(): void {
     $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1');
     $error = new \Exception('I drink your milkshake!');
-    Committer::setException($error);
+    LoggingCommitter::setException($error);
 
     $this->container->get('cron')->run();
     $expected_body = <<<END

tests/src/Kernel/UpdaterTest.php

@@ -11,7 +11,7 @@ use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Event\PreRequireEvent;
 use Drupal\package_manager\Exception\StageException;
 use Drupal\package_manager\ValidationResult;
-use Drupal\package_manager_bypass\Committer;
+use Drupal\package_manager_bypass\LoggingCommitter;
 use Drupal\Tests\package_manager\Kernel\TestStageValidationException;
 use Drupal\Tests\user\Traits\UserCreationTrait;
 use PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException;
@@ -188,7 +188,7 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
     ]);
     $updater->stage();
     $thrown_message = 'A very bad thing happened';
-    Committer::setException(new $thrown_class($thrown_message, 123));
+    LoggingCommitter::setException(new $thrown_class($thrown_message, 123));
     $this->expectException($expected_class);
     $expected_message = $expected_class === UpdateException::class ?
       "The update operation failed to apply completely. All the files necessary to run Drupal correctly and securely are probably not present. It is strongly recommended to restore your site's code and database from a backup."