Problem/Motivation

ComposerProjectTemplatesTest::testMinimumStabilityStrictness() tests that all the Composer dependencies of Drupal meet a minimum stability that's specified in the test.

However, the way this test is written, it doesn't check each dependency's stability against the required minimum and complain if one is not sufficient. Instead, it goes through all the dependencies, finds the lowest stability, and then that result is compared against the minimum.

This approach means that if the test fails, you just get this output:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'dev'
+'stable'

/Users/joachim/Sites/drupal-core-composer/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/Users/joachim/Sites/drupal-core-composer/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:94
/Users/joachim/Sites/drupal-core-composer/repos/drupal/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php:101

This is not useful, because it doesn't tell you which package caused the problem!

Steps to reproduce

Proposed resolution

Rewrite the test, so each package has its stability compared with the minimum.

(Ideally, we'd see ALL the failing packages in the test failure output, but I can't think how to do that.)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3225706-3_5.txt688 bytesgauravvvv
#5 3225706-5.patch3.64 KBgauravvvv

Issue fork drupal-3225706

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

joachim created an issue. See original summary.

joachim’s picture

So we *could* make an array of all the projects's stabilities, keyed by projects, and then do:

        $this->assertNotContains($stability, $project_stabilities);

which would fail if for example the array contains 'foo/bar' => 'dev'.

But PHPUnit doesn't show you the contents of the array in that circumstance: https://github.com/sebastianbergmann/phpunit/issues/3061

joachim’s picture

Status: Active » Needs review
gauravvvv’s picture

StatusFileSize
new3.64 KB
new688 bytes

Re-rolled patch #3, Attached interdiff for same.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joachim’s picture

Status: Needs review » Reviewed & tested by the community

LGTM. Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3225706-5.patch, failed testing. View results

yogeshmpawar’s picture

Status: Needs work » Reviewed & tested by the community

Moving to RTBC as test failures are unrelated.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3225706-5.patch, failed testing. View results

gauravvvv’s picture

Status: Needs work » Reviewed & tested by the community

Restoring status as test failures are unrelated.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This is a good idea. I checked that contrib is not using the removed method - https://git.drupalcode.org/search?group_id=2&repository_ref=&scope=blobs...

However given we are removing a method only backported to 9.4.x.

Committed 8957175 and pushed to 10.0.x. Thanks!
Committed 7afe7f5 and pushed to 9.4.x. Thanks!

  • alexpott committed 8957175 on 10.0.x
    Issue #3225706 by joachim, Gauravmahlawat: rewrite...

  • alexpott committed 7afe7f5 on 9.4.x
    Issue #3225706 by joachim, Gauravmahlawat: rewrite...

Status: Fixed » Closed (fixed)

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