As discovered by @Wim Leers in #2863607: Convert WebTestBaseTests of BigPipe to BrowserTestBase \Drupal\Tests\BrowserTestBase::checkForMetaRefresh is currently case sensitive and only allows a capitalised meta[http-equiv="Refresh"].

Since \Symfony\Component\HttpFoundation\RedirectResponse::__construct uses a lowercase meta[http-equiv="refresh"] we need to check for both.

CommentFileSizeAuthor
#5 2905818-5.patch3.97 KBWim Leers
#5 interdiff.txt2.43 KBWim Leers
#4 2905818-4.patch3.54 KBWim Leers
#4 2905818-4-test-only-FAIL.patch2.75 KBWim Leers
#2 2905818-2.patch3.56 KBLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Active » Needs review
Related issues: +#2863607: Convert WebTestBaseTests of BigPipe to BrowserTestBase
FileSize
2905818-2.patch3.56 KB

Here is the fix from #2863607: Convert WebTestBaseTests of BigPipe to BrowserTestBase (credit @Wim Leers), plus a test.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. Should we upload a pass fail patches as well?

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
2905818-4-test-only-FAIL.patch2.75 KB
2905818-4.patch3.54 KB

Yes, I was just doing that.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +blocker
FileSize
interdiff.txt2.43 KB
2905818-5.patch3.97 KB 
  1. +++ b/core/modules/system/tests/modules/test_page_test/src/Controller/Test.php
@@ -1,6 +1,8 @@
 namespace Drupal\test_page_test\Controller;
+use Drupal\Core\Url;
+use Symfony\Component\HttpFoundation\RedirectResponse;

    Nit: whitespace. 

  2. +++ b/core/modules/system/tests/modules/test_page_test/src/Controller/Test.php
@@ -108,4 +110,11 @@ public function renderPipeInLink() {
+  /**
+   * Loads a page that does a redirect.
+   */
+  public function metaRefresh() {
+    return new RedirectResponse(Url::fromRoute('test_page_test.test_page', [], ['absolute' => TRUE])->toString(), 302);
+  }

    This is a bit hard to understand. I think a comment could help here. 

  3. +++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
@@ -605,4 +605,22 @@ public function testLocalTimeZone() {
+   * Tests redirecting using the checkForMetaRefresh method.

    Nit: Let's make this comment consistent with others in this test class.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

Thanks @Wim Leers. Still RTBC.

The last submitted patch, 4: 2905818-4-test-only-FAIL.patch, failed testing. View results

  • larowlan committed aae0baf on 8.5.x 
    Issue #2905818 by Wim Leers, Lendude: Make sure \Drupal\Tests\...

  • larowlan committed 087c5cd on 8.4.x 
    Issue #2905818 by Wim Leers, Lendude: Make sure \Drupal\Tests\...

larowlan credited larowlan.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

Committed as aae0baf and pushed to 8.5.x.

Cherry-picked as 087c5cd and pushed to 8.4.x.

Thanks for splitting this one off.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

That was … surprisingly fast! Thanks, @Lendude + @larowlan!

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented

Blink your eyes and it's in already. Nice work, thanks all!

Status: Fixed » Closed (fixed)

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