Use the non-interactive installer in WebTestBase::setUp()

Subscribing.

I am trying to decide if this is a duplicate of #630446: Allow SimpleTest to test the non-interactive installer or a prerequisite. Especially since this is filed as a bug report (which I kind of agree with), I'm leaning towards prerequisite :)

The sollution is to allow some tests to install Drupal with non-interactive flag, while other tests should go as it is now.

This will make almost all tests to rum faster, while other will user normail site installer.

To deal with the performance issue, maybe it's possible to keep some version of $this->preloadRegistry() around? I guess that would require convincing the Drupal installer not to balk if the registry tables already exist when it tries to install, but it seems like that might be doable.

Also cross-linking this issue with my comment at #1052692-51: New import API for major version upgrades (was migrate module)... If we can get install_drupal() working inside the context of an already-running site, it would probably help that issue also.

What about doing a fake batch call if we detect other batch jobs when installing?
something like should do it but it's a hacky hack:

\
      $batch =& batch_get();
        $progressive = $batch['progressive'];
        $batch['progressive'] = FALSE;
        if (count($batch['sets']) > 1) {
        $current_set = $batch['current_set'];
          $batch['current_set'] = 1;
          //Fake batch processing since we are already in a batch job!
          _batch_populate_queue($batch, 1);
          require_once DRUPAL_ROOT . '/includes/batch.inc';
          _batch_process();
          // Reset batch status.
          $batch['current_set'] =$current_set;
        }
        $batch['progressive'] = $progressive;

(in function install_run_task($task, &$install_state) )
What do you think? too ugly?

Hey @sun I was debugging the installation process for a while and I think I know where the problem is:

The postSave function for UserStorageController does:

      // If the password has been changed, delete all open sessions for the
      // user and recreate the current one.
      if ($entity->pass != $entity->original->pass) {
        drupal_session_destroy_uid($entity->uid);
        if ($entity->uid == $GLOBALS['user']->uid) {
          drupal_session_regenerate();
        }
      }

And that last drupal_session_regenerate is what is breaking the session id and therefore, changing the token. The call stack for this is:

core/modules/user/lib/Drupal/user/UserStorageController.php.Drupal\user\UserStorageController->postSave:169	
core/modules/entity/lib/Drupal/entity/DatabaseStorageController.php.Drupal\entity\DatabaseStorageController->save:427	
core/modules/user/lib/Drupal/user/UserStorageController.php.Drupal\user\UserStorageController->save:83	
core/modules/entity/lib/Drupal/entity/Entity.php.Drupal\entity\Entity->save:231	
core/includes/install.core.inc.install_configure_form_submit:1961	
core/includes/form.inc.form_execute_handlers:1470	
core/includes/form.inc.drupal_process_form:868	
core/includes/form.inc.drupal_form_submit:706	
core/includes/install.core.inc.install_run_task:454	
core/includes/install.core.inc.install_run_tasks:382	
core/includes/install.core.inc.install_drupal:85	
core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php.Drupal\simpletest\WebTestBase->setUp:675	
core/modules/field/lib/Drupal/field/Tests/FieldTestBase.php.Drupal\field\Tests\FieldTestBase->setUp:30	
core/modules/field/lib/Drupal/field/Tests/BulkDeleteTest.php.Drupal\field\Tests\BulkDeleteTest->setUp:88	
core/modules/simpletest/lib/Drupal/simpletest/TestBase.php.Drupal\simpletest\TestBase->run:525	
core/modules/simpletest/simpletest.module._simpletest_batch_operation:181	
core/includes/batch.inc.call_user_func_array:289	
core/includes/batch.inc._batch_process:289	
core/includes/batch.inc._batch_do:166	
core/includes/batch.inc._batch_page:85	
core/modules/system/system.admin.inc.system_batch_page:2305	
core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php.call_user_func_array:45	
core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php.Drupal\Core\EventSubscriber\{closure}:45	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.call_user_func_array:128	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.Symfony\Component\HttpKernel\HttpKernel->handleRaw:128	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.Symfony\Component\HttpKernel\HttpKernel->handle:72	
index.php.{main}:40	

