API page: https://api.drupal.org/api/drupal/core%21modules%21simpletest%21lib%21Dr...

Enter a descriptive title (above) relating to protected function WebTestBase::drupalPlaceBlock, then describe the problem you have found:

There's some documentation in WebTestBase::drupalPlaceBlock() that says:

Note: Until this can be done programmatically, the active user account must have permission to administer blocks.

But it looks to me as though the code is programmatic and would not require this any more. Should this be removed?

For the moment, moving to simplest component for decision; afterwards you can move to "documentation" component and mark "Novice" for the actual patch if it is OK to remove this documentation.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andrei.dincu’s picture

Assigned: Unassigned » andrei.dincu
Status: Active » Needs work
andrei.dincu’s picture

FileSize
723 bytes
andrei.dincu’s picture

Status: Needs work » Needs review
jhodgdon’s picture

The patch looks fine to me, but we still need to figure out if the text should be removed.

longwave’s picture

NewDefaultThemeBlocksTest is a relatively simple test that logs in before calling drupalPlaceBlock(). Let's see what happens if we remove the drupalLogin() and drupalLogout() calls.

If this passes, we should probably consider removing similar login/logout calls and "administer blocks" permissions in tests where they are no longer required.

jhodgdon’s picture

I am not sure that's a valid test of this. I kind of thought that if you never login/logout, WebTestBase acts as if you are user 1, but maybe not?

longwave’s picture

I thought you had to use $this->drupalLogin($this->root_user); to log in as user 1? Otherwise, tests start off with you being anonymous?

longwave’s picture

See \Drupal\block\Tests\BlockHtmlTest which deliberately logs in as $this->root_user, and arguably would have been a better example to use to test this issue.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Yes, actually I think you are correct, at least for 8.x.

OK, I think this is ready to go, unless we should remove some other unnecessary logins... Thanks!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.