Problem/Motivation

Some requests in tests are broken because of a missing leading slash in front of user.

    if ($clean_urls) {
      $request_path = $base_path ? $base_path . '/user' : 'user';
    }

Proposed resolution

  • Add a leading slash

OR

  • Drop the ternary and build the request path by concatenating

Remaining tasks

Review Patch
Commit

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#30 missing_leading_slash-2548679-30.patch1.74 KBtduong
#30 missing_leading_slash-2548679-30-test_only.patch1.13 KBtduong
#30 interdiff-2548679-21-30.txt1.25 KBtduong
#30 interdiff-2548679-21-28.txt945 bytestduong
#28 missing_leading_slash-2548679-28.patch1.74 KBLKS90
#28 missing_leading_slash-2548679-28-TEST-ONLY.patch1.13 KBLKS90
#23 missing_leading_slash-2548679-21.patch1.79 KBLKS90
#23 missing_leading_slash-2548679-21-TEST-ONLY.patch1.17 KBLKS90
#20 missing_leading_slash-2548679-20-TEST-ONLY.patch1.21 KBLKS90
#19 missing_leading_slash-2548679-19_TEST-ONLY.patch1.19 KBLKS90
#16 missing_leading_slash-2548679-15.patch1.86 KBLKS90
#16 missing_leading_slash-2548679-15_TEST-ONLY.patch1.29 KBLKS90
#15 interdiff_14-15.txt1.19 KBLKS90
#14 missing_leading_slash-2548679-14.patch1.74 KBLKS90
#14 missing_leading_slash-2548679-14-TEST-ONLY.patch1.13 KBLKS90
#14 interdiff_12-14.txt652 bytesLKS90
#12 missing_leading_slash-2548679-12.patch1.67 KBLKS90
#12 interdiff_8-12.txt694 bytesLKS90
#8 interdiff_6-8.txt489 bytesLKS90
#8 missing_leading_slash-2548679-8.patch1.67 KBLKS90
#6 interdiff_5-6.txt475 bytesLKS90
#6 missing_leading_slash-2548679-6.patch1.67 KBLKS90
#5 missing_leading_slash-2548679-5.patch1.66 KBLKS90
#5 missing_leading_slash-2548679-5-TEST-ONLY.patch1.05 KBLKS90
#4 missing_leading_slash-2548679-3.patch631 bytesLKS90

Comments

LKS90 created an issue. See original summary.

LKS90’s picture

LKS90 commented
Issue summary: View changes
LKS90’s picture

LKS90 commented
Issue summary: View changes
Status: Active » Needs review

Here is a patch that drops the ternary and builds the request_path by concatenation.

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-3.patch631 bytes

The patch... working on a test now.

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-5-TEST-ONLY.patch1.05 KB
new missing_leading_slash-2548679-5.patch1.66 KB

Here is a test for the bug.

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-6.patch1.67 KB
new interdiff_5-6.txt475 bytes 
+++ b/core/modules/simpletest/src/Tests/RequestCreationTest.php
@@ -0,0 +1,37 @@
+ * Contains \Drupal\simpletest\Tests\BrowserTest.

Fixed the copy/paste description, no TEST-ONLY patch, since functionally nothing changes, I provide an interdiff though.

sasanikolic’s picture

sasanikolic commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/simpletest/src/Tests/RequestCreationTest.php
@@ -0,0 +1,37 @@
+
+

    Too many empty lines. 

  2. +++ b/core/modules/simpletest/src/Tests/RequestCreationTest.php
@@ -0,0 +1,37 @@
+    \Drupal::logger('Simpletest')->alert('Test message.');

    Is this supposed to be in the patch or it is some leftover?

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review
StatusFileSize
new missing_leading_slash-2548679-8.patch1.67 KB
new interdiff_6-8.txt489 bytes

Removed the two empty lines from the test.

Is this supposed to be in the patch or it is some leftover?

That's the function name and the whole point of the test. The test navigates to that log entry on lines 33 and 34, then we assert the Location of that log entry (it used to be missing a / in front of user). In the old test-only version you can see the result of the test when the bug is not fixed. Edit: The jenkins build failed as expected, I don't know what the testbot is doing once again...

LKS90 queued 5: missing_leading_slash-2548679-5-TEST-ONLY.patch for re-testing.

LKS90’s picture

LKS90 commented

