Regression in #3343827: GitExcluderTest should test with composer/installers enabled

This clearly should fail tests, but it passes… 😱

The test coverage proves the problem in HEAD …

… but in reviewing this I realized that the root cause is probably simply a regression caused by 📌 Update FixtureManipulator to work with InstalledPackagesList, real composer show command Fixed 😅

The fix in GitExcluder has proper test coverage, because uncommenting it causes the tests to fail:

Testing /Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder
F.                                                                  2 / 2 (100%)

Time: 00:11.157, Memory: 10.00 MB

There was 1 failure:

1) Drupal\Tests\package_manager\Kernel\PathExcluder\GitExcluderTest::testGitDirectoriesExcludedActive
Failed asserting that an array does not contain 'modules/package_known_to_composer_removed_later/.git'.

/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php:67
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

FAILURES!
Tests: 2, Assertions: 14, Failures: 1.
Process finished with exit code 1

👍

That leaves only one question on the MR to be addressed!

This now just needs $allow_plugins=TRUE for the final bit of clean-up 😊🙏

I found the problem. Or actually, two.

Pre-existing ComposerInstallersTrait bug

In the test, we only had this:

    "installer-paths": {
      "different_installer_path/package_known_to_composer": ["foo/package_with_different_installer_path_known_to_composer"]
    }

i.e.

      "core": ["type:drupal-core"],
      "libraries/{$name}": ["type:drupal-library"],
      "modules/contrib/{$name}": ["type:drupal-module"],
      "profiles/contrib/{$name}": ["type:drupal-profile"],
      "themes/contrib/{$name}": ["type:drupal-theme"],
      "drush/Commands/contrib/{$name}": ["type:drupal-drush"],
      "modules/custom/{$name}": ["type:drupal-custom-module"],
      "themes/custom/{$name}": ["type:drupal-custom-theme"]

was missing! 😱

Fixed in 10ee1d560d8c7eaa37b656dbad5203ae0004dadd.

Pre-existing \Drupal\fixture_manipulator\FixtureManipulator::addDotGitFolder() bug

The above also means that

->addDotGitFolder($project_root . "/modules/custom/custom_package_known_to_composer")

and

    $not_excluded_paths = [
…
      'modules/custom/custom_package_known_to_composer/.git',
…
    ];

in the MR are only passing tests because addDotGitFolder() is blindly creating a .git folder in a directory that doesn't even exist nor contains a composer.json ⇒ we need to add basic validation to addDotGitFolder(). Otherwise, @kunal.sachdev would have been alerted about this! 😢

Fixed in 0688af9937653905da1c19aaeea356bb9952fccb.

Next

Now that the test infrastructure makes sense again, I'm certain @kunal.sachdev can finish this one! 😊

🚀

Reviewing with an eye to commit.

Thanks for that very nice simplification right at the end! 😊👏

Automatically closed - issue fixed for 2 weeks with no activity.