Issue #3346520 by Wim Leers, phenaproxima: Explicitly validate lock files...

Issue #3346520 by Wim Leers, phenaproxima: Explicitly validate lock files everywhere: `composer validate --check-lock`

automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php

@@ -40,7 +40,8 @@ class ModuleUpdateTest extends UpdateTestBase {
 \$config['update_test.settings']['system_info'] = $system_info;
 END;
     $this->writeSettings($code);
-    $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../../../package_manager/tests/fixtures/build_test_projects/alpha/1.0.0'));
+    $alpha_repo_path = $this->copyFixtureToTempDirectory(__DIR__ . '/../../../../package_manager/tests/fixtures/build_test_projects/alpha/1.0.0');
+    $this->addRepository('alpha', $alpha_repo_path);
     $this->runComposer('COMPOSER_MIRROR_PATH_REPOS=1 composer require drupal/alpha --update-with-all-dependencies', 'project');
     $this->assertModuleVersion('alpha', '1.0.0');
     $fs = new SymfonyFilesystem();
@@ -51,8 +52,8 @@ END;
       'new_module',
     ]);
 
-    // Change both modules' upstream version.
-    $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../../../package_manager/tests/fixtures/build_test_projects/alpha/1.1.0'));
+    // Change the module's upstream version.
+    static::copyFixtureFilesTo(__DIR__ . '/../../../../package_manager/tests/fixtures/build_test_projects/alpha/1.1.0', $alpha_repo_path);
   }
 
   /**

package_manager/src/ComposerInspector.php

@@ -108,8 +108,7 @@ class ComposerInspector {
       try {
         $this->runner->run([
           'validate',
-          // @todo Check the lock file in https://drupal.org/i/3343827.
-          '--no-check-lock',
+          '--check-lock',
           '--no-check-publish',
           '--with-dependencies',
           '--no-interaction',

package_manager/src/Validator/ComposerSettingsValidator.php

@@ -40,7 +40,9 @@ final class ComposerSettingsValidator implements EventSubscriberInterface {
    * Validates Composer settings.
    */
   public function validate(PreOperationStageEvent $event): void {
-    $dir = $this->pathLocator->getProjectRoot();
+    $dir = $event instanceof PreApplyEvent
+      ? $event->stage->getStageDirectory()
+      : $this->pathLocator->getProjectRoot();
 
     try {
       $setting = (int) $this->inspector->getConfig('secure-http', $dir);

package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php

@@ -59,6 +59,7 @@ class FixtureManipulator {
     $runner = \Drupal::service(ComposerRunnerInterface::class);
     $runner->run([
       'validate',
+      '--check-lock',
       '--no-check-publish',
       '--with-dependencies',
       '--no-interaction',
@@ -68,8 +69,6 @@ class FixtureManipulator {
       // Unlike ComposerInspector::validate(), explicitly do NOT validate
       // plugins, to allow for testing edge cases.
       '--no-plugins',
-      // @todo remove this after FixtureManipulator uses composer commands exclusively!
-      '--no-check-lock',
       // Dummy packages are not meant for publishing, so do not validate that.
       '--no-check-publish',
       '--no-check-version',
@@ -634,6 +633,7 @@ class FixtureManipulator {
     // repos at the absolute path.
     $composer_json = file_get_contents($this->dir . '/composer.json');
     file_put_contents($this->dir . '/composer.json', str_replace('../path_repos/', "$path_repo_base/", $composer_json));
+    $this->runComposerCommand(['update', '--lock']);
     $this->runComposerCommand(['install']);
   }
 

package_manager/tests/src/Build/PackageInstallTest.php

@@ -22,6 +22,9 @@ class PackageInstallTest extends TemplateProjectTestBase {
       'alpha' => __DIR__ . '/../../fixtures/release-history/alpha.1.1.0.xml',
     ]);
     $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/build_test_projects/alpha/1.0.0'));
+    // Repository definitions affect the lock file hash, so update the hash to
+    // ensure that Composer won't complain that the lock file is out of sync.
+    $this->runComposer('composer update --lock', 'project');
 
     // Use the API endpoint to create a stage and install alpha 1.0.0. We ask
     // the API to return the contents of composer.json file of installed module,

package_manager/tests/src/Build/PackageUpdateTest.php

@@ -20,8 +20,12 @@ class PackageUpdateTest extends TemplateProjectTestBase {
   public function testPackageUpdate(): void {
     $this->createTestProject('RecommendedProject');
 
-    $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/build_test_projects/alpha/1.0.0'));
-    $this->addRepository('updated_module', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/build_test_projects/updated_module/1.0.0'));
+    $fixtures = __DIR__ . '/../../fixtures/build_test_projects';
+
+    $alpha_repo_path = $this->copyFixtureToTempDirectory("$fixtures/alpha/1.0.0");
+    $this->addRepository('alpha', $alpha_repo_path);
+    $updated_module_repo_path = $this->copyFixtureToTempDirectory("$fixtures/updated_module/1.0.0");
+    $this->addRepository('updated_module', $updated_module_repo_path);
     $this->setReleaseMetadata([
       'updated_module' => __DIR__ . '/../../fixtures/release-history/updated_module.1.1.0.xml',
     ]);
@@ -32,8 +36,8 @@ class PackageUpdateTest extends TemplateProjectTestBase {
     $this->installModules(['updated_module']);
 
     // Change both modules' upstream version.
-    $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/build_test_projects/alpha/1.1.0'));
-    $this->addRepository('updated_module', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/build_test_projects/updated_module/1.1.0'));
+    static::copyFixtureFilesTo("$fixtures/alpha/1.1.0", $alpha_repo_path);
+    static::copyFixtureFilesTo("$fixtures/updated_module/1.1.0", $updated_module_repo_path);
     // Make .git folder
 
     // Use the API endpoint to create a stage and update updated_module to

package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php

@@ -9,7 +9,6 @@ use Drupal\fixture_manipulator\ActiveFixtureManipulator;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Exception\StageEventException;
 use Drupal\package_manager\ValidationResult;
-use Symfony\Component\Process\Process;
 
 /**
  * @covers \Drupal\package_manager\Validator\ComposerPatchesValidator
@@ -75,25 +74,27 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase {
    *  @dataProvider providerErrorDuringPreCreate()
    */
   public function testErrorDuringPreCreate(int $options, array $expected_results): void {
+    $active_manipulator = new ActiveFixtureManipulator();
     if ($options & static::CONFIG_ALLOWED_PLUGIN) {
-      $this->addPatcherToAllowedPlugins();
+      $active_manipulator->addConfig(['allow-plugins.cweagans/composer-patches' => TRUE]);
     }
     if ($options & static::EXTRA_EXIT_ON_PATCH_FAILURE) {
-      $this->setRootExtra();
+      $active_manipulator->addConfig(['extra.composer-exit-on-patch-failure' => TRUE]);
     }
     if ($options & static::REQUIRE_PACKAGE_FROM_ROOT) {
-      $this->setRootRequires();
+      $active_manipulator->requirePackage('cweagans/composer-patches', '@dev');
     }
     elseif ($options & static::REQUIRE_PACKAGE_INDIRECTLY) {
-      (new ActiveFixtureManipulator())
-        ->addPackage([
-          'type' => 'package',
-          'name' => 'dummy/depends-on-composer-patches',
-          'description' => 'A dummy package depending on cweagans/composer-patches',
-          'version' => '1.0.0',
-          'require' => ['cweagans/composer-patches' => '*'],
-        ])
-        ->commitChanges();
+      $active_manipulator->addPackage([
+        'type' => 'package',
+        'name' => 'dummy/depends-on-composer-patches',
+        'description' => 'A dummy package depending on cweagans/composer-patches',
+        'version' => '1.0.0',
+        'require' => ['cweagans/composer-patches' => '*'],
+      ]);
+    }
+    if ($options !== static::ABSENT) {
+      $active_manipulator->commitChanges();
     }
     $this->assertStatusCheckResults($expected_results);
     $this->assertResults($expected_results, PreCreateEvent::class);
@@ -191,14 +192,18 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase {
    */
   public function testErrorDuringPreApply(int $in_active, int $in_stage, array $expected_results): void {
     // Simulate in active.
+    $active_manipulator = new ActiveFixtureManipulator();
     if ($in_active & static::CONFIG_ALLOWED_PLUGIN) {
-      $this->addPatcherToAllowedPlugins();
+      $active_manipulator->addConfig(['allow-plugins.cweagans/composer-patches' => TRUE]);
     }
     if ($in_active & static::EXTRA_EXIT_ON_PATCH_FAILURE) {
-      $this->setRootExtra();
+      $active_manipulator->addConfig(['extra.composer-exit-on-patch-failure' => TRUE]);
     }
     if ($in_active & static::REQUIRE_PACKAGE_FROM_ROOT) {
-      $this->setRootRequires();
+      $active_manipulator->requirePackage('cweagans/composer-patches', '@dev');
+    }
+    if ($in_active !== static::ABSENT) {
+      $active_manipulator->commitChanges();
     }
 
     // Simulate in stage.
@@ -284,35 +289,4 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase {
     $this->testErrorDuringPreApply($in_active, $in_stage, $expected_results);
   }
 
-  /**
-   * Add the installed patcher to allowed plugins.
-   */
-  private function addPatcherToAllowedPlugins(): void {
-    (new ActiveFixtureManipulator())
-      ->addConfig(['allow-plugins.cweagans/composer-patches' => TRUE])
-      ->commitChanges();
-  }
-
-  /**
-   * Sets the cweagans/composer-patches as required package for root package.
-   */
-  private function setRootRequires(): void {
-    $process = new Process(
-      ['composer', 'require', "cweagans/composer-patches:@dev"],
-      $this->container->get('package_manager.path_locator')->getProjectRoot()
-    );
-    $process->mustRun();
-  }
-
-  /**
-   * Sets the composer-exit-on-patch-failure key in extra part of root package.
-   */
-  private function setRootExtra(): void {
-    $process = new Process(
-      ['composer', 'config', 'extra.composer-exit-on-patch-failure', 'true'],
-      $this->container->get('package_manager.path_locator')->getProjectRoot()
-    );
-    $process->mustRun();
-  }
-
 }

package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php

@@ -4,7 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\Tests\package_manager\Kernel;
 
-use Drupal\Component\Serialization\Json;
+use Drupal\fixture_manipulator\ActiveFixtureManipulator;
 use Drupal\package_manager\Event\PreApplyEvent;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\ValidationResult;
@@ -29,23 +29,21 @@ class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase {
 
     return [
       'disabled' => [
-        Json::encode([
-          'config' => [
-            'secure-http' => FALSE,
-          ],
-        ]),
+        [
+          'secure-http' => FALSE,
+        ],
         [$error],
       ],
       'explicitly enabled' => [
-        Json::encode([
-          'config' => [
-            'secure-http' => TRUE,
-          ],
-        ]),
+        [
+          'secure-http' => TRUE,
+        ],
         [],
       ],
       'implicitly enabled' => [
-        '{}',
+        [
+          'extra.unrelated' => TRUE,
+        ],
         [],
       ],
     ];
@@ -54,17 +52,15 @@ class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase {
   /**
    * Tests that Composer's secure-http setting is validated.
    *
-   * @param string $contents
-   *   The contents of the composer.json file.
+   * @param array $config
+   *   The config to set.
    * @param \Drupal\package_manager\ValidationResult[] $expected_results
    *   The expected validation results, if any.
    *
    * @dataProvider providerSecureHttpValidation
    */
-  public function testSecureHttpValidation(string $contents, array $expected_results): void {
-    $active_dir = $this->container->get('package_manager.path_locator')
-      ->getProjectRoot();
-    file_put_contents("$active_dir/composer.json", $contents);
+  public function testSecureHttpValidation(array $config, array $expected_results): void {
+    (new ActiveFixtureManipulator())->addConfig($config)->commitChanges();
     $this->assertStatusCheckResults($expected_results);
     $this->assertResults($expected_results, PreCreateEvent::class);
   }
@@ -72,19 +68,15 @@ class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase {
   /**
    * Tests that Composer's secure-http setting is validated during pre-apply.
    *
-   * @param string $contents
-   *   The contents of the composer.json file.
+   * @param array $config
+   *   The config to set.
    * @param \Drupal\package_manager\ValidationResult[] $expected_results
    *   The expected validation results, if any.
    *
    * @dataProvider providerSecureHttpValidation
    */
-  public function testSecureHttpValidationDuringPreApply(string $contents, array $expected_results): void {
-    $this->addEventTestListener(function () use ($contents): void {
-      $active_dir = $this->container->get('package_manager.path_locator')
-        ->getProjectRoot();
-      file_put_contents("$active_dir/composer.json", $contents);
-    });
+  public function testSecureHttpValidationDuringPreApply(array $config, array $expected_results): void {
+    $this->getStageFixtureManipulator()->addConfig($config);
     $this->assertResults($expected_results, PreApplyEvent::class);
   }
 

package_manager/tests/src/Kernel/FixtureManipulatorTest.php

@@ -44,7 +44,16 @@ class FixtureManipulatorTest extends PackageManagerKernelTestBase {
   private function assertOriginalFixturePackagesUnchanged(array $installed_php): void {
     $original_package_names = array_keys($this->originalInstalledPhp);
     $installed_php_core_packages = array_intersect_key($installed_php, array_flip($original_package_names));
-    $this->assertSame($this->originalInstalledPhp, $installed_php_core_packages);
+    // `reference` is never the same as the original because the relative path
+    // repos from the `fake_site` fixture are converted to absolute ones, which
+    // causes a new reference to be computed.
+    $without_reference_key = function (array $a): array {
+      return array_diff_key($a, array_flip(['reference']));
+    };
+    $this->assertSame(
+      array_map($without_reference_key, $this->originalInstalledPhp),
+      array_map($without_reference_key, $installed_php_core_packages)
+    );
   }
 
   /**