Assert no errors after creating the test project in ModuleUpdateTest

Just like I extracted #3320792: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) from #3331168: Limit trusted Composer plugins to a known list, allow user to add more's https://git.drupalcode.org/project/automatic_updates/-/merge_requests/659, I believe I can do the same here.

(See #3320792-25: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) through #3320792-27: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers).)

First pushed a test. That should fail like this:

1) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testApi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'
+Drupal cannot be automatically updated from the installed version, 10.0.3-dev, because automatic updates from a dev version to any other version are not supported.'

As expected:

10.1

Testing Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest
FF                                                                  2 / 2 (100%)

Time: 02:26.304, Memory: 18.00 MB

There were 2 failures:

1) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testApi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'
+Drupal cannot be automatically updated from the installed version, 10.1.0-dev, because automatic updates from a dev version to any other version are not supported.'

2) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testUi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'
+Drupal cannot be automatically updated from the installed version, 10.1.0-dev, because automatic updates from a dev version to any other version are not supported.'

— https://www.drupal.org/pift-ci-job/2581810

9.5.2

Tests pass…

If I now extract/cherry-pick the solution from https://git.drupalcode.org/project/automatic_updates/-/merge_requests/65... (which finds its origin in https://git.drupalcode.org/project/automatic_updates/-/merge_requests/65...), we can see that … it does not pass tests.

Why did it work for #3331168: Limit trusted Composer plugins to a known list, allow user to add more? Probably because #3331168 did less strict assertions that assertStatusReportChecksSuccessful() (introduced by #3320792: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers)) does.

So I started debugging … and found a mind-boggling rabbit hole. Put a breakpoint in update_get_available() and observe how many times it gets called. It's insane. There's call stacks like this:

update.module:324, update_get_available()
ProjectInfo.php:195, Drupal\package_manager\ProjectInfo->getAvailableProjects()
ProjectInfo.php:109, Drupal\package_manager\ProjectInfo->getInstallableReleases()
VersionPolicyValidator.php:236, Drupal\automatic_updates\Validator\VersionPolicyValidator->getAvailableReleases()
VersionPolicyValidator.php:119, Drupal\automatic_updates\Validator\VersionPolicyValidator->validateVersion()
ReleaseChooser.php:57, Drupal\automatic_updates\ReleaseChooser->Drupal\automatic_updates\{closure:/private/tmp/build_workspace_aaf970c10050f82aef4c7bae01d73e2e72cpBo/project/web/modules/contrib/automatic_updates/src/ReleaseChooser.php:56-58}()
ReleaseChooser.php:62, array_filter()
ReleaseChooser.php:62, Drupal\automatic_updates\ReleaseChooser->getInstallableReleases()
ReleaseChooser.php:84, Drupal\automatic_updates\ReleaseChooser->getMostRecentReleaseInMinor()
ReleaseChooser.php:121, Drupal\automatic_updates\ReleaseChooser->getLatestInInstalledMinor()
CronUpdater.php:135, Drupal\automatic_updates\CronUpdater->getTargetRelease()
VersionPolicyValidator.php:210, Drupal\automatic_updates\Validator\VersionPolicyValidator->getTargetVersion()
VersionPolicyValidator.php:149, Drupal\automatic_updates\Validator\VersionPolicyValidator->checkVersion()
ContainerAwareEventDispatcher.php:111, call_user_func:{/private/tmp/build_workspace_aaf970c10050f82aef4c7bae01d73e2e72cpBo/project/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111}()
ContainerAwareEventDispatcher.php:111, Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch()
StatusCheckTrait.php:49, Drupal\automatic_updates\Validation\StatusChecker->runStatusCheck()
StatusChecker.php:105, Drupal\automatic_updates\Validation\StatusChecker->run()
automatic_updates.module:204, automatic_updates_modules_installed()
<snip>

