Add test coverage of Views RSS feeds with translated content

Here's the patch that reverts commit 69e0b9feb9ed from 8.8.x and adds the new test.

This will fail, since the test proves that multi-lingual RSS feeds were broken before the fix.

Note: this isn't a complete revert, since part of #2673980 was fixing the test view used for RSS tests to be configured properly to use reasonable settings for the RSS fields (e.g. title, link, description, etc). See #2673980-87: RSS view with fields give wrong URL from path field for more. So I left the changes to core/modules/views/tests/modules/views_test_config/test_views/views.view.test_display_feed.yml in my branch to generate this patch.

And here's the patch we should (probably) commit, which just adds the new test.

This should pass.

Let's see what the bot says...

Thanks!
-Derek

p.s. I submitted this as 'Major' since it's a follow-up to an existing 'Major' issue. 'Normal' is probably a more accurate priority, but given its relationship to an existing major, I figured I'd just stick with that. ;)

See, this is why I wanted to do this churning in a sub issue, not spamming everyone in #2673980: RSS view with fields give wrong URL from path field. ;)

I was running just the test class directly with phpunit. Didn't realize I had a mismatch of the filename and the class name (which is probably what's confusing the bot).

Also remove the unused use (which were from previous iterations of the test).

Forgive me, this is my first time writing a multilingual test, and I'm not sure I'm doing everything right. I pieced this together looking at some of the prior art.

Finally, I realized the actual assertions of the XML output could be simplified.

Anyway, here's a new version of the should-fail revert + new test.

... and the new test-only, without the revert. This one should pass.

p.s. For completeness, here's the interdiff against #3, but it's the identical interdiff as I posted in #5.

Yay, those are the results I expected. ;)

Here's the fail from #5:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://php-apache-jenkins-drupal-patches-15464/subdirectory/pt-br/node/1'
+'http://php-apache-jenkins-drupal-patches-15464/subdirectory/node/1'

So if we revert the fix from #2673980: RSS view with fields give wrong URL from path field we lose language prefixes on a feed of translated content.

All the links are right in your feed if you leave the fix in (#6).

There are probably improvements we could make to this test. As I said, I'm not well versed in writing multilingual tests for core.

But I think this is close to RTBC, if not ready.

Thanks!
-Derek

Nice catch, @Lendude! I didn't realize that was a thing. Thanks for pointing that out!

I was tempted to add an explicit sort order to the display view, but I re-wrote the test assertions to not care about the order of items in the feed. This all works as expected locally (MySQL) and I hope PgSQL and SQLite agree...

#11 should fail.

This should pass. I'll queue both for all 3 DBs.

Once again, identical interdiff, but posted for completeness.

p.s. Ideally #11 will fail on all DBs with this:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://localhost/drupal-8_8/pt-br/node/1'
+'http://localhost/drupal-8_8/node/1'

Otherwise, I still messed something up with ordering (but I doubt that'll happen, given how I re-wrote the assertions).

I'll check back in ~1.5 hours to see what the bot thinks. ;)

And here's #12 plus what we need so this doesn't fail once #3082655: Specify the $defaultTheme property in all functional tests lands. Doesn't hurt to specify this already since #2352949: Deprecate using Classy as the default theme for the 'testing' profile is already in. But #3082655 will make it required for all test classes. If we backport the test to 8.7.x, we should use #12. But for 8.8.x and above, we should use this.

Oh, duh. I queued #12 for test runs on 8.7.x, but #2673980: RSS view with fields give wrong URL from path field wasn't cherry-picked to 8.7.x. So those will fail. I guess that'll show why we want to backport the parent, too. ;)

p.s. I have a patch for #3092571: Fix RSS test view (and related tests) to not put links in the title element (testbot running now). If that lands first, we can remove the @todo and @see in here, and fix the xpath selector. If this lands first, that patch should be updated to fix the new selector in here and remove the @todo. Didn't seem worth blocking one on the other. I'll leave it to committer discretion.

Thanks,
-Derek

Re-queued #12 on SQLite since the Fail was a random fail from media_library JS tests:

1) Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest::testWidgetUpload
"The media item image-1.png has been removed." not found
Failed asserting that a boolean is not empty.

Excellent, #11 is failing properly:

MySQL: https://www.drupal.org/pift-ci-job/1462110

1) Drupal\Tests\views\Functional\Plugin\DisplayFeedTranslationTest::testFeedFieldOutput
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://php-apache-jenkins-drupal-patches-15606/subdirectory/pt-br/node/1'
+'http://php-apache-jenkins-drupal-patches-15606/subdirectory/node/1'

PgSQL: https://www.drupal.org/pift-ci-job/1462112

1) Drupal\Tests\views\Functional\Plugin\DisplayFeedTranslationTest::testFeedFieldOutput
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://php-apache-jenkins-drupal-patches-15608/subdirectory/es/node/1'
+'http://php-apache-jenkins-drupal-patches-15608/subdirectory/node/1'

