Problem/Motivation

The test controller added in #2505989: Controllers render caching at the top level and setting a custom page title lose the title on render cache hits doesn't properly set #cache[keys] to the returned render array. PageTitleTest::testRoutingTitle() passes because it tests the same output twice.

Proposed resolution

Use different #cache[keys] for '/test-page-cached-controller' and '/test-page-cached-controller-safe'.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#3 interdiff.txt1.59 KBolli
#3 2530908-3.patch2.37 KBolli
#2 interdiff.txt976 bytesolli
#2 2530908-2.patch1.63 KBolli
page-title-cache-test.patch696 bytesolli

Comments

Status: Needs review » Needs work

The last submitted patch, page-title-cache-test.patch, failed testing.

olli’s picture

olli commented
Status: Needs work » Needs review
StatusFileSize
new 2530908-2.patch1.63 KB
new interdiff.txt976 bytes

This fixes the test.

olli’s picture

olli commented
StatusFileSize
new 2530908-3.patch2.37 KB
new interdiff.txt1.59 KB 
+++ b/core/modules/system/src/Tests/System/PageTitleTest.php
@@ -151,10 +151,10 @@ public function testRoutingTitle() {
+    $this->assertTitle('<span>Cached title</span> | Drupal');
     $this->assertText(SafeMarkup::checkPlain('<span>Cached title</span>'));

I think this assertText() matches the <title> element so replacing them with assertRaw().

fabianx’s picture

fabianx commented
Issue tags: +Needs issue summary update

Can we have some issue summary update?

As it is tests only, I don't think we need to test the tests ...

olli’s picture

olli commented
Issue summary: View changes
Issue tags: -Needs issue summary update

Updated issue summary.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC, great catch!

alexpott’s picture

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

Nice work indeed. Thanks @olli for fixing my mistake. Committed e54aa5e and pushed to 8.0.x. Thanks!

  • alexpott committed e54aa5e on 8.0.x 
    Issue #2530908 by olli: Caching problem in PageTitleTest

Status: Fixed » Closed (fixed)

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