Convert all aggregator web tests to BrowserTestBase

1. diff --git a/core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php b/core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
   deleted file mode 100644
   

   Can you create the patch with "git diff -M25%" so that we can see that this file has been moved?

2. +++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   @@ -23,13 +23,13 @@ public function testAddFeed() {
   -    $this->assertResponse(200, 'Feed source exists.');
   -    $this->assertText($feed->label(), 'Page title');
   +    $this->assertResponse(200);
   +    $this->assertText($feed->label());
   

   Let's not change this lines if they don't break, only makes patch review harder and is out of scope for this conversion issue.

Looks like you are changing many lines just to remove the assertion messages. I think we should not do that here and just convert the test. A later cleanup can convert the assertions to proper $this->assertSession()->... calls that are not deprecated.

1. +++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   @@ -49,7 +49,7 @@ public function testAddFeed() {
   -  public function testFeedLabelEscaping() {
   +  public function _testFeedLabelEscaping() {
   

   So this method should be enabled again, right?

2. +++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   @@ -60,7 +60,7 @@ public function testFeedLabelEscaping() {
   -    $this->assertTrue(strpos(str_replace(["\n", "\r"], '', $this->getRawContent()), 'class="feed-icon">  Subscribe to Test feed title &lt;script&gt;alert(123);&lt;/script&gt; feed</a>') !== FALSE);
   +    $this->assertTrue(strpos(str_replace(["\n", "\r"], '', $this->getTextContent()), 'class="feed-icon">  Subscribe to Test feed title &lt;script&gt;alert(123);&lt;/script&gt; feed</a>') !== FALSE);
   

   Looks like getRawContent() is missing on BrowserTestBase, can you add it there similar to getTextContent()?

3. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   @@ -67,18 +67,18 @@ function testOverviewPage() {
   -    $this->assertEqual($feed->label(), (string) $result[0]->td[0]->a);
   +    $this->assertSession()->pageTextContains($feed->label());
   

   looks like this is changing the logic of the assertion. The new call should search in the specific table cell and link, right?

4. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   @@ -67,18 +67,18 @@ function testOverviewPage() {
   -    $this->assertEqual(\Drupal::translation()->formatPlural($count, '1 item', '@count items'), (string) $result[0]->td[1]);
   +    $this->assertSession()->pageTextContains(\Drupal::translation()->formatPlural($count, '1 item', '@count items'));
   

   Same here, we should look in the table cell like the original assertion did.

   Also elsewhere.

5. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorRenderingTest.php
   @@ -132,10 +132,12 @@ public function testFeedPage() {
   +    // @todo Find better way to handle this.
   +    // Can't do this see http://stackoverflow.com/a/17081317/1177774.
        $outline = $this->xpath('//outline[1]');
   -    $this->assertEqual($outline[0]['type'], 'rss', 'The correct type attribute is used for rss OPML.');
   -    $this->assertEqual($outline[0]['text'], $feed->label(), 'The correct text attribute is used for rss OPML.');
   -    $this->assertEqual($outline[0]['xmlurl'], $feed->getUrl(), 'The correct xmlUrl attribute is used for rss OPML.');
   +//    $this->assertEqual($outline[0]['type'], 'rss', 'The correct type attribute is used for rss OPML.');
   +//    $this->assertEqual($outline[0]['text'], $feed->label(), 'The correct text attribute is used for rss OPML.');
   +//    $this->assertEqual($outline[0]['xmlurl'], $feed->getUrl(), 'The correct xmlUrl attribute is used for rss OPML.');
   

   Ah, you are getting XML back, so you should probably use SimpleXml on the result of drupalGet() to navigate it.

OK, fixed all from #7.

@jibran, that failure (#8.1) is because Mink is getting the plain text from the browser in a different way. We were wrong in WebBaseTest. Mink thinks every time that the test is a human using a real browser. So, now assertText($test) converts the HTML entities in the character actually viewed by a human.

Let's say the the returned page contains this piece of markup: <p>Bad html &lt;script&gt;alert(123);&lt;/script&gt;</p>

In WebTestBase this is passing: $this->assertText('<script>alert(123);</script>') because it wrongly consider that the text means only stripping tags.

In BrowserTestBase will fail but $this->assertText('<script>alert(123);</script>') will pass because Mink is putting in plain text the text as is displayed to a human, in browser. And that is < not <.

1. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorRenderingTest.php
   @@ -132,10 +132,11 @@ public function testFeedPage() {
   +    $xml = simplexml_load_string($this->getRawContent());
   +    $attributes = $xml->xpath('//outline[1]')[0]->attributes();
   +    self::assertEquals($attributes->type, 'rss');
   +    self::assertEquals($attributes->text, $feed->label());
   +    self::assertEquals($attributes->xmlUrl, $feed->getUrl());
   

   Why can't we use $this->xpath instead? Also IMHO we should continue to use $this->assertEquals as we do in every other place in core.

2. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
   @@ -61,7 +61,7 @@ protected function setUp() {
   -    $this->assertText(t('The feed @name has been added.', array('@name' => $edit['title[0][value]'])), format_string('The feed @name has been added.', array('@name' => $edit['title[0][value]'])));
   +    $this->assertText(t("The feed {$edit['title[0][value]']} has been added."), format_string('The feed @name has been added.', array('@name' => $edit['title[0][value]'])));
   

   Do we really have to change this line? In case we do, we should IMHO drop the t() statement.

3. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -1580,6 +1594,44 @@ protected function getTextContent() {
   +
   +  /**
   +   * Checks for meta refresh tag and if found call drupalGet() recursively.
   +   *
   +   * This function looks for the http-equiv attribute to be set to "Refresh" and
   +   * is case-sensitive.
   +   *
   +   * @return string|false
   +   *   Either the new page content or FALSE.
   +   */
   +  protected function checkForMetaRefresh() {
   +    $refresh = $this->cssSelect('meta[http-equiv="Refresh"]');
   +    if (!empty($refresh) && (!isset($this->maximumMetaRefreshCount) || $this->metaRefreshCount < $this->maximumMetaRefreshCount)) {
   +      // Parse the content attribute of the meta tag for the format:
   +      // "[delay]: URL=[page_to_redirect_to]".
   +      if (preg_match('/\d+;\s*URL=(?<url>.*)/i', $refresh[0]->getAttribute('content'), $match)) {
   +        $this->metaRefreshCount++;
   +        return $this->drupalGet($this->getAbsoluteUrl(Html::decodeEntities($match['url'])));
   +      }
   +    }
   +    return FALSE;
   

   I'm wondering whether mink actually implements the same kind of logic internally already. It feels as if it would belong on that level

This I will postpone on #2773733: Fix AssertLegacyTrait::assert(No)Text to handle html encoded strings (part 3). There are several conversions that need that, so let's fix it in a dedicated issue.

+1 for checkForMetaRefresh().

It would make it possible to make functional tests on stuff that uses the batch API without requiring JavascriptTestBase and without adding the same function to every module that needs it.

It doesn't need to be recursive. A loop would use less memory.

I did some grepping around in /vendor and /core/lib/Drupal/Component for stuff like redirect and refresh and I don't think it is built into Mink.

Look at how Mahara does it: https://git.mahara.org/mahara/mahara/blob/16.04.3_RELEASE/htdocs/testing... in i_wait_to_be_redirected().

Similar example from Behat Drupal Extensions: https://www.drupal.org/node/2011390 .

Un-postponing because #2773733: Fix AssertLegacyTrait::assert(No)Text to handle html encoded strings (part 3) is in.

@dawehner,

#10.1

Why can't we use $this->xpath instead?

Because $this->xpath() doesn't handle non-HTML pages. Here we have OPML so we need this workaround.

#10.3: I searched but no luck. But if you find something..

1. diff --git a/core/modules/aggregator/src/Tests/AddFeedTest.php b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   similarity index 98%
   rename from core/modules/aggregator/src/Tests/AddFeedTest.php
   rename to core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   index b7ffe5f..90fff77 100644
   --- a/core/modules/aggregator/src/Tests/AddFeedTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Add feed test.
   ...
   diff --git a/core/modules/aggregator/src/Tests/AggregatorCronTest.php b/core/modules/aggregator/tests/src/Functional/AggregatorCronTest.php
   similarity index 97%
   rename from core/modules/aggregator/src/Tests/AggregatorCronTest.php
   rename to core/modules/aggregator/tests/src/Functional/AggregatorCronTest.php
   index 34a4cb9..ef8dc41 100644
   --- a/core/modules/aggregator/src/Tests/AggregatorCronTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/AggregatorCronTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Update feeds on cron.
   ...
   similarity index 98%
   rename from core/modules/aggregator/src/Tests/AggregatorTestBase.php
   rename to core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
   index 15be144..a6c9c32 100644
   --- a/core/modules/aggregator/src/Tests/AggregatorTestBase.php
   +++ b/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
   @@ -1,16 +1,16 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    use Drupal\aggregator\Entity\Feed;
    use Drupal\Component\Utility\Html;
   -use Drupal\simpletest\WebTestBase;
   +use Drupal\Tests\BrowserTestBase;
    use Drupal\aggregator\FeedInterface;
    
    /**
     * Defines a base class for testing the Aggregator module.
     */
   -abstract class AggregatorTestBase extends WebTestBase {
   +abstract class AggregatorTestBase extends BrowserTestBase {
    
      /**
       * A user with permission to administer feeds and create content.
   diff --git a/core/modules/aggregator/src/Tests/DeleteFeedItemTest.php b/core/modules/aggregator/tests/src/Functional/DeleteFeedItemTest.php
   similarity index 96%
   rename from core/modules/aggregator/src/Tests/DeleteFeedItemTest.php
   rename to core/modules/aggregator/tests/src/Functional/DeleteFeedItemTest.php
   index 0261ed5..95202b8 100644
   --- a/core/modules/aggregator/src/Tests/DeleteFeedItemTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/DeleteFeedItemTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Delete feed items from a feed.
   diff --git a/core/modules/aggregator/src/Tests/DeleteFeedTest.php b/core/modules/aggregator/tests/src/Functional/DeleteFeedTest.php
   similarity index 97%
   rename from core/modules/aggregator/src/Tests/DeleteFeedTest.php
   rename to core/modules/aggregator/tests/src/Functional/DeleteFeedTest.php
   index 4cde81a..5bf5bd3 100644
   --- a/core/modules/aggregator/src/Tests/DeleteFeedTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/DeleteFeedTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Delete feed test.
   diff --git a/core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php b/core/modules/aggregator/tests/src/Functional/FeedAdminDisplayTest.php
   similarity index 98%
   rename from core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
   rename to core/modules/aggregator/tests/src/Functional/FeedAdminDisplayTest.php
   index 0324e3b..9eb5cac 100644
   --- a/core/modules/aggregator/src/Tests/FeedAdminDisplayTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/FeedAdminDisplayTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Tests the display of a feed on the Aggregator list page.
   diff --git a/core/modules/aggregator/src/Tests/FeedFetcherPluginTest.php b/core/modules/aggregator/tests/src/Functional/FeedFetcherPluginTest.php
   similarity index 95%
   rename from core/modules/aggregator/src/Tests/FeedFetcherPluginTest.php
   rename to core/modules/aggregator/tests/src/Functional/FeedFetcherPluginTest.php
   index 4aac851..1a03186 100644
   --- a/core/modules/aggregator/src/Tests/FeedFetcherPluginTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/FeedFetcherPluginTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Tests the fetcher plugins functionality and discoverability.
   diff --git a/core/modules/aggregator/src/Tests/FeedLanguageTest.php b/core/modules/aggregator/tests/src/Functional/FeedLanguageTest.php
   similarity index 98%
   rename from core/modules/aggregator/src/Tests/FeedLanguageTest.php
   rename to core/modules/aggregator/tests/src/Functional/FeedLanguageTest.php
   index af8335e..81f8b42 100644
   --- a/core/modules/aggregator/src/Tests/FeedLanguageTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/FeedLanguageTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    use Drupal\language\Entity\ConfigurableLanguage;
    
   diff --git a/core/modules/aggregator/src/Tests/FeedParserTest.php b/core/modules/aggregator/tests/src/Functional/FeedParserTest.php
   similarity index 99%
   rename from core/modules/aggregator/src/Tests/FeedParserTest.php
   rename to core/modules/aggregator/tests/src/Functional/FeedParserTest.php
   index 6c4a04d..00c7b70 100644
   --- a/core/modules/aggregator/src/Tests/FeedParserTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/FeedParserTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    use Drupal\Core\Url;
    use Drupal\aggregator\Entity\Feed;
   diff --git a/core/modules/aggregator/src/Tests/FeedProcessorPluginTest.php b/core/modules/aggregator/tests/src/Functional/FeedProcessorPluginTest.php
   similarity index 97%
   rename from core/modules/aggregator/src/Tests/FeedProcessorPluginTest.php
   rename to core/modules/aggregator/tests/src/Functional/FeedProcessorPluginTest.php
   index 8a7ade0..5527d95 100644
   --- a/core/modules/aggregator/src/Tests/FeedProcessorPluginTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/FeedProcessorPluginTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    use Drupal\aggregator\Entity\Feed;
    use Drupal\aggregator\Entity\Item;
   diff --git a/core/modules/aggregator/src/Tests/ImportOpmlTest.php b/core/modules/aggregator/tests/src/Functional/ImportOpmlTest.php
   similarity index 99%
   rename from core/modules/aggregator/src/Tests/ImportOpmlTest.php
   rename to core/modules/aggregator/tests/src/Functional/ImportOpmlTest.php
   index 1f1e90d..8994e3f 100644
   --- a/core/modules/aggregator/src/Tests/ImportOpmlTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/ImportOpmlTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Tests OPML import.
   diff --git a/core/modules/aggregator/src/Tests/UpdateFeedItemTest.php b/core/modules/aggregator/tests/src/Functional/UpdateFeedItemTest.php
   similarity index 98%
   rename from core/modules/aggregator/src/Tests/UpdateFeedItemTest.php
   rename to core/modules/aggregator/tests/src/Functional/UpdateFeedItemTest.php
   index ae78205..ba99dd0 100644
   --- a/core/modules/aggregator/src/Tests/UpdateFeedItemTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/UpdateFeedItemTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    use Drupal\aggregator\Entity\Feed;
    
    /**
   diff --git a/core/modules/aggregator/src/Tests/UpdateFeedTest.php b/core/modules/aggregator/tests/src/Functional/UpdateFeedTest.php
   similarity index 97%
   rename from core/modules/aggregator/src/Tests/UpdateFeedTest.php
   rename to core/modules/aggregator/tests/src/Functional/UpdateFeedTest.php
   index 8c797a5..bb830a5 100644
   --- a/core/modules/aggregator/src/Tests/UpdateFeedTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/UpdateFeedTest.php
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
   

   Those ones are part of the move issue, so let's skip that here.

2. +++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
   similarity index 86%
   rename from core/modules/aggregator/src/Tests/AggregatorAdminTest.php
   
   rename from core/modules/aggregator/src/Tests/AggregatorAdminTest.php
   rename to core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   
   rename to core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   index 487e0ef..2a1bb2e 100644
   
   index 487e0ef..2a1bb2e 100644
   --- a/core/modules/aggregator/src/Tests/AggregatorAdminTest.php
   
   --- a/core/modules/aggregator/src/Tests/AggregatorAdminTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   
   +++ b/core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   +++ b/core/modules/aggregator/tests/src/Functional/AggregatorAdminTest.php
   @@ -1,6 +1,6 @@
   
   @@ -1,6 +1,6 @@
    <?php
    
   -namespace Drupal\aggregator\Tests;
   +namespace Drupal\Tests\aggregator\Functional;
    
    /**
     * Tests aggregator admin pages.
   @@ -67,18 +67,21 @@ function testOverviewPage() {
   
   @@ -67,18 +67,21 @@ function testOverviewPage() {
        // Check if the amount of feeds in the overview matches the amount created.
        $this->assertEqual(1, count($result), 'Created feed is found in the overview');
        // Check if the fields in the table match with what's expected.
   -    $this->assertEqual($feed->label(), (string) $result[0]->td[0]->a);
   +    $link = $this->xpath('//table/tbody/tr//td[1]/a');
   +    $this->assertEqual($feed->label(), $link[0]->getText());
        $count = $this->container->get('entity.manager')->getStorage('aggregator_item')->getItemCount($feed);
   -    $this->assertEqual(\Drupal::translation()->formatPlural($count, '1 item', '@count items'), (string) $result[0]->td[1]);
   +    $td = $this->xpath('//table/tbody/tr//td[2]');
   +    $this->assertEqual(\Drupal::translation()->formatPlural($count, '1 item', '@count items'), $td[0]->getText());
    
        // Update the items of the first feed.
        $feed->refreshItems();
        $this->drupalGet('admin/config/services/aggregator');
   -    $result = $this->xpath('//table/tbody/tr');
        // Check if the fields in the table match with what's expected.
   -    $this->assertEqual($feed->label(), (string) $result[0]->td[0]->a);
   +    $link = $this->xpath('//table/tbody/tr//td[1]/a');
   +    $this->assertEqual($feed->label(), $link[0]->getText());
        $count = $this->container->get('entity.manager')->getStorage('aggregator_item')->getItemCount($feed);
   -    $this->assertEqual(\Drupal::translation()->formatPlural($count, '1 item', '@count items'), (string) $result[0]->td[1]);
   +    $td = $this->xpath('//table/tbody/tr//td[2]');
   +    $this->assertEqual(\Drupal::translation()->formatPlural($count, '1 item', '@count items'), $td[0]->getText());
      }
    
    }
   

   At least those should be able to leverage #2809181: Provide forward compatibility layer for BrowserTestBase::xpath

3. +++ b/core/modules/aggregator/tests/src/Functional/UpdateFeedTest.php
   diff --git a/core/tests/Drupal/Tests/BrowserTestBase.php b/core/tests/Drupal/Tests/BrowserTestBase.php
   index f9f443c..a5e4535 100644
   
   index f9f443c..a5e4535 100644
   --- a/core/tests/Drupal/Tests/BrowserTestBase.php
   
   --- a/core/tests/Drupal/Tests/BrowserTestBase.php
   +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   
   +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -279,6 +279,20 @@
   
   @@ -279,6 +279,20 @@
      protected $preserveGlobalState = FALSE;
    
      /**
   +   * The number of meta refresh redirects to follow, or NULL if unlimited.
   +   *
   +   * @var null|int
   +   */
   +  protected $maximumMetaRefreshCount = NULL;
   

   Do we have a dedicated issue for those changes? These should be for example tested separately

Re-rolled.

1. +++ /dev/null
   @@ -1,382 +0,0 @@
   - * @deprecated Scheduled for removal in Drupal 9.0.0.
   - *   Use \Drupal\Tests\aggregator\Functional\AggregatorTestBase instead.
   - */
   -abstract class AggregatorTestBase extends WebTestBase {
   

   This is an old base class and must not be removed. It is depreacted but it must stay. Sorry for not telling you earlier :)

2. +++ b/core/modules/aggregator/tests/src/Functional/AggregatorRenderingTest.php
   @@ -132,10 +132,11 @@ public function testFeedPage() {
   +    $xml = simplexml_load_string($this->getRawContent());
   

   This should have a comment why we have to use simplexml_load_string() here. Example: // We can't use Mink XPAth queries here because it only supports HTML pages, but we are dealing with XML here.

3. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -1240,6 +1254,44 @@ protected function getTextContent() {
      /**
   +   * Retrieves the raw content from the current page.
   +   */
   +  protected function getRawContent() {
   

   not sure if we should add this method in this issue or create a new one for that ...

4. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -1240,6 +1254,44 @@ protected function getTextContent() {
   +  /**
   +   * Runs cron.
   +   */
   +  protected function cronRun() {
   

   we decided to not add this method to BrowserTestBase. Use the CronRunTrait for test cases that need cron instead.

5. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -1240,6 +1254,44 @@ protected function getTextContent() {
   +  /**
   +   * Checks for meta refresh tag and if found call drupalGet() recursively.
   +   *
   +   * This function looks for the http-equiv attribute to be set to "Refresh" and
   +   * is case-sensitive.
   +   *
   +   * @return string|false
   +   *   Either the new page content or FALSE.
   +   */
   +  protected function checkForMetaRefresh() {
   

   this method is only used by 2 tests in Drupal core. Let's not add it on BrowserTestBase, let's create a new issue to have a trait with that method instead that can be used by Simpletest's WebTestBase and the converted tests.

• #18.1, 2, 4 : fixed.
• #18.3: We can use the existing methods, I don't think we need a wrapper method for that.
• #18.5: Opened #2855942: Create ::checkForMetaRefresh() on BrowserTestBase. This is postponed on that because it need the trait. Please review that.

Hm. I think we should move the trait creation back here for the reasons I mentioned in #2855942-5: Create ::checkForMetaRefresh() on BrowserTestBase. What you think, @klausi?

Yes, checkForMetaRefresh() is only used by aggregator and big_pipe, so I think is fine to do here.

i'm working on this to merge with #2855942: Create ::checkForMetaRefresh() on BrowserTestBase

+++ b/core/modules/aggregator/tests/src/Functional/AddFeedTest.php
@@ -60,7 +64,7 @@ public function testFeedLabelEscaping() {
-    $this->assertTrue(strpos(str_replace(["\n", "\r"], '', $this->getRawContent()), 'class="feed-icon">  Subscribe to Test feed title &lt;script&gt;alert(123);&lt;/script&gt; feed</a>') !== FALSE);
+    $this->assertTrue(strpos(str_replace(["\n", "\r"], '', $this->getSession()->getPage()->getContent()), 'class="feed-icon">  Subscribe to Test feed title &lt;script&gt;alert(123);&lt;/script&gt; feed</a>') !== FALSE);

We shouldn't change this line, we should have the getRawContent() method on BrowserTestBase or AssertLegacyTrait. The original getRawContent() method is in AssertContentTrait, so I vote to implement it in AssertLegacyTrait. We should open an issue for that and add test coverage for it to BrowserTestBaseTest.

Postponing on that, please open an issue and link it here.

Issue already exists. getRawContent() should be discussed on #2795111: WTB to BTB, add getRawContent() in BTB

This issue can be worked on again ...

Let's postpone it on #2855942: Create ::checkForMetaRefresh() on BrowserTestBase for now

This landed #2862885: Batch: Convert system functional tests to phpunit and it unblocked this issue.
//Naveen

Assigning to myself for fixing the failures.

//Naveen

I have added two patches

2757023-34-2.patch: This is the straightforward conversion of the WTB to BTB
2757023-34-remove-AggregatorTestBase-class.patch: This patch removes the "src/Tests/AggregatorTestBase" Class from the codebase as no other test is using it. What're the thoughts on removing the deprecated Base class? However, tests are not the API as per the BC policy.

//Naveen

I think the discussion to remove the base class should be its own issue. Its one less thing to deal with for reviewers/committers ...

#35: Filed a follow-up #2886547: Add trigger_error to deprecated TestBase base classes/traits that have none, reference CR
2757023-34-2.patch is ready for review. Is it RTBC?

//Naveen

I'd say so

Thanks for the separate deprecation issue, makes this a much easier commit for the substantive change, then that one is just yes or no eventually.

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!