drupalCreateUser() breaks if testing site/profile does not install Comment module [#1366232]

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

xjm

she/her

English

commented 9 December 2011 at 15:01

FWIW, +1 on the RTBC. :)

Comment #2

sun

German

Karlsruhe

commented 11 December 2011 at 05:05

drupal8.test-drupalcreateuser.0.patch queued for re-testing.

Comment #3

sun

German

Karlsruhe

commented 11 December 2011 at 22:15

Updated the issue summary to explain the change.

Comment #4

catch

he/him

English

commented 13 December 2011 at 03:51

Version:	8.x-dev	» 7.x-dev
Status:	Reviewed & tested by the community	» Patch (to be ported)

Yeah 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

sun

German

Karlsruhe

commented 14 December 2011 at 18:40

Status:

Patch (to be ported)

» Reviewed & tested by the community

Status	File	Size
new	drupal8.test-drupalcreateuser.5.patch	1.87 KB

Thanks!

Identical patch for D7, only change is /core dir.

Comment #6

webchick

she/they

English

Vancouver 🇨🇦

commented 22 December 2011 at 09:53

Status:

Reviewed & tested by the community

» Fixed

At 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!