When trying to run simpletests with memcache as the caching backend, simpletest itself breaks your site by the child site writing cache entries that replace those from the parent site. To fix this dmemcache_prefix() can find the current test id if there is one, and add this to the key prefix, about the same as what happens with the database prefix.

With that done, everything works again via the UI. I haven't tried the tests via the cli yet, it might be necessary to convert static $prefix to drupal_static() as well for that buttthis needs backporting to D6 so taking it one step at a time.

With this change, all tests apart from the session test pass.

CommentFileSizeAuthor
#7 0001-1095608-Add-memcache.module-and-memcache.install-all.patch3.97 KBcatch
#1 simpletest_1095608.patch622 bytescatch
simpletest.patch626 bytescatch

Comments

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new simpletest_1095608.patch622 bytes

Better comment.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Committed to 6.x and 7.x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Might as well mark fixed. Will open another issue to backport tests to 6.x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Active

Grr, not quite fixed.

To have a class recognised by the registry, the foo.info where files[] is specified has to have an associated foo.module. This isn't the case for memcache.

One option is to move the tests to memcache_admin, add memcache.test to memcache_admin.info, and that should get picked up. However that doesn't make a lot of sense since we're not testing memcache_admin.

Alternatively, we could create a completely empty memcache.module in the main folder (probably add a comment up top explaining why), then the tests would get picked up, but that's potentially confusing. One advantage of doing it this way, is if people enabled the 'module' it'd show up at http://drupal.org/project/usage/memcache. It's also one empty file compared to moving a lot of things around.

jeremy’s picture

jeremy commented

The only negative I see to an "empty" memcache.module is that this will generate installation confusion: people have a tendency to not read documentation. One thought would be to add inline documentation to the memcache module -- when it's enabled, detect if memcache.inc is properly included, and if not throw up a message that steers people in the right direction?

catch’s picture

catch
he/him
Primary language English
commented

We could definitely add a hook_requirements() to the module, which would have the advantage of making it more of a real module than just a hack. That might also be somewhere we could add some inline docs re memcache vs. memcached, and hashing strategy as well.

catch’s picture

catch
he/him
Primary language English
commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new 0001-1095608-Add-memcache.module-and-memcache.install-all.patch3.97 KB

Here's a patch. Adds memcache.install and memcache.module. memcache.module just has a comment, memcache.install has a hook_requirements() implementation with a few basics there. Also tweaked the .info files to remove 'caching' package and so that memcache module wouldn't throw version compatibility errors. Posting this for 6.x first, will be trivial to forward port to 7.x.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented

Any reason why you chose to remove "Package = Caching" ?

catch’s picture

catch
he/him
Primary language English
commented

@fgm, package only makes sense if there are a number of modules to be used together (Ubercart for example). I don't think many sites will have more than one or two modules at most in the 'caching' package. Having said that this could be a different patch, it was just noticeable when looking at the modules list.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented

On any given site, only one caching module (+ possibly varnish) is likely to be active, indeed.

However, currently advcache, boost, cacherouter and varnish, all declare this package, and, by the same reasoning, should maybe remove it too ? At any rate, it seems this behaviour (caching package or no package) should be made consistent. (In the same vein, expire module should probably be removed from its "Performance and scalability" group and either put into "Caching" along with the other caching modules, or put in the big "Other" bag.

As a site admin, though, I tend to prefer finding any caching module in the caching group on a site modules page instead of having to go fishing in the "other" group.

catch’s picture

catch
he/him
Primary language English
commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

That's enough to persuade me this should go into a separate issue.

I went ahead and committed this without that change. Moving this to 7.x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Fixed

Alright I forward ported this patch, along with all the additional tests to the 7.x-1.x branch, marking fixed.

Current tests results from running all memcache tests:

202 passes, 2 fails, 2 exceptions, and 45 debug messages

http://drupalcode.org/project/memcache.git/commit/00ae205

Status: Fixed » Closed (fixed)

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

ryan_courtnage’s picture

ryan_courtnage commented

The original patch that was committed (simpletest_1095608.patch) will only solve the original problem ("trying to run simpletests with memcache as the caching backend") IF $conf['memcache_key_prefix'] isn't set.

In other words, if you have a settings.php that defines $conf['memcache_key_prefix'], AND you are trying to run simpletests on that system, you'll basically run into this issue. Not sure if this is by design?

if (!isset($prefix)) {
    $prefix = '';
    // memcache_key_prefix can be set in settings.php to support site namespaces
    // in a multisite environment.
    if ($prefix = variable_get('memcache_key_prefix', '')) {
      $prefix .= '-';
    }
    // When simpletest is running, emulate the simpletest database prefix here
    // to avoid the child site setting cache entries in the parent site.
    if (isset($GLOBALS['drupal_test_info']['test_run_id'])) {
      $prefix .= $GLOBALS['drupal_test_info']['test_run_id'];
    }
  }