Looks like it only fails on Jenkins builds, I don't know why the testbot doesn't fail, only explanation would be missing clean_urls?

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/simpletest/src/Tests/RequestCreationTest.php
@@ -0,0 +1,35 @@
+    $this->drupalGet('/admin/reports/dblog');

MH, I thought we don't support slashes in front for paths ... so do we really want to add this here?

LKS90’s picture

LKS90 commented
StatusFileSize
new interdiff_8-12.txt694 bytes
new missing_leading_slash-2548679-12.patch1.67 KB

Nope, that's just a copy/paste mistake, thanks for noticing.

While I have you here: Do you think the fail on the Jenkins build is enough?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Side note: We noticed this on travis with drupal_ti, which uses http://localhost:8080, and that resulted in a URL that looked like this: http://localhost:8080user, which throws an exception, while http://localhostuser would just result in a link that points into nothing.

LKS90’s picture

LKS90 commented
StatusFileSize
new interdiff_12-14.txt652 bytes
new missing_leading_slash-2548679-14-TEST-ONLY.patch1.13 KB
new missing_leading_slash-2548679-14.patch1.74 KB

Thanks for the inspiration, here is a test that should also fail for the testbot.
Interdiff, patch and test-only-patch are attached.

LKS90’s picture

LKS90 commented
StatusFileSize
new interdiff_14-15.txt1.19 KB

Here is another try. Note: I left the debug in the TEST-ONLY.patch for a reason :).

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-15_TEST-ONLY.patch1.29 KB
new missing_leading_slash-2548679-15.patch1.86 KB

Worked as expected on simplytest.me, I hope it fails for the testbot as well.

The last submitted patch, 16: missing_leading_slash-2548679-15_TEST-ONLY.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 16: missing_leading_slash-2548679-15.patch, failed testing.

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review
StatusFileSize
new missing_leading_slash-2548679-19_TEST-ONLY.patch1.19 KB

Another try at the test-only patch, with debug.

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-20-TEST-ONLY.patch1.21 KB

I'm getting desperate, so here is a patch with a debug of the report page so I can take a look at the link on the testbot...

Status: Needs review » Needs work

The last submitted patch, 20: missing_leading_slash-2548679-20-TEST-ONLY.patch, failed testing.

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review

Here is a cleaned up version ready for commit. The test-only patch only fails on jenkins, it's impossible to fail on the testbot.

LKS90’s picture

LKS90 commented
StatusFileSize
new missing_leading_slash-2548679-21-TEST-ONLY.patch1.17 KB
new missing_leading_slash-2548679-21.patch1.79 KB

The patches...

Status: Needs review » Needs work

The last submitted patch, 23: missing_leading_slash-2548679-21.patch, failed testing.

Status: Needs work » Needs review

LKS90 queued 23: missing_leading_slash-2548679-21.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 23: missing_leading_slash-2548679-21.patch, failed testing.

mbovan’s picture

mbovan commented 
+++ b/core/modules/simpletest/src/Tests/RequestCreationTest.php
@@ -0,0 +1,38 @@
+use Symfony\Component\HttpFoundation\Request;

Not used, can be removed.

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review
StatusFileSize
new missing_leading_slash-2548679-28-TEST-ONLY.patch1.13 KB
new missing_leading_slash-2548679-28.patch1.74 KB

Removed the unused class from the usages, also fixed the patch, the last one didn't have the / in front of users... :D
Sorry, no interdiff, I cleaned up my commit history after it got a little dirty and lost the single change there.

As usual, will only fail on Jenkins.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tduong’s picture

tduong commented
StatusFileSize
new interdiff-2548679-21-28.txt945 bytes
new interdiff-2548679-21-30.txt1.25 KB
new missing_leading_slash-2548679-30-test_only.patch1.13 KB
new missing_leading_slash-2548679-30.patch1.74 KB

Switched branch to 8.1.x and rerolled. Interdiff against #21 for patch #28 and #30.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

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

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone commented
Component: simpletest.module » phpunit

Triaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.

This looks like it a Phpunit issue, changing component.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joachim’s picture

joachim commented
Status: Needs review » Closed (outdated)

I don't see anything about clean URLs in BrowserTestBase, which AFAIK is the PHPUnit equivalent of WebTestBase -- or indeed in all of core/tests/Drupal.

I'm going to say this is probably outdated.