Support from Acquia helps fund testing for Drupal Acquia logo

Comments

boombatower’s picture

Status: Active » Needs review
FileSize
1.27 KB

Don't know. Have been wondering the same.

Here's a patch..lets see how bad it is.

boombatower’s picture

FileSize
821 bytes

Woops other patch included.

boombatower’s picture

umm...359 :)

boombatower’s picture

Assigned: Unassigned » boombatower

Ok so if I try to run the tests not in the list I get an awesome error with no stack trace...
Here is what's in there when it tries to fry it. Assuming one of the systems can't recover.

array (
  'drupal_bootstrap_final_phase' => 8,
  'drupal_bootstrap_phases' => 
  array (
  ),
  'drupal_bootstrap_completed_phase' => 8,
  'conf_path' => 'sites/default',
  'drupal_get_filename' => 
  array (
    'module' => 
    array (
      'dblog' => 'modules/dblog/dblog.module',
      'block' => 'modules/block/block.module',
      'color' => 'modules/color/color.module',
      'comment' => 'modules/comment/comment.module',
      'dashboard' => 'modules/dashboard/dashboard.module',
      'field' => 'modules/field/field.module',
      'field_sql_storage' => 'modules/field/modules/field_sql_storage/field_sql_storage.module',
      'field_ui' => 'modules/field_ui/field_ui.module',
      'file' => 'modules/file/file.module',
      'filter' => 'modules/filter/filter.module',
      'help' => 'modules/help/help.module',
      'image' => 'modules/image/image.module',
      'list' => 'modules/field/modules/list/list.module',
      'menu' => 'modules/menu/menu.module',
      'node' => 'modules/node/node.module',
      'number' => 'modules/field/modules/number/number.module',
      'options' => 'modules/field/modules/options/options.module',
      'path' => 'modules/path/path.module',
      'profile' => 'modules/profile/profile.module',
      'rdf' => 'modules/rdf/rdf.module',
      'search' => 'modules/search/search.module',
      'shortcut' => 'modules/shortcut/shortcut.module',
      'simpletest' => 'modules/simpletest/simpletest.module',
      'system' => 'modules/system/system.module',
      'taxonomy' => 'modules/taxonomy/taxonomy.module',
      'text' => 'modules/field/modules/text/text.module',
      'toolbar' => 'modules/toolbar/toolbar.module',
      'update' => 'modules/update/update.module',
      'user' => 'modules/user/user.module',
      'default' => 'profiles/default/default.profile',
    ),
  ),
  'drupal_load' => 
  array (
    'module' => 
    array (
      'dblog' => true,
      'block' => true,
      'color' => true,
      'comment' => true,
      'dashboard' => true,
      'field' => true,
      'field_sql_storage' => true,
      'field_ui' => true,
      'file' => true,
      'filter' => true,
      'help' => true,
      'image' => true,
      'list' => true,
      'menu' => true,
      'node' => true,
      'number' => true,
      'options' => true,
      'path' => true,
      'profile' => true,
      'rdf' => true,
      'search' => true,
      'shortcut' => true,
      'simpletest' => true,
      'system' => true,
      'taxonomy' => true,
      'text' => true,
      'toolbar' => true,
      'update' => true,
      'user' => true,
      'default' => true,
    ),
  ),
  'ip_address' => '127.0.0.1',
  'drupal_page_is_cacheable' => false,
  'drupal_page_header' => true,
  'drupal_send_headers' => true,
  'drupal_add_http_header' => 
  array (
    'content-type' => 'text/html; charset=utf-8',
  ),
  'drupal_lookup_path' => 
  array (
    'map' => 
    array (
    ),
    'no_source' => 
    array (
    ),
    'whitelist' => 
    array (
    ),
    'system_paths' => 
    array (
    ),
    'no_aliases' => 
    array (
    ),
    'first_call' => true,
  ),
  '_drupal_bootstrap_full' => 1,
  'fix_gpc_magic' => false,
  'file_get_stream_wrappers' => 
  array (
    'public' => 
    array (
      'name' => 'Public files',
      'class' => 'DrupalPublicStreamWrapper',
      'description' => 'Public local files served by the webserver.',
      'override' => false,
    ),
    'private' => 
    array (
      'name' => 'Private files',
      'class' => 'DrupalPrivateStreamWrapper',
      'description' => 'Private local files served by Drupal.',
      'override' => false,
    ),
    'temporary' => 
    array (
      'name' => 'Temporary files',
      'class' => 'DrupalTemporaryStreamWrapper',
      'description' => 'Temporary local files for upload and previews.',
      'override' => false,
    ),
  ),
  'module_implements' => 
  array (
    'stream_wrappers' => 
    array (
      'system' => false,
    ),
    'stream_wrappers_alter' => 
    array (
    ),
    'query_alter' => 
    array (
    ),
    'query_menu_get_item_alter' => 
    array (
    ),
    'init' => 
    array (
      'dblog' => false,
      'node' => false,
      'system' => false,
      'user' => false,
    ),
    'menu_active_handler_alter' => 
    array (
    ),
    'file_url_alter' => 
    array (
    ),
    'element_info' => 
    array (
      'file' => false,
      'number' => false,
      'options' => false,
      'system' => false,
      'taxonomy' => false,
      'text' => false,
      'user' => false,
    ),
    'query_translatable_alter' => 
    array (
    ),
    'element_info_alter' => 
    array (
    ),
    'form_simpletest_result_form_alter' => 
    array (
    ),
    'form_alter' => 
    array (
      'comment' => false,
      'menu' => false,
      'path' => false,
      'profile' => false,
      'default' => false,
    ),
    'page_delivery_callback_alter' => 
    array (
    ),
    'page_build' => 
    array (
      'block' => false,
      'dashboard' => false,
      'shortcut' => false,
      'system' => false,
      'toolbar' => false,
    ),
    'query_block_load_alter' => 
    array (
    ),
    'block_info_alter' => 
    array (
      'block' => false,
      'dashboard' => false,
    ),
    'dashboard_regions' => 
    array (
      'dashboard' => false,
    ),
    'dashboard_regions_alter' => 
    array (
    ),
    'node_grants' => 
    array (
    ),
    'forms' => 
    array (
      'node' => false,
      'search' => false,
      'user' => false,
    ),
    'node_info' => 
    array (
    ),
    'query_node_type_access_alter' => 
    array (
    ),
    'form_search_block_form_alter' => 
    array (
    ),
    'block_view_search_form_alter' => 
    array (
    ),
    'block_view_alter' => 
    array (
      'menu' => false,
    ),
    'block_view_system_navigation_alter' => 
    array (
    ),
    'block_view_user_login_alter' => 
    array (
    ),
    'block_view_system_management_alter' => 
    array (
    ),
    'menu_contextual_links_alter' => 
    array (
    ),
    'block_view_system_main_alter' => 
    array (
    ),
    'block_view_system_powered-by_alter' => 
    array (
    ),
    'menu_local_tasks_alter' => 
    array (
    ),
    'help' => 
    array (
      'block' => false,
      'color' => false,
      'comment' => false,
      'dblog' => false,
      'field' => false,
      'field_sql_storage' => false,
      'field_ui' => false,
      'filter' => false,
      'help' => false,
      'image' => false,
      'menu' => false,
      'node' => false,
      'path' => false,
      'profile' => false,
      'search' => false,
      'simpletest' => false,
      'system' => false,
      'taxonomy' => false,
      'update' => false,
      'user' => false,
    ),
    'block_view_system_help_alter' => 
    array (
    ),
    'shortcut_default_set' => 
    array (
    ),
    'page_alter' => 
    array (
    ),
    'query_user_load_multiple_alter' => 
    array (
    ),
    'query_file_load_multiple_alter' => 
    array (
    ),
    'field_info' => 
    array (
      'file' => false,
      'image' => false,
      'list' => false,
      'number' => false,
      'taxonomy' => false,
      'text' => false,
    ),
    'field_info_alter' => 
    array (
    ),
    'field_widget_info' => 
    array (
      'file' => false,
      'image' => false,
      'number' => false,
      'options' => false,
      'taxonomy' => false,
      'text' => false,
    ),
    'field_widget_info_alter' => 
    array (
      'taxonomy' => false,
    ),
    'field_formatter_info' => 
    array (
      'file' => false,
      'image' => false,
      'list' => false,
      'number' => false,
      'taxonomy' => false,
      'text' => false,
    ),
    'field_formatter_info_alter' => 
    array (
    ),
    'field_storage_info' => 
    array (
      'field_sql_storage' => false,
    ),
    'field_storage_info_alter' => 
    array (
    ),
    'entity_info' => 
    array (
      'comment' => false,
      'node' => false,
      'system' => false,
      'taxonomy' => false,
      'user' => false,
    ),
    'entity_info_alter' => 
    array (
      'rdf' => false,
    ),
    'rdf_mapping' => 
    array (
      'comment' => false,
      'node' => false,
      'taxonomy' => false,
      'user' => false,
    ),
    'field_attach_pre_load' => 
    array (
    ),
    'field_read_field' => 
    array (
    ),
    'field_read_instance' => 
    array (
    ),
    'field_build_modes' => 
    array (
      'node' => false,
      'taxonomy' => false,
      'user' => false,
    ),
    'field_storage_details_alter' => 
    array (
    ),
    'field_attach_load' => 
    array (
    ),
    'entity_load' => 
    array (
      'rdf' => false,
    ),
    'user_load' => 
    array (
      'profile' => false,
    ),
    'library_alter' => 
    array (
    ),
    'rdf_namespaces' => 
    array (
      'system' => false,
    ),
    'js_alter' => 
    array (
      'simpletest' => false,
    ),
    'css_alter' => 
    array (
    ),
    'exit' => 
    array (
    ),
  ),
  'menu_get_custom_theme' => NULL,
  'menu_get_item' => 
  array (
    'batch' => 
    array (
      'path' => 'batch',
      'load_functions' => '',
      'to_arg_functions' => '',
      'access_callback' => '1',
      'access_arguments' => 'a:0:{}',
      'page_callback' => 'system_batch_page',
      'page_arguments' => 
      array (
      ),
      'delivery_callback' => '',
      'fit' => '1',
      'number_parts' => '1',
      'context' => '0',
      'tab_parent' => '',
      'tab_root' => 'batch',
      'title' => '',
      'title_callback' => 't',
      'title_arguments' => '',
      'theme_callback' => '',
      'theme_arguments' => 
      array (
      ),
      'type' => '4',
      'block_callback' => '',
      'description' => '',
      'position' => '',
      'weight' => '0',
      'file' => 'modules/system/system.admin.inc',
      'href' => 'batch',
      'options' => 
      array (
      ),
      'access' => true,
      'localized_options' => 
      array (
      ),
      'original_map' => 
      array (
        0 => 'batch',
      ),
      'map' => 
      array (
        0 => 'batch',
      ),
    ),
  ),
  'arg' => 
  array (
    'batch' => 
    array (
      0 => 'batch',
    ),
  ),
  'drupal_add_css' => 
  array (
    'modules/node/node.css' => 
    array (
      'type' => 'file',
      'weight' => 0,
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/node/node.css',
    ),
    'modules/system/defaults.css' => 
    array (
      'weight' => -99.999,
      'type' => 'file',
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/system/defaults.css',
    ),
    'modules/system/system.css' => 
    array (
      'weight' => -99.998,
      'type' => 'file',
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/system/system.css',
    ),
    'modules/system/system-menus.css' => 
    array (
      'weight' => -99.997,
      'type' => 'file',
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/system/system-menus.css',
    ),
    'modules/user/user.css' => 
    array (
      'type' => 'file',
      'weight' => 0.004,
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/user/user.css',
    ),
    'modules/simpletest/simpletest.css' => 
    array (
      'type' => 'file',
      'weight' => 0.005,
      'media' => 'all',
      'preprocess' => true,
      'data' => 'modules/simpletest/simpletest.css',
    ),
  ),
  'batch_get' => 
  array (
    'sets' => 
    array (
      0 => 
      array (
        'sandbox' => 
        array (
          'max' => 4,
        ),
        'results' => 
        array (
        ),
        'success' => false,
        'start' => 1256102053.7693,
        'elapsed' => 0,
        'title' => 'Running tests',
        'operations' => 
        array (
          0 => 
          array (
            0 => '_simpletest_batch_operation',
            1 => 
            array (
              0 => 
              array (
                0 => 'BlockAdminThemeTestCase',
                1 => 'BlockTestCase',
                2 => 'NewDefaultThemeBlocks',
                3 => 'NonDefaultBlockAdmin',
              ),
              1 => '25',
            ),
          ),
        ),
        'finished' => '_simpletest_batch_finished',
        'progress_message' => '',
        'css' => 
        array (
          0 => 'modules/simpletest/simpletest.css',
        ),
        'init_message' => 'Processing test 1 of 4 - <em>Admin theme block admin accessibility</em>.<br/>&nbsp;',
        'error_message' => 'An error has occurred.',
        'total' => 1,
      ),
    ),
    'current_set' => 0,
    'progressive' => true,
    'url' => 'batch',
    'source_page' => 'admin/config/development/testing',
    'redirect' => 'admin/config/development/testing/results/25',
    'theme' => 'garland',
    'redirect_callback' => 'drupal_goto',
    'id' => '25',
    'error_message' => 'Please continue to <a href="/batch?id=25&amp;op=finished">the error page</a>',
  ),
  'simpletest_verbose' => false,
)
boombatower’s picture

