Convert web tests to browser tests for update module

Hi @Antonnavi, thank you for your contribution!

First tip: Try to use the following git configuration:

[diff]
  renames = copies

This detects renames, see https://www.drupal.org/documentation/git/configure for more details. This will make the patch size significantly smaller.

Here is some quick analysis of the failures:

1) Drupal\Tests\update\Functional\FileTransferAuthorizeFormTest::testViaAuthorize
copy(/var/www/html/core/modules/update/tests/src/Functional/../../tests/update_test_new_module/8.x-1.0/update_test_new_module.tar.gz): failed to open stream: No such file or directory

/var/www/html/core/modules/update/tests/src/Functional/FileTransferAuthorizeFormTest.php:27

FAILURES!
Tests: 1, Assertions: 5, Errors: 1.

You need to adapt the filepaths in places like core/modules/update/src/Tests/UpdateUploadTest.php:66

Update.Drupal\Tests\update\Functional\UpdateCoreTest

These tests rely on batch api to work, see #2855942: Create ::checkForMetaRefresh() on BrowserTestBase. Maybe try to get that issue over the finish line first :)

Here's the fresh patch with expected failures.
Postponed on #2870009: Update: Convert system functional tests to phpunit

//Naveen

Here we go with fixes

Self Review of Interdiff:

1. +++ b/core/modules/update/tests/src/Functional/UpdateTestBase.php
   @@ -58,7 +58,7 @@ protected function setUp() {
   +   * @see \Drupal\update_test\Controller\UpdateTestController::updateTest()
   

   This @see link was broken so I fixed it in this conversion. I hope that should be fine. If anyone will object for it, we could remove it from this patch and create a follow-up issue.

2. +++ b/core/modules/update/tests/src/Functional/UpdateTestBase.php
   @@ -69,6 +69,7 @@ protected function refreshUpdateStatus($xml_map, $url = 'update-test') {
   +    $this->drupalGet('admin/reports/updates');
   

   We need to send the get request again because WTB::clickLink which calls WTB::clickLinkHelper sends a new drupalget request to click the link that refresh the page but BTB::clickLink fires on the NodeElement and it does refresh the whole page. So refreshing it again so that we'll get the updated page. See the code of WTB::clickLinkHelper for reference.

     protected function clickLinkHelper($label, $index, $pattern) {
       // Cast MarkupInterface objects to string.
       $label = (string) $label;
       $url_before = $this->getUrl();
       $urls = $this->xpath($pattern, [':label' => $label]);
       if (isset($urls[$index])) {
         $url_target = $this->getAbsoluteUrl($urls[$index]['href']);
         $this->pass(SafeMarkup::format('Clicked link %label (@url_target) from @url_before', ['%label' => $label, '@url_target' => $url_target, '@url_before' => $url_before]), 'Browser');
         return $this->drupalGet($url_target);
       }
       $this->fail(SafeMarkup::format('Link %label does not exist on @url_before', ['%label' => $label, '@url_before' => $url_before]), 'Browser');
       return FALSE;
     }

//Naveen

Here we go with the more fixes.

Self Review of the interdiff attached:

1. +++ b/core/modules/update/tests/src/Functional/UpdateCoreTest.php
   @@ -76,6 +79,7 @@ public function testNormalUpdateAvailable() {
   +        $this->drupalGet('admin/reports/updates');
   

   This is required to refresh the page. See #8.2 for more details.

2. +++ b/core/modules/update/tests/src/Functional/UpdateCoreTest.php
   @@ -139,6 +143,7 @@ public function testMajorUpdateAvailable() {
   +          $this->drupalGet('admin/reports/updates');
   

   Ditto as above.

Expected Failure:

Drupal\Tests\update\Functional\UpdateContribTest::testUpdateBrokenFetchURL
'Checked available update data for 3 projects.' found only once on the page
Failed asserting that 0 is identical to 1.

/var/www/html/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:168
/var/www/html/core/modules/update/tests/src/Functional/UpdateContribTest.php:372

FAILURES!
Tests: 8, Assertions: 72, Failures: 1.

This test is still failing as it is trying to assert the Drupal set messages which are not present because we're refreshing the page explicitly as per #8.2. What're the thoughts on #8.2?

@naveenvalecha Looks like the solution to #8.2 is not using another drupalGet() but using checkForMetaRefresh() since the click triggers a batch operation.

Thanks Pal!

//naveen

I like that this api is kinda explicit

Thanks, the changes look good. The depreciations and trigger_error() sections match the existing patterns used for tests. Committed!

(Note I did not merge to 8.3.x because the depreciation should only start in 8.4.0).

Test conversion should go in both branches, similarly with the previous conversion issues. This patch could be rerolled for 8.3.x without deprecation notices.

//Naveen

Re-roll for 8.3.x without deprecation notices, as suggested by @naveenvalecha. Although I suspect that @Gábor Hojtsy meant that he does not intend to commit this to 8.3.x.

I'm honestly not 100% clear on the policy here, will ask other committers :)

Ok discussed with @catch. He said so long as the class is not part of the testing infrastructure/API itself, just as in this case a base class to simplify writing tests for the module itself, we can backport the patch as-is. It is unlikely that contributed modules extended the class.

So merged the original commit. Sorry for making you work on this, should have asked other committers first.

Thanks, Gabor! for committing and Jo Fitzgerald! for the patch.

Thanks, Gabor. Don't worry about the extra work - a 997 byte interdiff doesn't take long to produce!