Posted by scor on February 5, 2010 at 6:24pm
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | simpletest.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | HEAD broken |
Issue Summary
The Comment blocks test returns 1 fail, 21 exceptions such as:
Undefined index: 1#comment-1 Notice comment.module 1616 comment_load()All these exceptions are triggered when using clickLink() on a comment link, such as line 1013 in comment.test
<?php
$this->clickLink($comment1->subject);
?>The fragment gets encoded as part of the q variables and interpreted as part of the comment id. The fragment should be removed in clickLink() as the webserver cannot deal with it (only the client's browser can).
Note that I'm able to reproduce this on both debian PHP 5.2.6-1+lenny4 and MAMP PHP 5.2.10.
Comments
#1
#2
or more strictly
#3
Just found #671520: Curl and fragment URLs break testing. I suggest to commit the work around present in the attached patch (which disables the fragment before the CURL call), and remove it when the CURL fragment issue has been resolved.
#4
making the fragment optional in clickLink().
#5
I can confirm that this does fix the issue. While it might not be a proper fix, it's a necessary workaround to have the tests pass again.
#6
+++ modules/simpletest/drupal_web_test_case.php@@ -1452,6 +1452,10 @@ class DrupalWebTestCase extends DrupalTestCase {
+ // CURL breaks if the url contains a fragment.
+ // @todo remove when http://drupal.org/node/671520 is fixed.
+ unset($options['fragment']);
URL should be written URL instead of url.
+++ modules/simpletest/drupal_web_test_case.php@@ -1959,7 +1963,13 @@ class DrupalWebTestCase extends DrupalTestCase {
+ $url_target = explode('#',$url_target, 2);
Missing space.
Would be good to have some code comments too.
#7
Here is a more generic approach, which works for any URL with fragment passed to drupalGet(). $path can contain a fragment and url() is meant to handle it fine, so no need to split it in clickLink(). The work CURL around should only live in drupalGet().
#8
fix typo and space.
#9
Ick. :\
Committed to HEAD, but let's make sure we get #671520: Curl and fragment URLs break testing fixed so we can remove that hack...
#10
Ok, turns out that this triggers other test fails..
There are two tests that use a very special URL that does, beneath other things, also contain an # which needs to be escaped. The above change however strips everything behind the # off. That's not what we want in this case.
I don't see a way to automatically distinguish between these two use cases, any idea?
#11
roll back of #8 and fix of #1 upon chx's advice to isolate the fix in the call rather than evil drupalGet().
#12
It's way, way too late to change drupalGet. It's too central and leads to failures :) but fixing clickLink is what this issue is about.
#13
Same patch with more comments. This fixes:
Menu router 130 passes, 0 fails, and 0 exceptionsPath alias functionality 102 passes, 0 fails, and 0 exceptions
#14
Ok, let's try this one. Committed to HEAD.
#15
Automatically closed -- issue fixed for 2 weeks with no activity.