Fix System tests that rely on UID1's super user behavior

Created on 10 April 2024, 3 months ago
Updated 30 April 2024, about 2 months ago

Problem/Motivation

In ๐Ÿ“Œ Add a container parameter that can remove the special behavior of UID#1 Fixed 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:

  1. Know these tests are correctly assigning the necessary permissions to run
  2. Can turn off the super user access policy in D11, knowing it won't break core
  3. 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:

  1. Remove the code below that sets the usesSuperUserAccessPolicy to TRUE.
  2. 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

  1. core/modules/system/tests/src/
    • Functional/Bootstrap/DrupalMessengerServiceTest.php
    • Functional/Entity/EntityReferenceFieldCreationTest.php
    • Functional/File/FileSaveHtaccessLoggingTest.php
    • Functional/Menu/LocalTasksTest.php
    • Functional/Module/ClassLoaderTest.php
    • Functional/Module/GenericModuleTestBase.php
    • Functional/System/DateFormatsLockedTest.php
    • Functional/Theme/MaintenanceThemeUpdateRegistryTest.php
    • Functional/UpdateSystem/UpdateScriptTest.php
    • Kernel/DateFormatAccessControlHandlerTest.php
    • Kernel/Entity/EntityReferenceSelection/EntityReferenceSelectionAccessTest.php
    • Kernel/MenuAccessControlHandlerTest.php
๐Ÿ“Œ Task
Status

Fixed

Version

10.3 โœจ

Component
Systemย  โ†’

Last updated about 9 hours ago

No maintainer
Created by

๐Ÿ‡ฌ๐Ÿ‡ทGreece vensires

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @vensires
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia pradhumanjainOSL

    pradhumanjain2311 โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    2 months ago
    Total: 990s
    #150345
  • First commit to issue fork.
  • Pipeline finished with Failed
    2 months ago
    Total: 178s
    #153162
  • Pipeline finished with Failed
    2 months ago
    #153172
  • Pipeline finished with Failed
    2 months ago
    Total: 1102s
    #153211
  • Pipeline finished with Success
    2 months ago
    Total: 990s
    #153340
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Left comments on MR.

  • Pipeline finished with Failed
    2 months ago
    Total: 1109s
    #153446
  • Pipeline finished with Success
    2 months ago
    Total: 993s
    #153475
  • Pipeline finished with Success
    2 months ago
    Total: 1698s
    #153898
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Left more feedback.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Also @pradhumanjain2311 why did you just remove $usesSuperUserAccessPolicy but not try and fix any of the tests? Not saving credit for that. Please if going to work on these try and fix them.

  • Pipeline finished with Failed
    2 months ago
    Total: 175s
    #155990
  • Pipeline finished with Success
    2 months ago
    Total: 992s
    #155998
  • Pipeline finished with Failed
    2 months ago
    Total: 1080s
    #156010
  • Pipeline finished with Failed
    2 months ago
    Total: 961s
    #156023
  • Pipeline finished with Success
    2 months ago
    Total: 1909s
    #156033
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s

    comments responded / resolved

  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    The scope of the tickets are to just add permissions for the logged in users.

    Adding additional assertions is out of scope and definitely shouldnโ€™t be updating the performance test as nothing here should impact that.

  • Pipeline finished with Failed
    2 months ago
    Total: 8254s
    #157028
  • Pipeline finished with Canceled
    2 months ago
    Total: 770s
    #157151
  • Pipeline finished with Success
    2 months ago
    Total: 988s
    #157175
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s

    - reverted performance tests. worked now. looks like it was a random error in test the first time
    - removed one extra assert
    - keeping one assertion. If this assert is removed and the permission 'administer account settings', is also removed, the test passes but it would not be correct since the page that opens is access denied which doesn't have local tasks.

  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    A follow up should then be opened for expanding test coverage. Should stick within the scope of the ticket please

  • Pipeline finished with Success
    2 months ago
    Total: 1531s
    #157492
  • Pipeline finished with Success
    2 months ago
    Total: 1022s
    #157668
  • Status changed to Needs review 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s
  • Assigned to smustgrave
  • Status changed to Needs work 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Will fix later changing to permissionless seems to be changing what's tested.

  • Pipeline finished with Success
    2 months ago
    Total: 1110s
    #157718
  • Issue was unassigned.
  • Status changed to RTBC 2 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    All I did was some cleanup, still green

  • Status changed to Needs work about 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Now that user 1 is no longer special we can make the kernel tests much nicer here...

  • Pipeline finished with Success
    about 2 months ago
    Total: 1041s
    #160329
  • Status changed to RTBC about 2 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sukr_s

    suggestions applied

  • Status changed to Fixed about 2 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    Committed and pushed 8aeebfb32e to 11.x and 4ed57d454b to 10.3.x. Thanks!

Production build 0.69.0 2024