Break up FeedParserTest

The last submitted patch, 2: 3392515-2.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

1. +++ /dev/null
   @@ -1,177 +0,0 @@
   -  /**
   -   * Tests a feed that uses the RSS 0.91 format.
   -   */
   -  public function testRSS091Sample() {
   

   testRSS091Sample() was moved to the Kernel DefaultParserTest::testRss091Sample().

2. +++ /dev/null
   @@ -1,177 +0,0 @@
   -    // Several additional items that include elements over 255 characters.
   -    $this->assertSession()->pageTextContains("Second example feed item title.");
   -    $this->assertSession()->pageTextContains('Long link feed item title');
   -    $this->assertSession()->pageTextContains('Long link feed item description');
   -    $this->assertSession()->linkByHrefExists('http://example.com/tomorrow/and/tomorrow/and/tomorrow/creeps/in/this/petty/pace/from/day/to/day/to/the/last/syllable/of/recorded/time/and/all/our/yesterdays/have/lighted/fools/the/way/to/dusty/death/out/out/brief/candle/life/is/but/a/walking/shadow/a/poor/player/that/struts/and/frets/his/hour/upon/the/stage/and/is/heard/no/more/it/is/a/tale/told/by/an/idiot/full/of/sound/and/fury/signifying/nothing');
   -    $this->assertSession()->pageTextContains('Long author feed item title');
   -    $this->assertSession()->pageTextContains('Long author feed item description');
   -    $this->assertSession()->linkByHrefExists('http://example.com/long/author');
   -
   -    // Test author fields.
   -    $items = \Drupal::entityTypeManager()->getStorage('aggregator_item')->loadByProperties(['title' => 'Long author feed item title.']);
   -    $this->assertCount(1, $items);
   -    $item = reset($items);
   -    assert($item instanceof ItemInterface);
   -    $this->assertStringContainsString('I wanted to get out and walk eastward toward', $item->getAuthor());
   

   Data truncation is the responsibility of the processor. These assertions were moved to new functions in the Kernel DefaultProcessorTest.

3. +++ /dev/null
   @@ -1,177 +0,0 @@
   -  /**
   -   * Tests a feed that uses the Atom format.
   -   */
   -  public function testAtomSample() {
   

   testAtomSample was moved to the Kernel DefaultParserTest::testAtomSample().

4. +++ /dev/null
   @@ -1,177 +0,0 @@
   -  /**
   -   * Tests a feed that uses HTML entities in item titles.
   -   */
   -  public function testHtmlEntitiesSample() {
   -    $feed = $this->createFeed($this->getHtmlEntitiesSample());
   -    $feed->refreshItems();
   -    $this->drupalGet('aggregator/sources/' . $feed->id());
   -    $this->assertSession()->statusCodeEquals(200);
   -    $this->assertSession()->responseContains("Quote" Amp&");
   -  }
   

   This tests Item entity rendering and has nothing to with the parser. The function was moved to AggregatorRenderingTest.

5. +++ /dev/null
   @@ -1,177 +0,0 @@
   -  /**
   -   * Tests that a redirected feed is tracked to its target.
   -   */
   -  public function testRedirectFeed() {
   -    $test_cases = [
   

   Redirection is the responsibility of the fetcher plugin. This was moved to a new Functional DefaultFetcherTest class. I would love to make this a Kernel test. In fact, I still have a Kernel version of it sitting in my file system. But the fact that fetch() builds its own immutable HTTP client makes it impossible without refactoring the fetcher in a backward-incompatible way.

6. +++ /dev/null
   @@ -1,177 +0,0 @@
   -  /**
   -   * Tests error handling when an invalid feed is added.
   -   */
   -  public function testInvalidFeed() {
   

   testInvalidFeed() was moved to the Kernel DefaultParserTest::testInvalidFeed().

I've tried hard to make certain nothing was lost in moving things, particularly in converting from Functional to Kernel tests. One thing that was happening a lot was implicit testing of rendering. I believe this is covered by AggregatorRenderingTest. Please let me know if you notice anything that I missed.

The changes look good to me, just one additional observation

+++ b/tests/src/Functional/Plugin/aggregator/fetcher/DefaultFetcherTest.php
@@ -0,0 +1,64 @@
+  public function testRedirectFeed() {

this test is making no http requests (drupalGet/submitForm) so could be a kernel test?

I'm not sure if this will pass. The kernel DefaultFetcherTest doesn't pass locally. The requests result in a 404 error because the base URL is incorrect. I don't know if Drupal CI corrects this. I don't remember if this is why I made the test a Functional test in the first place. I need to go look up that issue.

Yeah, I had this same problem a month ago. See https://www.drupal.org/project/aggregator/issues/2987167#comment-15219287 🐛 Do not save temporary redirect URLs Fixed .

The last submitted patch, 9: 3392515-9.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Yeah, this is exactly what happens locally. $feed->url doesn't change because the request results in a 404, so the redirect callback that changes the URL is never executed. This is why I tried so hard to mock the HTTP client, so we would avoid the issue of making a request. But the way that the fetcher works makes that impossible. I think we're stuck with a Functional test for this until the fetcher plugin can be redesigned.

I committed the patch from #4 as it was. We'll revisit the fetcher test again later.

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