SQLite: https://www.drupal.org/pift-ci-job/1462113

1) Drupal\Tests\views\Functional\Plugin\DisplayFeedTranslationTest::testFeedFieldOutput
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://php-apache-jenkins-drupal-patches-15609/subdirectory/pt-br/node/1'
+'http://php-apache-jenkins-drupal-patches-15609/subdirectory/node/1'

Meanwhile, #12 is failing on 8.8.x for PgSQL for unrelated problems:
PgSQL: https://www.drupal.org/pift-ci-job/1462114

There were 2 failures:

1) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "composite_primary_key" (array('test_field', 'other_test_field'), array('test_field_renamed', 'other_test_field'))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'test_field_renamed'
-    1 => 'other_test_field'
+    0 => 'other_test_field'
+    1 => 'test_field_renamed'

/var/www/html/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit6/TestCompatibilityTrait.php:51
/var/www/html/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php:858

2) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "composite_primary_key_different_order" (array('other_test_field', 'test_field'), array('other_test_field', 'test_field_renamed'))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'other_test_field'
-    1 => 'test_field_renamed'
+    0 => 'test_field_renamed'
+    1 => 'other_test_field'

/var/www/html/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit6/TestCompatibilityTrait.php:51
/var/www/html/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php:858

So far, #14 is looking good, so instead of causing more testbot churn re-queuing #12, let's see how things settle with #14 for PgSQL...

Almost there! :)

Thanks,
-Derek

Well, #14 results aren't exactly "clean". But that's the fault of #2982755: Random failure in SchemaTest::testSchemaChangePrimaryKey with order of composite primary key not this test.

If you look at the results from #6 on PgSQL, we see 2 failures:
https://www.drupal.org/pift-ci-job/1461812

Testing Drupal\Tests\views\Functional\Plugin\DisplayFeedTranslationTest
F                                                                   1 / 1 (100%)

Time: 28.15 seconds, Memory: 4.00MB

There was 1 failure:

1) Drupal\Tests\views\Functional\Plugin\DisplayFeedTranslationTest::testFeedFieldOutput
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Brasilian Portugues translation'
+'Spanish translation'

(that was my bug, now fixed)
...

There was 1 failure:

1) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "composite_primary_key" (array('test_field', 'other_test_field'), array('test_field_renamed', 'other_test_field'))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'test_field_renamed'
-    1 => 'other_test_field'
+    0 => 'other_test_field'
+    1 => 'test_field_renamed'

If you look at #14, we only see the later:
https://www.drupal.org/pift-ci-job/1462123

1) Drupal\KernelTests\Core\Database\SchemaTest::testSchemaChangePrimaryKey with data set "composite_primary_key" (array('test_field', 'other_test_field'), array('test_field_renamed', 'other_test_field'))
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'test_field_renamed'
-    1 => 'other_test_field'
+    0 => 'other_test_field'
+    1 => 'test_field_renamed'
...

My ordering bug is gone from the test results. Fixing SchemaTest::testSchemaChangePrimaryKey() is the business of #2982755, not this issue.

So, +1 to RTBC. ;)

Thanks!
-Derek

p.s. Updating remaining tasks to point to the right patches and cross off now complete steps.

#3092571: Fix RSS test view (and related tests) to not put links in the title element is now RTBC, and @Lendude wrote:

I also think we need to lands this before #3092125 to not introduce new @todo's that we can avoid

So here's a test-only patch for after #3092571 lands. I think we've got enough bot results above to prove the revert is a bad idea, so I'm just going to focus on what we should commit -- the new test.

This will fail until #3092571 is in, hence "DO-NOT-TEST-YET". ;)

Thanks,
-Derek

p.s. I had requeued #14 on PgSQL and now it's all green. The luck of the draw! ;) But yay, that looks better for here.

Yeah, sorry about that.

Since you also committed #3092571: Fix RSS test view (and related tests) to not put links in the title element we needed #22 not #14.

I was trying to attach a hotfix patch, but you already reverted. So I queued #22 instead.

NR for #22.

Might fail on PgSQL due to #2982755: Random failure in SchemaTest::testSchemaChangePrimaryKey with order of composite primary key

Indeed, #22 is passing on PgSQL, MySQL + SQLite. RTBC. Committing this would help what I'm trying to do to finish off #2673980: RSS view with fields give wrong URL from path field once and for all.

Sorry for the confusion on which patch to commit. I wasn't expecting both of these to land within 5 minutes of each other. ;) Apologies I didn't put a big warning in the summary about the interdependence on #3092571: Fix RSS test view (and related tests) to not put links in the title element and which patch to commit depending on what landed first.

I'm still calibrating when to combine things into 1 big patch vs. when to try to keep the scope small for discrete changes.

Thanks/sorry/onward,
-Derek

Yay, thanks!