Eventually got

ini_set(): A session is active. You cannot change the session module's ini settings at this time	Warning	bootstrap.inc	477	drupal_environment_initialize()

after moving it.

moshe weitzman’s picture

Status: Needs review » Needs work

debug code in there ... i do think that resetting statics makes sense. thats why we built this whole static feature.

boombatower’s picture

Status: Needs work » Needs review
FileSize
691 bytes

Removed debug, still won't run all tests most likely.

catch’s picture

effulgentsia’s picture

The problem with why so few tests ran in prior patches is that Batch API doesn't survive a reset, and drupal_static_reset() is broken anyway (#620688: drupal_static_reset is broken). This patch merges the code from http://drupal.org/node/620688#comment-2250000. Let's see what bot thinks.

Status: Needs review » Needs work

The last submitted patch failed testing.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
7.63 KB

This solves the 3 exceptions from #9's test run. There remains the 1 failure in translatable fields. I need to put this down for now, but if someone can debug what's going on with translatable fields, and start thinking about how we need to refactor drupal_bootstrap() and Batch API, so that their variables can survive reset, then we should be golden.

Status: Needs review » Needs work

The last submitted patch failed testing.

yched’s picture

Hm, batch API is sufficiently low level (required by install, update, testing...), that I don't think batch_get() should use drupal_static() to begin with. It's precisely not something that we want to be resettable from the outside...

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
8.77 KB

