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.
Blocks #606840: Enable internal page cache by default.
This fixes the 15 failures o/t parent issue in:
Drupal\config\Tests\ConfigDependencyWebTest
(4)Drupal\config\Tests\ConfigEntityFormOverrideTest
(2)Drupal\config\Tests\ConfigEntityTest
(9)
Comment | File | Size | Author |
---|---|---|---|
#11 | config_test_routes-2460677-11.patch | 7.65 KB | Wim Leers |
Comments
Comment #1
Wim LeersComment #2
Wim LeersComment #3
Wim LeersComment #4
Fabianx CreditAttribution: Fabianx for Drupal Association commentedRTBC, Config Entity Test s should not rely on data being available as anoymous user.
Comment #5
BerdirCan we have a quick explanation *why* it fixes those tests? What does page cache have to do with it?
Comment #6
Wim LeersSure.
The
config_test
routes have_access: 'TRUE'
set in their requirements. This means any user can access them, including the anonymous user. Since the tests currently use the anonymous user, the pages end up in the page cache.The better fix would be to replace
_access: 'TRUE'
with an actual permission check. But that also means creating a custom permission, setting up a role with the right permissions, creating a user, etc.Hrm, actually, no, the root user is user 1, which has all permissions by default. So it'd only require setting up a custom permissions for this test-only module. Which still seems overkill. But if somebody has a good reason to prefer that, then I'd be okay with that :) Feel free to mark NW if you think we should do that.
Comment #7
Fabianx CreditAttribution: Fabianx for Drupal Association commentedIn the end however the we still would need to login. It is just that the tests would fail with page cache off, too and anonymous user, which I don't think is needed to fix this.
Comment #8
alexpottIt's not needed to fix this - but it is need to prevent us adding any new usages of these routes without logging in in the meantime. Let's replace
_access: 'TRUE'
with a perm.Comment #9
Wim LeersDone.
First, I added a new permission. Then I realized we can simply reuse
'administer site configuration'
, that's sufficient for this test AFAICT.Comment #11
Wim LeersThree additional tests now had failures, because I updated all
config_test
routes to require a permission.Comment #12
Fabianx CreditAttribution: Fabianx for Drupal Association commentedLooks good now :)
Comment #13
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed a739166 and pushed to 8.0.x. Thanks!