Problem/Motivation
In #540008: Add a container parameter that can remove the special behavior of UID#1 an approach was taken where we can simply flag tests that are failing if we turn off user 1's super user powers, so that they can be taken care of in a followup. This issue is to collect all of these followups.
The goal is to have no tests in Drupal core that rely on UID1's special privileges so that we:
- Know these tests are correctly assigning the necessary permissions to run
- Can turn off the super user access policy in D11, knowing it won't break core
- Can remove the super user access policy in D12, providing an admin account recovery tool to replace it
Steps to reproduce
Go into any of the tests flagged with:
/**
* {@inheritdoc}
*
* @todo Remove and fix test to not rely on super user.
* @see https://www.drupal.org/project/drupal/issues/3437620
*/
And:
- Remove the code below that sets the usesSuperUserAccessPolicy to TRUE.
- Run the test to see which test methods are failing
Proposed resolution
Assign the right permissions to make the test go green without the super user access policy. Those few tests that specifically test said policy can obviously stay, but will be removed along with the policy in D12.
Remaining tasks
- core/tests/Drupal/
- FunctionalTests/Theme/ClaroTest.php
- KernelTests/Core/Entity/RouteProviderTest.php
- KernelTests/Core/Render/RenderCacheTest.php
Issue fork drupal-3439925
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:
- 3439925-fix-core-tests
changes, plain diff MR !7575
Comments
Comment #4
simonbaeseAfter aligning with @kristiaanvandeneynde the
RenderCacheTestshould be removed entirely. See related issue #3436395: UserRolesCacheContext can lead to poisoned cache returns for user 1.Comment #5
smustgrave commentedRemoving the tests appear to be scope creep though. Believe the test removal should be done in the issue you mentioned.
Comment #6
simonbaeseThen the mentioned issue should be solved first, because we can not fix this issue if not removing the test.
Comment #7
smustgrave commentedSo actually could just leave that one test as is (revert in MR).
Put a todo in it to #3436395: UserRolesCacheContext can lead to poisoned cache returns for user 1 that it will be removed
Updated #3436395: UserRolesCacheContext can lead to poisoned cache returns for user 1 to make sure they remove the test there.
Then the other 2 can move forward.
Comment #8
simonbaese@smustgrave I reverted the deletion of the
RenderCacheTest. Can you please make respective comments and add todos in the other issues. I am having a hard time understanding your last comment.Comment #9
smustgrave commentedAdded todo to the test we aren't updating here and updated IS in #3436395: UserRolesCacheContext can lead to poisoned cache returns for user 1
Comment #10
alexpottCommitted and pushed cbad32ab34 to 11.x and 097371801c to 10.3.x. Thanks!