Thanks, yched, excellent point. Besides, it returns a reference, so anything wanting to mess with it can call batch_get() and manipulate the result. Also, for reasons commented in the patch, the drupal_bootstrap() statics shouldn't use drupal_static() either. This patch is therefore clean, except for the Field Translation failure. CNR so bot can re-confirm that that's the only remaining failure.

Status: Needs review » Needs work

The last submitted patch failed testing.

yched’s picture

 function &batch_get() {
-  $batch = &drupal_static(__FUNCTION__, array());
-  return $batch;
+  // Batch API operates at a lower level than most use-cases for resetting
+  // static variables, and we specifically do not want a global
+  // drupal_static_reset() resetting the batch information, so do not use
+  // drupal_static(). However, we need to return a reference, and PHP does
+  // not allow static variables to be set with or returned as references, but
+  // does allow references inside of static arrays.
+  static $static = array();
+  if (!isset($static['batch'])) {
+    $static['batch'] = array();
+  }
+  return $static['batch'];
 }

I don't think I get this. Before #422362: convert form.inc to use new static caching API, the function was :

function &batch_get() {
  static $batch = array();
  return $batch;
}

Worked fine - probably thanks to the & in front of the function name ? (just like drupal_static(), BTW)

effulgentsia’s picture

You're right. Statics can't be set to references, and I assumed that meant they can't be returned by reference, but I just tested it, and it turns out that returning static by reference works fine, so I'll include the cleanup in the next patch, once I or someone figures out what's causing the Field Translation test failure.

