WebTestBase::checkPermissions() statically caches the available module permissions.

That is completely unnecessary and gets into the way of all tests that need to set up fixtures.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

I thought you were trying to make the tests more efficient. How much does removing this slow the tests down?

Status: Needs review » Needs work

The last submitted patch, drupal8.test-checkpermissions.0.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
8.1 KB

Yes, we're working hard to improve the overall test suite performance. However, this is an "optimization" on the completely wrong end — performance optimizations should never cripple the developer experience, and in addition, this custom static variable can get out of sync too easily.

checkPermissions() is only invoked from drupalCreateUser() / drupalCreateRole(). Implementations of hook_permissions() are either returning simple arrays (fast), or if they call into other services, then those services are caching already. Therefore, there's no need to duplicate any caches here.

I mistakenly removed the array_keys() from the hook invocation.

jhodgdon’s picture

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Nice cleanup

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Agreed. I'd have no idea I needed to call that were I doing something with permissions in my own test.

Committed and pushed to 8.x. Thanks!

  • Commit 5d29f32 on 8.x by webchick:
    Issue #2256317 by sun: Remove static caching from WebTestBase::...

Status: Fixed » Closed (fixed)

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