Several tests change variables through page requests (ie. drupalGet, drupalPost), but the changes aren't picked up in the testing thread. This requires the test to manually reload variables, which is cumbersome and not obvious.

The variables should be reloaded automatically after page requests and setUp (new database).

If we decide that this is a good idea I will create a patch to clean up the tests based on this.

CommentFileSizeAuthor
#6 simpletest_refresh.patch7.79 KBboombatower
simpletest_refresh.patch2.25 KBboombatower

Comments

boombatower’s picture

boombatower commented

Do we want to take this one set further and refresh menus, cache, etc? Less common, but might happen.

Also need to consider performance loss.

dries’s picture

dries commented

boombatower, do you have an example scenario that helps me understand the need for this. I can 'sense' a need for it, but I'd like to back it up with a real data point. If you could help with that, that would be great.

boombatower’s picture

boombatower commented

An example of this issue is in the dblog test.

private function verifyRowLimit($row_limit) {
    // Change the dblog row limit.
    $edit = array();
    $edit['dblog_row_limit'] = $row_limit;
    $this->drupalPost('admin/settings/logging/dblog', $edit, t('Save configuration'));
    $this->assertResponse(200);
    // Reload variable cache (since the global $conf array was changed in another "process" when the settings page above was posted).
    $this->reloadVariables();
    // Check row limit variable.
    $current_limit = variable_get('dblog_row_limit', 1000);
    $this->assertTrue($current_limit == $row_limit, t('[Cache] Row limit variable of @count equals row limit of @limit', array('@count' => $current_limit, '@limit' => $row_limit)));
    // Verify dblog row limit equals specified row limit.
    $current_limit = unserialize(db_result(db_query("SELECT value FROM {variable} WHERE name = '%s'", 'dblog_row_limit')));
    $this->assertTrue($current_limit == $row_limit, t('[Variable table] Row limit variable of @count equals row limit of @limit', array('@count' => $current_limit, '@limit' => $row_limit)));
  }

The call to reloadVariables in necessary since a variable was changed in the previous drupalPost call, but isn't picked up since that call was processed in a different PHP thread. Reload variables then reads the values out of the database to ensure that changes are picked up.

boombatower’s picture

boombatower commented

As a note this was found in this issue: http://drupal.org/node/241467, but was part of the list that was ignored after SimpleTest got into core. :)

dries’s picture

dries commented
Status: Needs review » Needs work

OK, I understand now. It wasn't obvious from the PHPdoc -- it could be more explanatory, IMO.

Secondly, I think this patch should also clean up existing 'variables reload' code, such as the one in the example you quotes.

If you can reroll the patch with slightly more verbose information, and some clean-up code, I'll commit it right away.

Keep up the good work!

boombatower’s picture

boombatower commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new simpletest_refresh.patch7.79 KB

Tests cleaned, documentation enhanced, and cleaned up setUp and tearDown methods.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks for improving the documentation. Much appreciated.

dries’s picture

dries commented

Committed to CVS HEAD. Thanks boombatower.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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