Problem/Motivation

While investigating a strange failure on #2857157: Implement registration after guest checkout I was getting some weird behaviour when trying to recreate and fix the test failures:

$this->assertUrl('user/3/edit'); was causing:

'Current page is "/subdirectory/user/3/edit", but "/user/3/edit" expected.'

When I first tried to run tests locally with drupal in a subdirectory (i.e. /var/www/htdocs/subdirectory with php ./core/scripts/run-tests.sh --url http://test.local/subdirectory ...) then a lot of url assertions failed and only after replicating the CI environment by moving drupal to /var/www/html and creating the symlink (ln -s /var/www/html /var/www/html/subdirectory) could I get all tests to pass except the one in question.

I was able to 'fix' the test by changing $this->drupalGet($this->product->toUrl()->toString()); to $this->drupalGet($this->product->toUrl());.

I think this weird fix works because the url generator used by $this->product->toUrl()->toString() is correctly adding '/subdirectory' but $this->drupalGet() is not. This kicks the test environment to / but still works because although run-tests.sh is called with i.e. --url "http://php-apache-jenkins-drupal8-contrib-patches-32847/subdirectory" and starts off at /subdirectory, it is just a symlink of the webroot so all paths without /subdirectory will always work still.

As unlikely as this first seems, I think an explanation for no other tests spotting it could be because nobody has committed a test that passes a Url->toString() to $this->drupalGet(), always a Url object (i.e. $this->drupalGet(Url::fromRoute('<front>'));) or a hard coded uri (i.e. $this->drupalGet('admin/content');) and anyone who even notices the /subdirectory in URLs assumes that drupalGet() is automatically adding '/subdirectory' and assertUrl() is handling it like a uri.

Proposed resolution

Strip $base_path from the path given to "drupalGet" when this path is a string and the "url_generator" service is available.
Note that when a string is given and the service is not available, the function "getAbsoluteUrl" already strips the base path.

Remaining tasks

  1. Confirm that tests fail with drupal installed in a subdirectory
  2. Review proposed resolution
  3. Do fixes
  4. Fix tests broken by fixes
  5. Open a discussion in the relevant project about removing the symlink from drupal CI and installing to /var/www/htdocs/subdirectory (there is probably a very good reason for the symlink but i assume the whole point of it was to test everything works when drupal is in a subdirectory?)

Issue fork drupal-2986962

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Sut3kh created an issue. See original summary.

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.

krzysztof domański’s picture

krzysztof domański
Location Poland
commented
Version: 8.6.x-dev » 8.7.x-dev

Similar problem #2527628: Deleting a block does not redirect back to the correct theme.

Current page is "/subdirectory/admin/structure/block", but "/admin/structure/block" expected.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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 an issue for the Phpunit component.

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

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

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.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.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ppseftogiannis made their first commit to this issue’s fork.

ppseftogiannis opened merge request !1950

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now 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.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now 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.

bhanu951’s picture

bhanu951 commented
Assigned: Unassigned » bhanu951
Issue tags: +Needs reroll
bhanu951’s picture

bhanu951 commented
Assigned: bhanu951 » Unassigned
Issue tags: -Needs reroll

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

fjgarlin’s picture

fjgarlin commented
Related issues: +#3375827: Investigate tests failures

We are running into this issue when trying to convert DrupalCI to GitlabCI.

See the slack message: https://drupal.slack.com/archives/C223PR743/p1692017659269539
Quoting here for those not in slack:

We are trying to recreate DrupalCI with GitlabCI for Drupal core.
* Project: https://www.drupal.org/project/gitlab_ci_testbed_for_drupal_core
* Issue: https://www.drupal.org/project/gitlab_ci_testbed_for_drupal_core/issues/...

Most tests do pass: phpunit, kernel, nightwatch… but some of the Functional tests fail, and in most cases, the reason is a duplicated subdirectory/subdirectory in the URL, leading to a 404. Having and testing Drupal in a subdirectory is intentional, but I’m not sure where that duplication comes from.

I’ve tested some variations of the htaccess file provided by Drupal core (MR here: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/m... and you can see the commit history for the MR).

You can see an example of a failed job here: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/j.... You can download the artifacts and see the logs and see that in most cases, the reason is due to a path with subdirectory/subdirectory. These same tests in DrupalCI do work.

ie: InlineBlockPrivateFilesTest produces simpletest/browser_output/Drupal_Tests_layout_builder_FunctionalJavascript_InlineBlockPrivateFilesTest-1-59609102.html and if you click on Next links, you’ll see the duplication if the 404 page where the test fails.

I'm still not sure why it fails in GitlabCI and _not_ in DrupalCI, but the situation is the same as the one presented in this issue.
If I find a solution to it, I'll also report here, I just wanted to "link" these two issues as the fix might help one another.

fjgarlin’s picture

fjgarlin commented

I made a change that I think fixes the issue, but I find it hard to write a test that will fail...
MRs:
* 9.4.x: https://git.drupalcode.org/project/drupal/-/merge_requests/1950/diffs
* 11.x: https://git.drupalcode.org/project/drupal/-/merge_requests/4602/diffs

So far, DrupalCI has not caught this, even though there are multiple issues related to this.

See core:
* #3060842: Improve documentation on the working of Url::fromUserInput
* #2582295: Confirmation cancel links are incorrect if installed in a subdirectory
* #2990664: Media library does not work when Drupal is installed into a sub-directory

And contrib:
* #3320228: IMCE UI generates links that are not the same as those generated by Drupal Core
* #2899166: WORKAROUND: Issue #2582295: Confirmation cancel links are incorrect if installed in a subdirectory
* #3046182: Work around Core bug: 404 on confirmation cancel in subdirectory installs

For this fix, while using GitlabCI for core, we can see the before and after (open the link and then click on the right arrow in the "Downstream" job to expand the child pipeline):
* Before: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/p...
* After: https://git.drupalcode.org/project/gitlab_ci_testbed_for_drupal_core/-/p...

“Functional Javascript” errors go from 8 to 3.
“Functional” ones from 5 to 0.

The related issue where those tests are being made is: #3375827: Investigate tests failures

fjgarlin’s picture

fjgarlin commented
Status: Active » Needs review

fjgarlin opened merge request !4602

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Fix \Drupal\Tests\BrowserTestBase::drupalGet() so it does not lose the base path if given a uri

Could the issue summary be updated to match the new solution please.

I've been bit by this bug numerous times too and had to do some weird workarounds. Usually the failure only happened on drupalCi but would pass locally because of this issue.

Not sure if any test coverage is needed.

fjgarlin’s picture

fjgarlin commented
Issue summary: View changes
fjgarlin’s picture

fjgarlin commented
Status: Needs work » Needs review

Updated the issue description to match the solution proposed in the MR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update +Needs Review Queue Initiative, +Bug Smash Initiative

Thanks! Think this is ready.

  • catch committed feab11e4 on 11.x 
    Issue #2986962 by fjgarlin, ppseftogiannis, smustgrave, Sut3kh:...

  • catch committed 4ec03b57 on 10.1.x 
    Issue #2986962 by fjgarlin, ppseftogiannis, smustgrave, Sut3kh:...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs followup

It's probably theoretically possible to test the trait with different values for $base_path, however this is going to be implicitly tested by every gitlab CI run for the forseeable future, so I think that could be a follow-up.

Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!

catch’s picture

catch
he/him
Primary language English
commented
quietone’s picture

quietone commented

This is tagged for a followup. Is that #5 of the remaining tasks? Does anyone know if that has been created or is no longer needed?

catch’s picture

catch
he/him
Primary language English
commented

Sorry the follow-up would be to add additional, explicit, test coverage to UiHelperTrait to ensure that it works with different values for $base_url. However because we have implicit test coverage of this on every test run, I'm wasn't sure whether it's really worth adding tests for the test helper in this case.

quietone’s picture

quietone commented
Issue tags: -Needs followup

Seems better to make one than to forget. Here it is #3386051: Add tests of $base_url in UiHelperTrait

Removing tag

Status: Fixed » Closed (fixed)

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