Note how VersionPolicyValidator (which throws the error messages mentioned in #6 causing tests to fail on 10.x) ends up calling itself. And there's plenty more examples where that came from. The amount of unnecessary function calls is incredible. Now, of course, "premature optimization is the root of all evil", but at this point it interferes with debuggability and hence maintainability.

Another example:

ProjectInfo.php:149, Drupal\package_manager\ProjectInfo->getInstallableReleases()
VersionPolicyValidator.php:236, Drupal\automatic_updates\Validator\VersionPolicyValidator->getAvailableReleases()
VersionPolicyValidator.php:119, Drupal\automatic_updates\Validator\VersionPolicyValidator->validateVersion()
ReleaseChooser.php:57, Drupal\automatic_updates\ReleaseChooser->Drupal\automatic_updates\{closure:/private/tmp/build_workspace_aaf970c10050f82aef4c7bae01d73e2e72cpBo/project/web/modules/contrib/automatic_updates/src/ReleaseChooser.php:56-58}()
ReleaseChooser.php:62, array_filter()
ReleaseChooser.php:62, Drupal\automatic_updates\ReleaseChooser->getInstallableReleases()
ReleaseChooser.php:84, Drupal\automatic_updates\ReleaseChooser->getMostRecentReleaseInMinor()
ReleaseChooser.php:121, Drupal\automatic_updates\ReleaseChooser->getLatestInInstalledMinor()
CronUpdater.php:135, Drupal\automatic_updates\CronUpdater->getTargetRelease()
VersionPolicyValidator.php:210, Drupal\automatic_updates\Validator\VersionPolicyValidator->getTargetVersion()
VersionPolicyValidator.php:149, Drupal\automatic_updates\Validator\VersionPolicyValidator->checkVersion()
<snip>

ProjectInfo->getInstallableReleases() cannot return different results within the same PHP process by definition, yet it is recomputed countless times.

So I did not understand why CoreUpdateTest was not affected by this. It also has a assertStatusReportChecksSuccessful() and it passes tests, why?! What is it doing differently?

Turns out that both ModuleUpdateTest and CoreUpdateTest end up in \Drupal\automatic_updates\Validator\VersionPolicy\SupportedBranchInstalled::validate() looking at the real, actually requested Drupal core version info! 😬😳😱

Turns out that both CoreUpdateTest and ModuleUpdateTest use

    $this->setReleaseMetadata([
      'drupal' => __DIR__ . '/../../../../package_manager/tests/fixtures/release-history/drupal.9.8.1-security.xml',

… and setReleaseMetadata() writes this to settings.php: $config['update_test.settings']['xml_map'] = … → but the update_test module is never even installed! 😳😱

Reclassifying this as core-mvp because this severely undermined my trust in both of these tests

Compared to #6, the extracted/cherry-picked commit results in:

10.1

As expected: the error message no longer mentions 10.1 but 9.8

Testing Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest
FF                                                                  2 / 2 (100%)

Time: 02:39.279, Memory: 18.00 MB

There were 2 failures:

1) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testApi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'
+Updating from Drupal 9.8.1 is not allowed. The currently installed version of Drupal core, 9.8.1, is not in a supported minor version. Your site will not be automatically updated during cron until it is updated to a supported minor version.See the available updates page for available updates.'


2) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testUi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'
+Updating from Drupal 9.8.1 is not allowed. The currently installed version of Drupal core, 9.8.1, is not in a supported minor version. Your site will not be automatically updated during cron until it is updated to a supported minor version.See the available updates page for available updates.'

9.5

… the exact same failures as on 10.1! Which is good: it means we're testing the same thing across all Drupal versions.

Finally found it!

Simplified further.

It looks like the edge case uncovered/investigated in #7 and #9 can also cause random failures: see #3325654-36: Improve the user experience of having your staged update deleted before it was applied, and specifically https://www.drupal.org/pift-ci-job/2583990, which failed like this:

Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-1.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest
.F                                                                  2 / 2 (100%)

Time: 03:13.888, Memory: 18.00 MB

There was 1 failure:

1) Drupal\Tests\automatic_updates_extensions\Build\ModuleUpdateTest::testUi
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-''
+'Error message There is a security update available for your version of Drupal. To ensure the security of your server, you should update immediately! See the available updates page for more information and to install your missing updates.'

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:79
/var/www/html/modules/contrib/automatic_updates/automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php:123
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestSuite.php:673
/var/www/html/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:661
/var/www/html/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/var/www/html/vendor/phpunit/phpunit/src/TextUI/Command.php:97

FAILURES!
Tests: 2, Assertions: 162, Failures: 1.

This now blocks #3341708 — see #3341708-4: Update requirements for 3.x.

Rebased the old MR onto 3.0.x and opened a new MR 👍

Nope, doesn't block that anymore: #3341708-6: Update requirements for 3.x.

@tedbow and @tim.plunkett discussed the plan for getting Automatic Updates, Project Browser and Package Manager into core. We're likely going to aim to land Package Manager first. So postponing this issue for now, until the dust settles on that.

This now blocks #3358878: Test failing on 8.x-2.x on 10.0.x and 10.1.x.

Update on #20: confirmed at #3358878-7: Test failing on 8.x-2.x on 10.0.x and 10.1.x that the fix in this issue is essential.

I'll push this across the finish line, there's too much tricky past history at play here.

No time for this.

I don't see a problem with this. More checks and safety in our tests, especially our build tests, is a good thing.

🥳 Weird to see code you wrote in January landing suddenly 😄