On our hosting environment, when running this test, we get a "preg_split()" unknown error notice in the CommentToggle test:
GET http://simpletest.gardens-simpletest.acquia-sites.com/ returned 200 (7.84 KB). Browser search.test 1058 SearchCommentCountToggleTestCase->testSearchCommentCountToggle()
Valid HTML found on "http://simpletest.gardens-simpletest.acquia-sites.com/" Browser search.test 1058 SearchCommentCountToggleTestCase->testSearchCommentCountToggle()
Verbose message Debug search.test 1058 SearchCommentCountToggleTestCase->testSearchCommentCountToggle()
preg_split() [function.preg-split]: Unknown error Notice search.module 1306 search_simplify_excerpt_match()
I am unable to determine what the root cause is. it isn't a segfault so strace doesn't provide any more information. We just get Notice: preg_split(0 unknown error in the simpletest results. The tests otherwise pass, but it makes our greenboard look red :(
I did find this potentially related bug:
http://www.mail-archive.com/php-bugs@lists.php.net/msg103869.html
If I make the change in the attached path, it works fine changing the single quotes to double quotes. Not sure why there are quotes there in the first place though.
| Comment | File | Size | Author |
|---|---|---|---|
| 537278_searh_comment_toggle_quotes.patch | 1.52 KB | JacobSingh |
Comments
Comment #1
pwolanin commentedlooks mostly harmless, but the last change doesn't follow the pattern of the changes to the other strings.
Comment #2
JacobSingh commentedYeah, since there is no space, I figured there was no point in having any quotes there.
Comment #3
jhodgdonVery odd. This PHP bug (if that's what it is) is not cropping up on the d.o test servers, but all that would mean is that they are running a different version of PHP from what you are running.
The test bot appears to be happy with the changes you have made. I can't see a reason that the tests are any less valid with single vs. double quotes in the first three cases, or why we needed quotes at all in the last case, so let's just get this in if it gets around some strange PHP issue on Jacob's test environment.
Comment #4
JacobSingh commentedAcquia hosting uses a hardened version of PHP which detects memory corruption. So sometimes bugs in PHP which go undetected in "normal" builds can throw notices or even worse segfaults there.
Thanks for the prompt review! You're a star as always.
Comment #5
jhodgdonNeeds to go to d8 and then d7
Comment #6
damien tournoud commentedHm. I don't see the point in hiding a real bug under the carpet? The tests do not pass on your environment, but it doesn't seem to be a bug in the tests. Fix the environment, not the tests, please.
Comment #7
JacobSingh commentedDo we explicitly state somewhere that the tests should work in "xyz" environments, but not "abc"? There should be a list of supported platforms, but I don't know where it is. What I do know is that the version of PHP Acquia Hosting uses is not from php.net, but is very widely used and comes as a .deb in Ubuntu. Whether that means that it is a "broken environment" or not is not my call (I'm not sure who's it is). At any rate, the patch fixes it for everyone and AFAICT does no harm.
I'll have Barry Jaspan (architect of Acquia Managed Cloud) comment with the specifics.
Comment #8
pwolanin commented@DamZ if the change *should* be a no-op for any environment, but happens to work around a bug in PHP 5.2.4 (which is supported for Drupal 7 at least, and hence all tests should pass on it), I don't understand why this is unacceptable.
Comment #9
damien tournoud commentedIt seems that the issue is inside the search module (in search_simplify_excerpt_match()), and could be triggered by an end-user in production.
Having a workaround in search_simplify_excerpt_match() for broken versions of PHP (or probably more likely broken versions of the PCRE lib) would be totally acceptable. What is not acceptable is changing the test to hide the bug under the carpet, which is what this patch does.
Comment #10
pwolanin commentedok, thanks for clarifying.
Comment #11
jhodgdonI disagree. I think this is an artifact of the SimpleTest system's form submission interacting with a strange PHP bug, and I am reasonably certain it cannot be triggered from a normal user form submission in a Drupal site. Jacob: perhaps you can verify that single and double quotes don't trigger a similar error when using the Search functionality on your test box where you get this testing error?
Comment #12
damien tournoud commentedIf this is the case, we need to get to the bottom of it and understand which "strange PHP bug" this is an why this is only triggered under SimpleTest. But from the limited debug information provided in the original post, it seems that the notice is triggered in the child site, not on the parent site.
Comment #13
jhedstromIt's been quite some time here. Is this still relevant to 8.x with our updated php requirements? If not, it may still effect 7.x.
Comment #21
pameeela commentedThanks for reporting this. It looks like we haven't been able to reproduce this issue since before the release of Drupal 7. It also seems to have involved:
Based on that, I think it is safe to close this issue. If a similar error is encountered in the future, I would recommend filing a new issue rather than reopening this one.
Thanks!