Contrib.social

Explain why ComposerPluginsValidatorTest::testAddDisallowedPlugin() exists, even if it is not directly testing ComposerPluginsValidator

Open on Drupal.org →
Created on 26 April 2023, about 1 year ago
Updated 3 May 2023, about 1 year ago

Problem/Motivation

In the course of #3354594-8: Merge ComposerSettingsValidator into ComposerValidator → , I discovered that ComposerPluginsValidatorTest::testAddDisallowedPlugin() is only passing by coincidence. Quoting from that comment:

Here's the thing: we assert that we get an exception that contains a particular string. So far, so good. But are we sure we know which validator, exactly, is causing that exception to be thrown? 😈 We're assuming it's ComposerPluginsValidator, but during my trace, it was actually coming from EnabledExtensionsValidator.

That's because the exception will be raised by the first validator that happens to do a ComposerInspector operation on the stage directory -- which might not be ComposerPluginsValidator at all! So that particular test has never actually tested ComposerPluginsValidator. It might not actually be needed at all, ultimately, since it's really just testing that...we call ComposerInspector::validate() on the stage directory. Not all that useful.

Proposed resolution

We decided that we should keep the test, to ensure that we're keeping Composer honest.

Composer has been known to change its behavior in patch releases, on occasion, and we want to be absolutely sure we're always noticing if there is a plugin installed that config doesn't allow. Right now, Composer notices that for us, and dies with an exception. But what if that changes? We need to be sure we're always detecting this situation as an error condition, since plugins pose a security risk. So the test is actually useful, even if it's not directly testing ComposerPluginsValidator itself.

All we need to do is add a comment explaining the rationale.

🐛 Bug report
Status

Fixed

Version

3.0

Component

Documentation

Created by

🇺🇸United States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024