Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Blocks #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!), causes 6 test failures there.
Proposed resolution
Specify proper cacheability for the session_test
module's routes/controllers: Most of the routes should not be cached, and the session_test.id_from_cookie
route needs to have the cookie ID as a context.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#1 | 2512734-1.patch | 3.47 KB | Wim Leers |
Comments
Comment #1
Wim LeersComment #2
BerdirLooks fine, just changes of tests, we have "test coverage" in #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!), but we can commit this separately to keep that patch there smaller.
Comment #3
dawehnerWell the patch doesn't look like that. We seem to opt out a lot here
Comment #4
Wim LeersIndeed. Because that's the proper course of action here, since these routes/controllers are all just printing session IDs.
Comment #5
dawehnerWell, should we not specify the session ID as cache context then? I mean its about test coverage and giving good examples for people.
Comment #6
xjmDiscussed with @Wim Leers, @dawehner, and @Berdir. The summary confused me a little, but all we are doing here is specifying that most of the routes in the session test should not be cached, and that the
session_test.id_from_cookie
route needs to have the cookie ID as a context.We discussed @dawehner's suggestion, but that would essentially just add test coverage that the cache metadata bubbling works, which is not the purpose of the test.
This issue only changes test code, so it is unfrozen during the beta per https://www.drupal.org/core/beta-changes, and it is a prioritized change as a bugfix and blocker for #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!). Committed and pushed to 8.0.x. Thanks!
Comment #8
xjm