From 0316b69572bc0e0fdad095dd7ffa5214baa3f1a0 Mon Sep 17 00:00:00 2001 From: Ted Bowman <ted+git@tedbow.com> Date: Mon, 30 Jan 2023 20:18:11 -0500 Subject: [PATCH] Issue #3320792 by tedbow, Wim Leers, kunal.sachdev: Make build tests fail 1) more explicitly, 2) earlier when possible (failing StatusCheckEvent subscribers) --- .../tests/src/Build/ModuleUpdateTest.php | 32 +++++++++-------- .../tests/src/Build/PackageInstallTest.php | 26 ++++++-------- .../tests/src/Build/PackageUpdateTest.php | 31 ++++++++-------- .../src/Build/TemplateProjectTestBase.php | 36 +++++++++++++++++-- tests/src/Build/CoreUpdateTest.php | 14 ++++++-- tests/src/Build/UpdateTestBase.php | 28 +++++++++++++++ 6 files changed, 116 insertions(+), 51 deletions(-) diff --git a/automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php b/automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php index cf0ec57043..9175c79437 100644 --- a/automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php +++ b/automatic_updates_extensions/tests/src/Build/ModuleUpdateTest.php @@ -79,20 +79,17 @@ END; // composer.json file, so we can assert that they were updated to the // version we expect. // @see \Drupal\automatic_updates_extensions_test_api\ApiController::run() - $query = http_build_query([ - 'projects' => [ - 'alpha' => '1.1.0', - ], - 'files_to_return' => [ - 'web/modules/contrib/alpha/composer.json', - ], - ]); - $this->visit("/automatic-updates-extensions-test-api?$query"); - $mink = $this->getMink(); - $mink->assertSession()->statusCodeEquals(200); - - $file_contents = $mink->getSession()->getPage()->getContent(); - $file_contents = json_decode($file_contents, TRUE); + $file_contents = $this->getPackageManagerTestApiResponse( + '/automatic-updates-extensions-test-api', + [ + 'projects' => [ + 'alpha' => '1.1.0', + ], + 'files_to_return' => [ + 'web/modules/contrib/alpha/composer.json', + ], + ] + ); $module_composer_json = json_decode($file_contents['web/modules/contrib/alpha/composer.json']); $this->assertSame('1.1.0', $module_composer_json->version); @@ -118,6 +115,13 @@ END; // Confirm that a 'New Module' project does not appear on the form. $assert_session->pageTextContains('Other updates were found, but they must be performed manually.'); $assert_session->fieldNotExists('projects[new_module]'); + // Ensure test failures provide helpful debug output when failing readiness + // checks prevent updates. + // @see \Drupal\Tests\WebAssert::buildStatusMessageSelector() + if ($error_message = $session->getPage()->find('xpath', '//div[@data-drupal-messages]//div[@aria-label="Error message"]')) { + /** @var \Behat\Mink\Element\NodeElement $error_message */ + $this->assertSame('', $error_message->getText()); + } $page->checkField('projects[alpha]'); $page->pressButton('Update'); $this->waitForBatchJob(); diff --git a/package_manager/tests/src/Build/PackageInstallTest.php b/package_manager/tests/src/Build/PackageInstallTest.php index 62c9330778..886834f659 100644 --- a/package_manager/tests/src/Build/PackageInstallTest.php +++ b/package_manager/tests/src/Build/PackageInstallTest.php @@ -27,21 +27,17 @@ class PackageInstallTest extends TemplateProjectTestBase { // the API to return the contents of composer.json file of installed module, // so we can assert that the module was installed with the expected version. // @see \Drupal\package_manager_test_api\ApiController::run() - $query = http_build_query([ - 'runtime' => [ - 'drupal/alpha:1.0.0', - ], - 'files_to_return' => [ - 'web/modules/contrib/alpha/composer.json', - ], - ]); - $this->visit("/package-manager-test-api?$query"); - $mink = $this->getMink(); - $mink->assertSession()->statusCodeEquals(200); - - $file_contents = $mink->getSession()->getPage()->getContent(); - $file_contents = json_decode($file_contents, TRUE); - + $file_contents = $this->getPackageManagerTestApiResponse( + '/package-manager-test-api', + [ + 'runtime' => [ + 'drupal/alpha:1.0.0', + ], + 'files_to_return' => [ + 'web/modules/contrib/alpha/composer.json', + ], + ] + ); $this->assertArrayHasKey('web/modules/contrib/alpha/composer.json', $file_contents); } diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php index 63c1da6fd3..59f3e0651e 100644 --- a/package_manager/tests/src/Build/PackageUpdateTest.php +++ b/package_manager/tests/src/Build/PackageUpdateTest.php @@ -42,23 +42,20 @@ class PackageUpdateTest extends TemplateProjectTestBase { // of both modules' composer.json files, so we can assert that they were // updated to the versions we expect. // @see \Drupal\package_manager_test_api\ApiController::run() - $query = http_build_query([ - 'runtime' => [ - 'drupal/updated_module:1.1.0', - ], - 'files_to_return' => [ - 'web/modules/contrib/alpha/composer.json', - 'web/modules/contrib/updated_module/composer.json', - 'bravo.txt', - "system_changes.json", - ], - ]); - $this->visit("/package-manager-test-api?$query"); - $mink = $this->getMink(); - $mink->assertSession()->statusCodeEquals(200); - - $file_contents = $mink->getSession()->getPage()->getContent(); - $file_contents = json_decode($file_contents, TRUE); + $file_contents = $this->getPackageManagerTestApiResponse( + '/package-manager-test-api', + [ + 'runtime' => [ + 'drupal/updated_module:1.1.0', + ], + 'files_to_return' => [ + 'web/modules/contrib/alpha/composer.json', + 'web/modules/contrib/updated_module/composer.json', + 'bravo.txt', + "system_changes.json", + ], + ] + ); $expected_versions = [ 'alpha' => '1.0.0', diff --git a/package_manager/tests/src/Build/TemplateProjectTestBase.php b/package_manager/tests/src/Build/TemplateProjectTestBase.php index 47ecdac12c..278346f487 100644 --- a/package_manager/tests/src/Build/TemplateProjectTestBase.php +++ b/package_manager/tests/src/Build/TemplateProjectTestBase.php @@ -488,10 +488,12 @@ END; * that once if they will be fired multiple times. If there are no events * specified all life cycle events from PreCreateEvent to PostDestroyEvent * will be asserted. + * @param string|null $message + * (optional) A message to display with the assertion. * * @see \Drupal\package_manager_test_event_logger\EventSubscriber\EventLogSubscriber::logEventInfo */ - protected function assertExpectedStageEventsFired(string $expected_stage_class, ?array $expected_events = NULL): void { + protected function assertExpectedStageEventsFired(string $expected_stage_class, ?array $expected_events = NULL, ?string $message = NULL): void { if ($expected_events === NULL) { $expected_events = [ PreCreateEvent::class, @@ -535,7 +537,37 @@ END; foreach ($expected_events as $event) { $expected_titles[] = "package_manager_test_event_logger-start: Event: $event, Stage instance of: $expected_stage_class:package_manager_test_event_logger-end"; } - $this->assertSame($expected_titles, $actual_titles); + $this->assertSame($expected_titles, $actual_titles, $message ?? ''); + } + + /** + * Gets a /package-manager-test-api response. + * + * @param string $url + * The package manager test API URL to fetch. + * @param array $query_data + * The query data. + * + * @return array + * The received JSON. + */ + protected function getPackageManagerTestApiResponse(string $url, array $query_data): array { + $url .= '?' . http_build_query($query_data); + $this->visit($url); + $mink = $this->getMink(); + $session = $mink->getSession(); + $file_contents = $session->getPage()->getContent(); + + // Ensure test failures provide helpful debug output when there's a fatal + // PHP error: don't use \Behat\Mink\WebAssert::statusCodeEquals(). + if ($session->getStatusCode() == 500) { + $this->assertEquals(200, 500, 'Error response: ' . $file_contents); + } + else { + $mink->assertSession()->statusCodeEquals(200); + } + + return json_decode($file_contents, TRUE); } // BEGIN: DELETE FROM CORE MERGE REQUEST. diff --git a/tests/src/Build/CoreUpdateTest.php b/tests/src/Build/CoreUpdateTest.php index 10296accb1..a39abb67ad 100644 --- a/tests/src/Build/CoreUpdateTest.php +++ b/tests/src/Build/CoreUpdateTest.php @@ -84,6 +84,8 @@ class CoreUpdateTest extends UpdateTestBase { $this->visit('/admin/modules/update'); $this->getMink()->assertSession()->pageTextContains('9.8.1'); + $this->assertStatusReportChecksSuccessful(); + // Ensure that Drupal has write-protected the site directory. $this->assertDirectoryIsNotWritable($this->getWebRoot() . '/sites/default'); } @@ -126,7 +128,8 @@ class CoreUpdateTest extends UpdateTestBase { PostApplyEvent::class, PreDestroyEvent::class, PostDestroyEvent::class, - ] + ], + 'Error response: ' . $file_contents ); // Even though the response is what we expect, assert the status code as // well, to be extra-certain that there was no kind of server-side error. @@ -179,8 +182,6 @@ class CoreUpdateTest extends UpdateTestBase { $mink = $this->getMink(); $page = $mink->getSession()->getPage(); $assert_session = $mink->assertSession(); - - $assert_session->pageTextContains('Your site is ready for automatic updates.'); $page->clickLink('Run cron'); $cron_run_status_code = $mink->getSession()->getStatusCode(); $this->assertExpectedStageEventsFired(CronUpdater::class); @@ -396,6 +397,13 @@ class CoreUpdateTest extends UpdateTestBase { $session->reload(); $assert_session->pageTextNotContains('There is a security update available for your version of Drupal.'); + // Ensure test failures provide helpful debug output when failing readiness + // checks prevent updates. + // @see \Drupal\Tests\WebAssert::buildStatusMessageSelector() + if ($error_message = $session->getPage()->find('xpath', '//div[@data-drupal-messages]//div[@aria-label="Error message"]')) { + /** @var \Behat\Mink\Element\NodeElement $error_message */ + $this->assertSame('', $error_message->getText()); + } $page->pressButton('Update to 9.8.1'); $this->waitForBatchJob(); $assert_session->pageTextContains('Ready to update'); diff --git a/tests/src/Build/UpdateTestBase.php b/tests/src/Build/UpdateTestBase.php index 839b3f5553..6ff090d657 100644 --- a/tests/src/Build/UpdateTestBase.php +++ b/tests/src/Build/UpdateTestBase.php @@ -62,4 +62,32 @@ END; } } + /** + * Asserts the status report does not have any readiness errors or warnings. + */ + protected function assertStatusReportChecksSuccessful(): void { + $this->visit('/admin/reports/status'); + $mink = $this->getMink(); + $page = $mink->getSession()->getPage(); + + $readiness_check_summaries = $page->findAll('css', 'summary:contains("Update readiness checks")'); + // There should always either be the summary section indicating the site is + // ready for automatic updates or the error or warning sections. + $this->assertNotEmpty($readiness_check_summaries); + $ready_text_found = FALSE; + $status_checks_text = ''; + foreach ($readiness_check_summaries as $readiness_check_summary) { + $parent_element = $readiness_check_summary->getParent(); + if (str_contains($parent_element->getText(), 'Your site is ready for automatic updates.')) { + $ready_text_found = TRUE; + continue; + } + $description_list = $parent_element->find('css', 'div.description ul'); + $this->assertNotEmpty($description_list); + $status_checks_text .= "\n" . $description_list->getText(); + } + $this->assertSame('', $status_checks_text); + $this->assertTrue($ready_text_found); + } + } -- GitLab