So basically, using drupal_install, calls install_configure_form_submit() which in turn creates a new account and does:

$account->save();

and destroys the session.

Technically, we could overcome this problem if we temporarily changed $GLOBALS['user']->uid or, even better, if we could set $entity->original->pass to $entity->pass.

We need to figure out if we really need to regenerate the session when creating the user on the installation process, and then we could patch install_configure_form_submit() or maybe doing some juggling with $GLOBALS['user']->uid in WebTestBase::setUp()...

@sun no problem!

I think that drupal_session_regenerate() invalidates the old session id in the database:

  if (isset($old_session_id)) {
    $params = session_get_cookie_params();
    $expire = $params['lifetime'] ? REQUEST_TIME + $params['lifetime'] : 0;
    setcookie(session_name(), session_id(), $expire, $params['path'], $params['domain'], $params['secure'], $params['httponly']);
    $fields = array('sid' => session_id());
    if ($is_https) {
      $fields['ssid'] = session_id();
      // If the "secure pages" setting is enabled, use the newly-created
      // insecure session identifier as the regenerated sid.
      if (variable_get('https', FALSE)) {
        $fields['sid'] = $session_id;
      }
    }
    db_update('sessions')
      ->fields($fields)
      ->condition($is_https ? 'ssid' : 'sid', $old_session_id)
      ->execute();
  }

so maybe the only option is to avoid that session regeneration?

I will try a few options and report back.

Hey, wait... this is using the prefixed database so the session should be still working for the batch... *gets confused by simpletest* right?

Ok, quick update, session_id with parameters can only be used before session_start so it would not work for restoring the session id. so there are 2 options:

* Updating the session id for the old session in the database, and the token for the batch process or
* Avoid regenerating the session by setting the original password in the installer.

I'd go with the second option if this does not represent a problem for other reasons, since technically we're not changing the user's password (and I think we can get away with not regenerating the session when installing a new site, right?)

Well, adding

  $account->original = new stdClass();
  $account->original->pass = $account->pass;

before

  $account->save();

worked but there's another problem, with user_login_finalize() which does drupal_session_regenerate(); If I disable that session regenerate, the batch process works but I'm afraid it could break the installation process...

So back to restoring stuff on WebTestCase.php, probably is the best option, but we need to update session ids and tokens for batch processes.

There's also the problem of the cache that generates the failure in the tests above (http://qa.drupal.org/pifr/test/299058).

This is failing because on install, the cache class is set to InstallBackend which is a fake cache with dummy functions and does not save anything in the cache.

This class gets saved in the static $cache_objects in cache() (cache.inc: 26) and since this is not using drupal_static "because we do not want to change the storage of a cache bin mid-request" it keeps using the fake cache even after resetting the cache_classes variable.

@sun: do you think it is possible to perform the installation unsing $this->drupalPost('install.php') or something like that? maybe running though all the installation steps and clicking on the submit button?
This way we would be doing new requests and we would solve the session/batch problem as well as the cache issue...

Non-interactive install is a feature of Drupal that merits testing just like interactive install. It is heavily used by drush users, jenkins jobs, etc. The title of this issue suggests that we favor exercising one install method over the other.

@moshe weitzman indeed, although in this case we're not testing the installation process itself but trying to install drupal to run tests, but I see your point.

I think the issue here is that simpletest is using a completely different method to install drupal, reproducing a variation of the install process that in some cases does not work like neither the interactive install or the non interactive install, so that's what needs to be fixed.

Oh, also, using drupalGet wouldn't work because it needs the database.

It seems to me we need (at least) this change:

