[Module]Bundle is registered too late in WebTestBase::setUp()

We've been having this problem in Views, as well as #1708692: Bundle services aren't available in the request that enables the module, I think we just assumed it was all the other issue.

@tim.plunkett: Can you post a small patch here that demonstrates the bug with a failing test? I just tried a fresh checkout of Views 8.x-3.x, and tests seem to be passing without the hack in #1708692-19: Bundle services aren't available in the request that enables the module.

Simply add protected $profile = 'standard'; to any Views test.

I can work over the next few days to distill it down to a "small patch", but in the meantime, here's this :D

Perhaps the install profile part is a separate issue. Perhaps not.
But this is not normal:

#0 /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php(338): Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition('plugin.manager....')
#1 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/views.module(1460): Symfony\Component\DependencyInjection\ContainerBuilder->get('plugin.manager....')
#2 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/views.module(202): views_get_plugin_definitions()
#3 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/theme.inc(425): views_theme(Array, 'module', 'views', 'core/modules/vi...')
#4 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/theme.inc(577): _theme_process_registry(Array, 'views', 'module', 'views', 'core/modules/vi...')
#5 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/theme.maintenance.inc(95): _theme_build_registry(Object(stdClass), Array, 'phptemplate')
#6 [internal function]: _theme_load_offline_registry(Object(stdClass), Array, 'phptemplate', false)
#7 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/theme.inc(276): call_user_func_array('_theme_load_off...', Array)
#8 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/common.inc(2328): theme_get_registry(false)
#9 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/filter/filter.module(366): l('Filtered HTML', 'admin/config/co...')
#10 [internal function]: filter_permission()
#11 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/module.inc(901): call_user_func_array('filter_permissi...', Array)
#12 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/user/user.module(2341): module_invoke('filter', 'permission')
#13 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/user/user.module(2403): user_permission_get_modules()
#14 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/user/user.module(3070): user_role_grant_permissions('administrator', Array)
#15 [internal function]: user_modules_installed(Array)
#16 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/module.inc(925): call_user_func_array('user_modules_in...', Array)
#17 /var/lib/drupaltestbot/sites/default/files/checkout/core/includes/module.inc(515): module_invoke_all('modules_install...', Array)
#18 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php(718): module_enable(Array, true)
#19 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/comment/lib/Drupal/comment/Tests/CommentTestBase.php(29): Drupal\simpletest\WebTestBase->setUp()
#20 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/comment/lib/Drupal/comment/Tests/CommentNodeAccessTest.php(37): Drupal\comment\Tests\CommentTestBase->setUp()
#21 /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php(584): Drupal\comment\Tests\CommentNodeAccessTest->setUp()
#22 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(381): Drupal\simpletest\TestBase->run()
#23 /var/lib/drupaltestbot/sites/default/files/checkout/core/scripts/run-tests.sh(22): simpletest_script_run_one_test('59', 'Drupal\comment\...')
#24 {main}FATAL Drupal\comment\Tests\CommentNodeAccessTest: test runner returned a non-zero error code (1)

Copying this over from the other issue:

Here's the relevant call stack for one such occurence: https://privatepaste.com/ddf8081988
Note lines 13 and 14. l() is being called, which is calling theme_get_registry(), and then everything goes to hell.

So, I tried putting this into ViewTestBase:

  /**
   * Overrides TestBase::changeDatabasePrefix().
   *
   * This is the dirtiest hack. It is the only method called in
   * WebTestBase::setUp() that is after it resets $conf and before it calls
   * module_enable().
   */
  protected function changeDatabasePrefix() {
    variable_set('theme_link', FALSE);
    return parent::changeDatabasePrefix();
  }

And it all worked out.

From #1805996-8: [META] Views in Drupal Core:

5. Include Views and Views UI in the standard install profile.
Blocked on #1786990: [Module]Bundle is registered too late in WebTestBase::setUp()

Here's a patch that's identical to that one, but with views and views_ui added as dependencies to standard.info. I'm curious what breaks.

Will the exact same tests fail on just this?

That has got to be the single best example of narrowing scope I've ever seen. 2.5 MB to less than 2K. ;)

And yep, looks like they're all croaking with:

exception 'Symfony\Component\DependencyInjection\Exception\InvalidArgumentException' with message 'The service definition "views.bundle" does not exist.' in /var/lib/drupaltestbot/sites/default/files/checkout/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php:690

Looks like there's 4 ModuleAPI failures in #7 that aren't in #9. Trying to find out what's causing them.

Ah, those were a red herring, just due to standard.info not including 'config', a Views dependency. Ok, looks like registration of [Module]Bundle happening after hook_theme() is the key scope of this issue, as demonstrated in #9.

That's caused by filter_permission()

$format_name_replacement = user_access('administer filters') ? l($format->name, 'admin/config/content/formats/' . $format->format) : drupal_placeholder($format->name);

and then common.inc l()

  if (!isset($use_theme) && function_exists('theme')) {
    // Allow edge cases to prevent theme initialization and force inline link
    // rendering.
    if (variable_get('theme_link', TRUE)) {
      drupal_theme_initialize();
      $registry = theme_get_registry(FALSE);

That treats the symptom, yes.

But as sun points out in #14, this could happen in other ways as well.

The problem is in WebTestBase::setUp(), we install_drupal() for the profile being tested and then module_enable() any additional modules needed by the test. The module_enable() invokes hooks which could end up calling l(), or any number of other functions that end up expecting services from modules enabled by install_drupal(). Here's a really ugly fix, though I hope we can fix this more robustly in other issues like #1708692: Bundle services aren't available in the request that enables the module and #1784312: Stop doing so much pre-kernel bootstrapping.

Since both patches in #17 are green, I removed the mock Views example from here on out. I don't think we need any test case in this patch, because the real problem still needs to be dealt with in #1708692: Bundle services aren't available in the request that enables the module. The purpose of this issue is just to allow #1805996: [META] Views in Drupal Core to add Views to the Standard install profile.

This implements #18.2. #18.1 is already taken care of: notice there's 2 lines in WebTestBase :).

@sun, if this is what you meant in your first comment, then that's fine.

As to the second part, first to RTBC wins :)

EDIT: Ignore this, go with 19.

Same as #19, but needed to copy the use statement to TestBase.

#21: bundle-registration-1786990-21.patch queued for re-testing.

Looks straight-forward enough.

Committed and pushed to 8.x. Thanks!

I don't see this in the commit log, can you

git pull --rebase;
git push

please?

Pushed now, thanks