This issue has been run into multiple times and even discussed several times with nothing done about it.

Internal Drupal functions like xmlrpc call drupal_http_request which makes an HTTP request. When tests try to call these functions they don't pass the SimpleTest user-agent (database-prefix) information like the cURL handler does.

This broke a number of tests that used to pass before the database-prefixing was implemented.

To fix this a patch similar to simpletest_review_blogapi_xmlrpc.patch at http://drupal.org/node/245955 should be created. Instead of that patch which is specific for xmlrpc stuff a more broad catch-all patch should be applied.

I have create a patch that fixes the issue.

CommentFileSizeAuthor
#7 add_simpletest_user_agent.patch1.43 KBboombatower
#5 add_simpletest_user_agent.patch1.21 KBboombatower
#3 add_simpletest_user_agent.patch1.02 KBboombatower
add_simpletest_user_agent.patch1.06 KBboombatower
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work

$simpletest_ua_key this is not used in core.

boombatower’s picture

boombatower CreditAttribution: boombatower commented

I was wondering about that since I remember discussion about it not being necessary.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs work » Needs review
FileSize
add_simpletest_user_agent.patch1.02 KB

Updated patch. Previous one still worked since it just ignored the extra key. :)

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

I'd like to see a bit more code comments in the code. We should explain _why_ this is needed. Thanks for your hard work. :)

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs work » Needs review
FileSize
add_simpletest_user_agent.patch1.21 KB

Added more comments.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Needs work

I still think it needs more documentation. Why do we have to use headers instead of, say, a database variable. I know the answer, but it would be good to explain this. You added the "what", but the "why" and "how" is still missing from the documentation. We're almost there ...

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs work » Needs review
FileSize
add_simpletest_user_agent.patch1.43 KB

Probably a good idea since it is a rather odd bit of code.

boombatower’s picture

boombatower CreditAttribution: boombatower commented
Status: Needs review » Reviewed & tested by the community

I think that has been reviewed and it just needed comments...

boombatower’s picture

boombatower CreditAttribution: boombatower commented

This is showing up in another issue so it would be nice to get it resolved.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

For what is worth, this looks good now.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Normal » Critical

This fixes http://drupal.org/node/259988 - so confirming the RTBC and marking as critical.

floretan’s picture

floretan CreditAttribution: floretan commented

This should fix #249154: Aggregator tests fail when existing content is present. as well.

cwgordon7’s picture

cwgordon7 CreditAttribution: cwgordon7 commented

Note that this fixes xmlrpc tests too (#268063: Move includes/tests to simpletest/tests & provide hidden .info propery).

boombatower’s picture

boombatower CreditAttribution: boombatower commented

Yea! Three tests!

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

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

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