+++ b/includes/cache.inc
@@ -16,13 +16,12 @@
 function cache($bin = 'cache') {
+  $cache_objects = &drupal_static(__FUNCTION__, array());
+
   // Temporary backwards compatibiltiy layer, allow old style prefixed cache
   // bin names to be passed as arguments.
   $bin = str_replace('cache_', '', $bin);
 
-  // We do not use drupal_static() here because we do not want to change the
-  // storage of a cache bin mid-request.
-  static $cache_objects;
   if (!isset($cache_objects[$bin])) {
     $class = variable_get('cache_class_' . $bin);
     if (!isset($class)) {
@@ -30,6 +29,7 @@ function cache($bin = 'cache') {

@@ -30,6 +29,7 @@ function cache($bin = 'cache') {
     }
     $cache_objects[$bin] = new $class($bin);
   }

from #1297136: Use DrupalNullCache for the installer (originally at #1082328: Provide a proper no-op cache.inc) to make the cache work and then something like the attached patch would work (let's see what the testbot says).

I've also had to add

// Build a dummy 'context' array to avoid exceptions.
        $batch_context = array(
          'sandbox'  => &$current_set['sandbox'],
          'results'  => &$current_set['results'],
          'finished' => &$finished,
          'message'  => &$task_message,
        );

Before calling _install_module_batch to avoid exceptions and solved the batch problem regenerating and updating the tokens in the database, doing on startUp() (at the very beginning):

   $batch = batch_get();
    $this->batch_id = $batch['id'];
    $this->old_token = drupal_get_token($this->batch_id);

and then at the end of tearDown():

//Regenerate batch token after install_drupal created a new session_id.
    db_update('batch')
      ->fields(array(
        'token' => drupal_get_token($this->batch_id),
      ))
      ->condition('token', $this->old_token)
      ->execute();

This obviously needs a lot of cleanup but we need to come up with something that works first ;)

So far this is passing my small subset of tests and I'll let testBot decide whether this passes or not all of the tests for core :)

now with support for non-batched tests! (tested with drush test-run)

Ah, OK. I misunderstood the issue. Removing the first part since it is too vague (different from what?).

Hey @sun that's almost there, that one missing test seems to be related to drupal_save_session and, while debugging, I found that it's reset to TRUE in drupal_cron_run. See stack trace:

core/includes/session.inc.drupal_save_session:528	
core/includes/common.inc.drupal_cron_run:5279	
core/includes/install.core.inc.install_finished:1661	
core/includes/install.core.inc.install_run_task:530	
core/includes/install.core.inc.install_run_tasks:390	
core/includes/install.core.inc.install_drupal:85	
core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php.Drupal\simpletest\WebTestBase->setUp:667	
core/modules/system/lib/Drupal/system/Tests/Session/SessionTest.php.Drupal\system\Tests\Session\SessionTest->setUp:22	
core/modules/simpletest/lib/Drupal/simpletest/TestBase.php.Drupal\simpletest\TestBase->run:525	
core/modules/simpletest/simpletest.module._simpletest_batch_operation:181	
core/includes/batch.inc.call_user_func_array:286	
core/includes/batch.inc._batch_process:286	
core/includes/batch.inc._batch_do:163	
core/includes/batch.inc._batch_page:82	
core/modules/system/system.admin.inc.system_batch_page:2305	
core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php.call_user_func_array:45	
core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php.Drupal\Core\EventSubscriber\{closure}:45	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.call_user_func_array:128	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.Symfony\Component\HttpKernel\HttpKernel->handleRaw:128	
core/vendor/symfony/http-kernel/Symfony/Component/HttpKernel/HttpKernel.php.Symfony\Component\HttpKernel\HttpKernel->handle:72	
index.php.{main}:40	

core/includes/common.inc.drupal_cron_run:5279 calls

  drupal_save_session(TRUE);

and is executed after the drupal_save_session(FALSE) in TestBase. The thing here is that prepareEnvironment is being executed before install_drupal

When I re add drupal_save_session(FALSE) below install_drupal, this doesn't fail anymore. Here's a patch with this change.

The last patch had some garbage from my tests, sorry about that. Here's the same patch without the test files.

Just an update, here's a work in progress attempt to port this patch to D7.

Nice code cleanup. No UI impact.

#58: test.setup_.57.without-block.patch queued for re-testing.

#1719488: Rename language_manager() to language() and related cleanup might fix that.

It was recently decided to remove t() from assertion messages anyway: #500866: [META] remove t() from assert message

So that problem itself isn't a blocker here, but committing that patch arguably is.

The hard coded string array in settings.php works regardless of locale module. So it is untrue that t() does nothing if locale is disabled.

Coming from #1738330: Confusing Language negotiation when accessing / it would be nice if it was possible to change setup parameters like the langcode... looks like that's not possible in patch #71 at the moment. So maybe we can move the $settings variable to a class field, or add a respective parameter to setUp()?

Sorry, didn't mean to change the status.

This patch has 500 test passes LESS than the referenced one in the last comment. Thats weird....
Is it possible some tests don't get run, or is there something else going on?

Just a reroll for HEAD changes in TestBase::tearDown().

Just some questions: none of it necessarily warrants a "needs work".

+++ b/core/includes/batch.inc
@@ -439,18 +439,17 @@ function _batch_finished() {
+    // Clean-up the session. Not needed for CLI updates.
+    if (isset($_SESSION)) {
+      unset($_SESSION['batches'][$batch['id']]);
+      if (empty($_SESSION['batches'])) {
+        unset($_SESSION['batches']);
+      }
+    }
   }
   $_batch = $batch;
   $batch = NULL;
 
-  // Clean-up the session. Not needed for CLI updates.
-  if (isset($_SESSION)) {
-    unset($_SESSION['batches'][$batch['id']]);
-    if (empty($_SESSION['batches'])) {
-      unset($_SESSION['batches']);
-    }
-  }
-

I don't understand why this got moved into the if ($batch['progressive']) { block. There might be an obvious reason, but I don't know the Batch API well enough to say whether this makes sense.

+++ b/core/includes/bootstrap.inc
@@ -2584,7 +2605,12 @@ function drupal_fast_404() {
+  // tests from using the non-interactive installer, in which case Drupal is
+  // only happens to be installed within the same request, but subsequently

"Drupal is only happens to be" doesn't sound quite right.

+++ b/core/includes/install.core.inc
@@ -94,19 +94,27 @@ function install_drupal($settings = array()) {
+  // After execution, all tasks might be complete, in which case
+  // $install_state['installation_finished'] is TRUE. In case the last task
+  // has been processed, remove the global $install_state, so other code can
+  // reliably check whether it is running during the installer.
+  // @see drupal_installation_attempted()
+  $state = $install_state;
+  unset($install_state);

Comment says to do this in case it's the last task, but it's being done with no if statement. What if it's not the last task? Does removing $install_state cause problems then?

+++ b/core/includes/install.core.inc
@@ -240,11 +250,10 @@ function install_begin_request(&$install_state) {
   // The user agent header is used to pass a database prefix in the request when
   // running tests. However, for security reasons, it is imperative that no
   // installation be permitted using such a prefix.
-  if (isset($_SERVER['HTTP_USER_AGENT']) && strpos($_SERVER['HTTP_USER_AGENT'], "simpletest") !== FALSE) {
+  elseif (isset($_SERVER['HTTP_USER_AGENT']) && strpos($_SERVER['HTTP_USER_AGENT'], "simpletest") !== FALSE) {

Comment says this is important for security, but we're now limiting it to interactive mode only. Are we sure we don't need this security in non-interactive mode?

+++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php
@@ -222,36 +222,30 @@ abstract class UpgradePathTestBase extends WebTestBase {
-    drupal_static_reset();

Why is this being removed?

Thanks.

+++ b/core/includes/bootstrap.incundefined
@@ -1448,6 +1448,27 @@ function bootstrap_hooks() {
+
+  // Check whether locale() is available and operational.
+  if (!isset($locale_exits)) {
+    // The availability of locale() requires more explanation:
+    // - Whether the module system has been loaded.
+    //   In early bootstrap, in the installer, update.php, and also in early
+    //   error/exception conditions, the module system might not be loaded yet.
+    // - Whether Locale module is enabled.
+    //   The mere existence of locale() does not imply that Locale module is
+    //   actually enabled and its database tables are installed. Since PHP code
+    //   cannot be unloaded, this is typically the case in the environment that
+    //   is executing a test.
+    // - Whether Locale module actually has been loaded and locale() exists.
+    //   The Locale module may be enabled, but the actual .module might not be
+    //   loaded. This is typically the case in update.php, which needs to
+    //   populate the full module list to invoke hook_requirements(), but must
+    //   not load any actual modules and APIs, since they are not guaranteed to
+    //   be operational.
+    $locale_exists = function_exists('locale') && function_exists('module_exists') && module_exists('locale');
+  }

@@ -1469,7 +1490,7 @@ function t($string, array $args = array(), array $options = array()) {
     $string = $custom_strings[$options['langcode']][$options['context']][$string];
   }
   // Translate with locale module if enabled.
-  elseif ($options['langcode'] != LANGUAGE_SYSTEM && ($options['langcode'] != 'en' || variable_get('locale_translate_english', FALSE)) && function_exists('locale')) {
+  elseif ($options['langcode'] != LANGUAGE_SYSTEM && ($options['langcode'] != 'en' || variable_get('locale_translate_english', FALSE)) && $locale_exists) {
     $string = locale($string, $options['context'], $options['langcode']);
   }
   if (empty($args)) {

This is massively changing the meaning of $locale_exists. It's allowably to have a different module that implements the locale() function without locale module being enabled. That's a weird hack that is incredibly outdated, but if we have that feature we can't just lose it here.

Also:

+  if (!isset($locale_exits)) {

+++ b/core/includes/cache.incundefined
@@ -26,13 +26,12 @@
 function cache($bin = 'cache') {
+  $cache_objects = &drupal_static(__FUNCTION__, array());

Might be a case for the drupal_static_fast pattern?

+++ b/core/includes/module.incundefined
@@ -60,14 +64,29 @@ function module_load_all($bootstrap = FALSE) {
+function module_list($type = 'module_enabled', array $fixed_list = NULL, $reset = FALSE) {
+  // This static is only used for $fixed_list. It must not be a drupal_static(),
+  // since any call to drupal_static_reset() in unit tests would cause an
+  // attempt to retrieve the list of modules from the database (which does not
+  // exist).
+  static $module_list;
+
+  if ($reset) {
+    $module_list = NULL;
+    // Do nothing if no $type and no $fixed_list have been passed.
+    if (!isset($type) && !isset($fixed_list)) {
+      return;
+    }

We just got rid of this in #1503224: Cleanup module_list() :(

While theoretically another module could implement locale() it was also possible in Drupal 7 to swap in the cache array with an arrayaccess implementation and skip most of the logic in locale() by feeding the data there. I have not looked at the updated cache backend for locale() but it is likely still swappable in some way now.

+1 to RTBC

@catch says the cache clearing techniques here might help with solving #1787520: Translations not imported on installation as well which is also about a module being enabled but the fixed module list kept intact and therefore locale not exposing its stream wrapper which would be needed for importing .po files in the installer.

+++ b/core/modules/locale/locale.moduleundefined
@@ -254,10 +254,22 @@ function locale($string = NULL, $context = NULL, $langcode = NULL) {
+  if (!isset($locale_exits)) {
+    $locale_exists = function_exists('module_exists') && module_exists('locale');

Still locale_exits.

Just in case I've re tested installing using drush si and interactively using the browser. It all seemed to be working fine both before and after applying the patch. I've also ran the tests using the UI (the only other thing that is not tested by the bot, I think) and it works.

Thanks! Committed/pushed to 8.x. Bit of luck this will unblock some other issues.

awesome! is this worth backporting to D7? I started working on that a while ago but there have been some changes along the way, and @sun mentioned it could be tricky... I'd like to try anyways unless you guys think it can't or shouldn't be done.

Opsie. Let me quote: http://php.net/unset

To unset() a global variable inside of a function, then use the $GLOBALS array to do so

RTBC +1, just for process's sake.

I'd ah... hesitate to backport this.

Committed and pushed the follow-up in #119.

For those to whom #119 didn't make sense, here's what chx had to say when I asked him about the patch:

"webchick: when it finishes install it doesnt unset the install_state so the system thinks it's still installing
webchick: for example, drupal_get_profile is broken (at least)"

OK, so to follow up on how I found this: after a test method is torn down, drupal_get_profile still reports the testing profile because install_state lingers. Any other code that peeks at install_state would be similarly broken. And it's particularly hard to write a test for something that only happens outside of a test...