Problem/Motivation

In #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait we removed a legacy call to drupal_flush_all_caches() but this had the unintended impact of causing \Drupal\Tests\node\Functional\Views\FrontPageTest to fail. This is because it uses \Drupal\views\Tests\AssertViewsCacheTagsTrait::assertViewsCacheTags() which is primarily designed to work in kernel tests. It does $request = new Request(); whereas it should be $request = Request::createFromGlobals(); so that it uses the $_SERVER settings done in \Drupal\Core\Test\FunctionalTestSetupTrait::setupBaseUrl(). Because it pushes an incorrect request onto the request stack, if the RequestContext is not yet initialised, the url generator with not be able to produce absolute urls.

Proposed resolution

Remove Url::fromRoute('<front>')->setAbsolute()->toString();.

Contrib / custom tests might need to be fixed but if so they are pushing a request to the request stack with incorrect info.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

@todo

Issue fork drupal-3207896

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:

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Version: 10.0.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. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

andypost’s picture

Issue summary: View changes
Status: Active » Needs work

andypost’s picture

Status: Needs work » Needs review

Looks it have no effect now, so not sure about change record

bramdriesen’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record

Less code, and a todo removed 🤩 yay!

Code wise this looks good, and all tests are green. Marking as RTBC for that. Removed the Needs CR tag for now as I also don't feel this really needs it. But feel free to re-add it.

  • longwave committed fd525a15 on 10.4.x
    Issue #3207896 by andypost, alexpott: Remove URL generator priming in \...

  • longwave committed 307c72ca on 11.0.x
    Issue #3207896 by andypost, alexpott: Remove URL generator priming in \...

  • longwave committed 4acd1d09 on 11.x
    Issue #3207896 by andypost, alexpott: Remove URL generator priming in \...
longwave’s picture

Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 10.4.x but not backported to 10.3.x to avoid surprising contrib/custom tests while we are in RC just in case they were doing something wrong.

Committed and pushed 4acd1d0902 to 11.x and 307c72cad5 to 11.0.x and fd525a15c5 to 10.4.x. Thanks!

Status: Fixed » Closed (fixed)

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