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.Spin-off from #913086: Allow modules to provide default configuration for running tests
Patch literally extracted from that issue; has been peer-reviewed plenty of times already.
Problem
- drupalCreateUser() throws an error when Comment module is not installed.
Goal
- Remove dependency on Comment and Node modules from testing API.
Details
- drupalCreateUser() attempts to create a new user with a set of default permissions pertaining to Node and Comment modules.
- When Comment or Node module is not installed, drupalCreateUser() fails to create a user role with the default permissions, because the permissions do not exist.
- Those default permissions are superfluous anyway, because they are assigned by default to the authenticated user role, which applies to all authenticated users. Thus, it is pointless to create a new user role for the default permissions.
API changes
None. As detailed above, the removed permissions are the default permissions either way.
Impact
This patch is contained in several other patches already, which will have to be re-rolled after this lands:
#375397: Make Node module optional
#913086: Allow modules to provide default configuration for running tests
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | drupal8.test-drupalcreateuser.5.patch | 1.87 KB | sun |
| drupal8.test-drupalcreateuser.0.patch | 1.88 KB | sun | |
Comments
Comment #1
xjmFWIW, +1 on the RTBC. :)
Comment #2
sundrupal8.test-drupalcreateuser.0.patch queued for re-testing.
Comment #3
sunUpdated the issue summary to explain the change.
Comment #4
catchYeah this looks completely fine to me, the fact no tests need to be changed at all shows these permissions as redundant.
The only caveat here is that if we change the default permissions for the authenticated user role, then tests might need to be updated, but I doubt we'll do that for comments anyway, and really only tests in comment module should break in that case (and rightly so). So committed/pushed to 8.x. fwiw I think it looks like a good change for 7.x as well but not sure what we gain from the backport? That might need fleshing out a bit, but up to webchick of course.
Comment #5
sunThanks!
Identical patch for D7, only change is /core dir.
Comment #6
webchickAt first glance, this seemed like a no-go for D7 since it looked like it would change the default permissions assigned to users out from under various test authors' feet. However, as explained both in this issue and in the PHPDoc of drupalCreateUser(), what it's actually doing is avoiding the work of creating additional roles if there are no additional permissions, and just defaulting them to whatever auth user has. Makes sense.
So this seems like a harmless change for D7, although I guess if anyone is using their own custom testing profile, and they've customized the default permissions for auth user to not include the comment permissions, and they are nevertheless depending on all users having comment-related permission already in their tests, they'll see breakage. That seems pretty edge-casey to me. It seems far more likely that the code as it is would create a WTF for them since they never specified comment permissions, and so would have to explicitly undo that. I guess in any case, we still have enough time before the release next month for someone to catch that on their dev server, if it bites anyone. Let's give it a shot.
Committed and pushed to 7.x. Thanks!
Comment #8
cweagansUpdating tags per http://drupal.org/node/1517250
Comment #8.0
cweagansUpdated issue summary.