Backport SimpleTest testing in drupal_http_request()
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
From #478982: Wondering how to test a drupal_http_request() I have discovered what a PITA it is to try and use SimpleTest on Drupal 6.x when testing drupal_http_request for a request to your own site. Basically, in Drupal 7, we added a check to see if the db_prefix global matches SimpleTest, and pass that as the user agent to drupal_http_request instead of the default Drupal user agent. This allows page executions made with drupal_http_request() will not be using the testing db. We can't check if watchdog calls are made, etc.
You can read all the details in #478982: Wondering how to test a drupal_http_request() but what I'm wondering is if it would be ok to backport this change to D6. It does not break APIs and enables easier testing for contrib modules. Patch attached for review.
If this is not accepted, any modules that want to test running a request on a local path will need to do the following in their own code every time they call drupal_http_request();
<?php
...
// Since drupal_http_request() does not pass the SimpleTest user agent 'flag'
// in Drupal 6 core, we need to manually set the user agent header.
$headers = array();
if (preg_match("/simpletest\d+/", $GLOBALS['db_prefix'], $matches)) {
$headers['User-Agent'] = $matches[0];
}
...
drupal_http_request($url, $headers);
...
?>| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| 0-simpletest-httprequest-D6.patch | 1.22 KB | Ignored | None | None |
#1
+1
#2
Might as well bump to RTBC, this is a backport and works fine. We just need a decision on it since it isn't a bug per say.
#3
Helping tests work is important, so committed.
#4
Great! Thanks.
#5
Agreed! Thanks very much Gabor!
#6
Automatically closed -- issue fixed for 2 weeks with no activity.
#7
drupal_http_request() is producing an error with this add-on when using an array for $GLOBALS['db_prefix']
Line 492:
preg_match("/simpletest\d+/", $GLOBALS['db_prefix'], $matches)preg_match expects the second argument to be a string, not an array... Perhaps the if statement should be extended with an is_string() call like this:
if (is_string($GLOBALS['db_prefix']) && preg_match("/simpletest\d+/", $GLOBALS['db_prefix'], $matches)) {Thanx in advance & cheers
hctom
#8
Since Drupal 7 allows an array for db_prefix seems like this would need to be fixed in HEAD first now. Possibly other SimpleTest code is affected?
#9
@hctom: Line 492 causes "string expected" problem for us even without the "simpletest" module on Drupal 6.13.
I am not sure if another broken module causes this, but if we use "Check manually" on Available Updates page, Drupal 6.13 gives series of
warning: preg_match() expects parameter 2 to be string, array given in /includes/common.inc on line 492warnings after a remarkably long pause.
With your modified "if", warning disappears but simpletest module does not work anyway.
Tx.
#10
Please do not change the version and issue information.
#11
Followup patch for D7 that checks the $_SERVER['HTTP_USER_AGENT'] for the SimpleTest user agent instead of the db_prefix.
#12
The last submitted patch failed testing.
#13
@Dave Reid: Sorry, post form default values have set that.
#14
@chawl: Of course the error occures without the simpletest module installed... the mentioned line of code is code form the core common.inc file, so no other module may be in charge :)
I guess the only way to solve this is something like this:
<?phpif (is_string($GLOBALS['db_prefix']) && preg_match("/simpletest\d+/", $GLOBALS['db_prefix'], $matches)) {
$defaults['User-Agent'] = 'User-Agent: ' . $matches[0];
}
elseif (is_array($GLOBALS['db_prefix'])) {
foreach ($GLOBALS['db_prefix'] as $prefix) {
if (preg_match("/simpletest\d+/", $prefix, $matches)) {
$defaults['User-Agent'] = 'User-Agent: ' . $matches[0];
break;
}
}
}
?>
@Dave: what do you think about this solution?
Cheers
hctom
#15
@hctom: Tx :) I was really confused even I might suspect a rainy day.
Our problem turns out to be a remnant from Faceted Search module which needs $db_prefix to be defined as an array in the settings.php. But there were no problems before 6.13 strangely.
Anyway, sorry for bloating the thread.
#16
I realized the bug during a multisite install preparation where some tables are shared... and so the $GLOBALS['db_prefix'] also has to be an array...
... but you are right... this code fragment is new in 6.13
cheers
hctom
#17
Will we find solutions to the 6.13 problem here?
#18
Here are the followup patches for D7 and D6 that should fix things of db_prefix is an array. Please test and report back.
#19
Committed to CVS HEAD. Thanks!
#20
+1, please commit to D6 ASAP. 6.13 currently generates errors for any http request it generates (when module updates are checked, for example) on any site (as above) with db_prefix setup as an array, ie any multi-site, or using faceted search, for just a few common examples.
#21
+1
Used the D6 patch on a 6.13 multisite, the errors are gone with this patch.
#22
I have also been affected with on multisite install of D6.13. The D6 patch from #18 should really be committed before releasing D6.14.
#23
The patch in #18 resolved the problem for me in 6.13.
#24
Thanks, committed to Drupal 6 too.
#25
Subscribing. (I know it's fixed, but I wanted to remind myself I needed to patch this in my Drupal 6.x installs until 6.14 is released.)
#26
Automatically closed -- issue fixed for 2 weeks with no activity.
#27
In a multisite setup with shared tables, $db_prefix is *not* a string, but an array (see http://drupal.org/node/2622). The patch for this issue that made it into D6.13, however, doesn't handle this case ($db_prefix an array) correctly, since line 492 in common.inc
if (preg_match("/simpletest\d+/", $GLOBALS['db_prefix'], $matches)) {
now assumes $db_prefix to be a string. Therefore, on certain requests/actions an error/warning occurs, like:
warning: preg_match() expects parameter 2 to be string, array given in /srv/www/vhosts/freerunning.net/httpdocs/includes/common.inc on line 492.
Is there any solution in the making?
(like an "foreach($db_prefix ...)" loop?)
#28
@Breakerandi The patch was committed to the Drupal 6 branch and will be included in the next release (6.14). It is not included in 6.13.
#29
So in Drupal 6.14 the case that $db_prefix is an array will be handled correctly, right?
#30
@Breakerandi: Correct. When 6.14 is released, everything will work just fine.
#31
@Everybody, who helped with this issue: Thanx for all the work :)
I'm looking for ward to the new Drupal release
Cheers
hctom
#32
Yea, again!
#33
Great, Thank you all, too :-)
#34
Now everything works fine as Dave Reid hase prognosed in #30. Thank you very much! Drupal 6.14 rulz :-)
#35
Automatically closed -- issue fixed for 2 weeks with no activity.