effulgentsia’s picture

Status: Needs work » Needs review
FileSize
9.26 KB

Found it! This one should pass.

effulgentsia’s picture

Re-rolled due to #620688: drupal_static_reset is broken. Any takers on RTBC'ing it or providing feedback?

effulgentsia’s picture

Better re-roll.

yched’s picture

Nitpick in batch_get():
"The only legitimate API for resetting batch information is batch_set()."
That's inaccurate. batch_set() only lets you add new sets to the batch. The only way to reset the batch to array() is:

$batch = &batch_get();
$batch = array();

Frankly, I'm not sure it's worth mentioning, I don't see the use case.

effulgentsia’s picture

@yched: better?

effulgentsia’s picture

@yched: I just re-read batch_set() and now see what you meant. So, how's this comment?

yched’s picture

@effulgentsia: well, IMO it would be OK to stop at "we specifically do not want a global drupal_static_reset() resetting the batch information", but the text is correct ;-). Thanks.

effulgentsia’s picture

@yched: Thanks. I'm okay with stopping where you suggest if you or someone else prefers that. My motivation in having the more verbose text is to stop a core developer in the future from changing this back to drupal_static() because they have a use-case for needing to reset.

chx’s picture

final_phase says

+ // Not drupal_static(), because the only legitimate API to control this is to
+ // call drupal_static() with a new phase parameter.

