Follow up from #2982149: Routing: Convert system functional tests to phpunit

First apply patch from #4 then work on this.

At least this needs to be fixed:

$this->drupalGet('system-test/get-destination', ['query' => ['destination' => $test_case['input']]]);
      $this->assertIdentical($test_case['output'], $this->getRawContent(), $test_case['message']);
      // Test $_REQUEST['destination'].
      $post_output = $this->drupalPost('system-test/request-destination', '*', ['destination' => $test_case['input']]);
      $this->assertIdentical($test_case['output'], $post_output, $test_case['message']);
CommentFileSizeAuthor
#21 interdiff.txt1.75 KBmile23
#21 2983970_21.patch2.86 KBmile23
#16 interdiff-2983970-14-16.txt811 bytesApacheEx
#16 2983970-16.patch2.08 KBApacheEx
#14 2983970-14.patch2.09 KBApacheEx
#12 interdiff-2983970-10-12.txt980 bytesApacheEx
#12 2983970-12.patch3.04 KBApacheEx
#10 2983970-10.patch2.47 KBApacheEx
#8 2983970-8.patch2.33 KBApacheEx
#4 2983970-4.patch754 bytesscuba_fly

Comments

scuba_fly created an issue. See original summary.

scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
commented
Issue summary: View changes
mitsuroseba’s picture

mitsuroseba commented
Assigned: Unassigned » mitsuroseba
scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
commented
StatusFileSize
new 2983970-4.patch754 bytes

Please note that I'm splitting up #2982149: Routing: Convert system functional tests to phpunit
You can use the attached patch as starting point so this can be sumitted without waiting for #2982149

scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
commented
Issue summary: View changes
scuba_fly’s picture

scuba_fly
Location Netherlands, Vinkeveen
commented

Also note the files are not in the right location in the patch #4 They need to be in System > Test > src >Routing

mitsuroseba’s picture

mitsuroseba commented

Okay.

ApacheEx’s picture

ApacheEx commented
Assigned: mitsuroseba » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2983970-8.patch2.33 KB

Here it is.

Status: Needs review » Needs work

The last submitted patch, 8: 2983970-8.patch, failed testing. View results

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2983970-10.patch2.47 KB

Recreated patch. Should be better now.

Status: Needs review » Needs work

The last submitted patch, 10: 2983970-10.patch, failed testing. View results

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2983970-12.patch3.04 KB
new interdiff-2983970-10-12.txt980 bytes

Hm, locally all tests are passed.
Let's try once more with minimum improvements.

Status: Needs review » Needs work

The last submitted patch, 12: 2983970-12.patch, failed testing. View results

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2983970-14.patch2.09 KB

let's try once more.

Status: Needs review » Needs work

The last submitted patch, 14: 2983970-14.patch, failed testing. View results

ApacheEx’s picture

ApacheEx commented
Status: Needs work » Needs review
StatusFileSize
new 2983970-16.patch2.08 KB
new interdiff-2983970-14-16.txt811 bytes

What if that's the reason.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs review » Needs work

Looking at the fail for #2982149: Routing: Convert system functional tests to phpunit for the other router test, we should really include that in this conversion too, I think.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Reviewed & tested by the community

I take back #17

I took a look at converting the other routing test and that isn't easy at all. So lets keep this to a one test scope, or else it would drown in the other test.

This looks good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 16: 2983970-16.patch, failed testing. View results

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review
StatusFileSize
new 2983970_21.patch2.86 KB
new interdiff.txt1.75 KB

Failed due to AssertLegacyTrait::getRawContent() being deprecated.

Status: Needs review » Needs work

The last submitted patch, 21: 2983970_21.patch, failed testing. View results

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review

Testbot hiccup.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/system/tests/src/Functional/Routing/DestinationTest.php
@@ -61,10 +64,12 @@ public function testDestination() {
       // Test $_REQUEST['destination'].
-      $post_output = $this->drupalPost('system-test/request-destination', '*', ['destination' => $test_case['input']]);
-      $this->assertIdentical($test_case['output'], $post_output, $test_case['message']);
+      $post_output = $http_client->request('POST', $this->buildUrl('system-test/request-destination'), [
+        'form_params' => ['destination' => $test_case['input']],
+      ]);

I do like that it is easier to understand what is exactly in the HTTP response!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5fee025533 to 8.7.x and 715786dd1e to 8.6.x. Thanks!

  • alexpott committed 5fee025 on 8.7.x 
    Issue #2983970 by ApacheEx, Mile23, scuba_fly: Convert system functional...

  • alexpott committed 715786d on 8.6.x 
    Issue #2983970 by ApacheEx, Mile23, scuba_fly: Convert system functional...

Status: Fixed » Closed (fixed)

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