heh no. call drupal_bootstrap with a new phase parameter.

effulgentsia’s picture

D'oh! Thanks for catching that!

effulgentsia’s picture

I guess bot was down when #27 was posted, and even though it passed (click "view details"), it's not the nice and shiny green we all like to see, so re-uploading to get the green.

effulgentsia’s picture

Title: DrupalUnitTestCase::setUp() should blow up all statics » setUp() function for unit and web test cases should reset all resettable statics

This still needs someone to set it to RTBC.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

I'm gonna risk breaking protocol here, and will set my own patch to RTBC. Because:

1) The concept of resetting all statics during the setup of a test is supported by many people, on this issue and others. See #6.
2) This specific implementation has been reviewed by chx and yched, and both of them reported only minor issues with comments, which have been addressed.
3) It just makes sense to get this into HEAD sooner rather than later, since it may help uncover bugs with other patches in the issue queue.

Dries’s picture

I think this is a smart thing to do because it will make things more robust. However, the patch needs a reroll because of the field.test changes.

Dries’s picture

Status: Reviewed & tested by the community » Needs work
yched’s picture

yched’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.24 KB

Here's a reroll for the rest of the patch, and for the field.test hunk after #638356: Reorganize field_test.module

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Alright-y, committed to CVS HEAD. Hopefully the tests will all pass.

yched’s picture

Status: Fixed » Active

Hm, is it possible that this patch broke the 'verbose' information on test results ?

Even with 'verbose' checkbox set on admin/config/development/testing/settings, test results do not contain the links to the HTML pages after a drupalPost / drupalGet. Worked yesterday :-(.

yched’s picture

Status: Active » Needs review
FileSize
942 bytes

Yup, simpletest_verbose() had a drupal_static()-based $verbose variable. That one should be a regular static as well.

yched’s picture

FileSize
949 bytes

Er, let's explicitly initialize it to NULL like the other statics in the function.

Dries’s picture

Status: Needs review » Fixed

Committed. Thanks yched.

Status: Fixed » Closed (fixed)

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