The most obvious one that I've found so far is that uploading files using the @ trick is now deprecated in favor of CurlFile(), easy to fix. That is an E_DEPRECATED, which is missing in Simpletests error map, so it blows up because the group is empty.

See also #1867192: Testbots need to run on php 5.3.10, 5.4, 5.5, 5.6 and 7

For reference, this notice is thrown:

Deprecated: curl_setopt(): The usage of the @filename API for file uploading is deprecated. Please use the CURLFile class instead.

Files: 
CommentFileSizeAuthor
#20 php55-curl-file-upload-2054205-20.patch1.4 KBpfrenssen
PASSED: [[SimpleTest]]: [MySQL] 40,405 pass(es).
[ View ]
#20 interdiff.txt873 bytespfrenssen
#18 php55-curl-file-upload-2054205-18.patch1.39 KBpfrenssen
PASSED: [[SimpleTest]]: [MySQL] 40,449 pass(es).
[ View ]
#18 interdiff.txt1.38 KBpfrenssen
#9 php55-curl-file-upload-2054205-9.patch1.05 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 40,496 pass(es).
[ View ]
#1 php55-curl-file-upload-2054205-1.patch1.99 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 57,623 pass(es).
[ View ]

Comments

Berdir’s picture

Status:Active» Needs review
StatusFileSize
new1.99 KB
PASSED: [[SimpleTest]]: [MySQL] 57,623 pass(es).
[ View ]

Here's a patch for that. Haven't found anything else yet that's broken except those that are broken on 5.4 too.

Berdir’s picture

Ok, found something weird. NodeEntityFieldQueryAlterTest is a test without any test methods, that throws a fatal in the CLI runner and shows now results page in the UI.

Doesn't seem to be 5.5 specific?

Crell’s picture

I can't help but wonder why we're still using manual curl in Simpletest instead of Guzzle...

catch’s picture

Priority:Normal» Critical

@Crell I don't think anyone is spending time trying to refactor SimpleTest at the moment.

chx’s picture

Status:Needs review» Reviewed & tested by the community

I have verified that the new constants are available since 5.3.0 so that's good. And http://php.net/curl_file_create looks good too.

Berdir’s picture

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

I've run the two File upload tests without and without this patch on 5.4 and 5.5 and can confirm we currently have fail on 5.5 in HEAD and that the patch resolves the issue.

The changes look good. Committed 7f02544 and pushed to 8.x. Thanks!

David_Rothstein’s picture

Version:8.x-dev» 7.x-dev
Priority:Critical» Normal
Status:Fixed» Patch (to be ported)
Issue tags:+needs backport to D7

I think this affects Drupal 7 too, but not sure this qualifies as critical for Drupal 7... at least not yet.

Berdir’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.05 KB
PASSED: [[SimpleTest]]: [MySQL] 40,496 pass(es).
[ View ]

Yes, makes sense.

pfrenssen’s picture

Status:Needs review» Reviewed & tested by the community

Excellent, thanks, RTBC!

Crell’s picture

alexpott’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

No longer applies

git ac https://drupal.org/files/php55-curl-file-upload-2054205-9.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1080  100  1080    0     0   1346      0 --:--:-- --:--:-- --:--:--  1465
error: modules/simpletest/drupal_web_test_case.php: does not exist in index
Berdir’s picture

Status:Needs work» Reviewed & tested by the community

@alexpott, uh you confused me for a second ;) *You* don't apply to this issue :)

This is a 7.x backported, you already committed the 8.x patch a while ago :p

And, the patch still applies fine :)

klausi’s picture

Status:Reviewed & tested by the community» Needs work

E_DEPRECATED and E_USER_DEPRECATED only exist since PHP 5.3, Drupal 7 should run on 5.2 as well. You could argue that we only make this change in the web test file, but I think a lot of users will still run their automated build tests on 5.2, too.

Or is there a policy that we abandoned 5.2 for D7 now?

pfrenssen’s picture

Indeed Drupal 7 still officially supports PHP 5.2.5 and higher, even though PHP 5.3 is recommended. I suppose we can simply define those constants ourselves if they do not exist. They will never be thrown on PHP 5.2, but they will generate undefined constant notices.

I have looked into potential problems with bitwise operations with these constants and E_ALL which has different values on different PHP versions. Most of the times when this is used it is to subtract a value from E_ALL (eg E_ALL & ~E_USER_DEPRECATED) and this will give correct results, even on PHP 5.2.

Berdir’s picture

I think we can simply add those two to the array conditionally if they exist. We don't care if they exist or not, we only care that when they are thrown, the test doesn't explode.

pfrenssen’s picture

pfrenssen’s picture

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new1.38 KB
new1.39 KB
PASSED: [[SimpleTest]]: [MySQL] 40,449 pass(es).
[ View ]

Added the constants conditionally. Also restored the comment from the D8 patch that was lost in the backport to D7.

klausi’s picture

Status:Needs review» Needs work
+++ b/modules/simpletest/drupal_web_test_case.php
@@ -2042,7 +2051,14 @@ class DrupalWebTestCase extends DrupalTestCase {
+                // Use the new CurlFile class for file uploads when using PHP
+                // 5.5.0.

why 5.5.0? Also applies to any version higher than 5.5.0? Just use "5.5" as in the original patch or "5.5 or higher".

Otherwise looks ready.

pfrenssen’s picture

Status:Needs work» Needs review
StatusFileSize
new873 bytes
new1.4 KB
PASSED: [[SimpleTest]]: [MySQL] 40,405 pass(es).
[ View ]

Fully agree!

chx’s picture

Status:Needs review» Needs work
+      if (defined('E_DEPRECATED')) {
+        $error_map += array(
+          E_DEPRECATED => 'Deprecated',

I bet you'd meant !defined

klausi’s picture

Status:Needs work» Reviewed & tested by the community

No, the patch is correct. If the constant is defined we add them to our error map of known error codes, otherwise we are on PHP 5.2 and they don't exist anyway.

klausi’s picture

Issue summary:View changes

Included the notice that is thrown to help people find this issue.

Heine’s picture

Actual notice on PHP 5.5 is "Notice: Undefined offset: 8192 in /var/www/webroot/modules/simpletest/drupal_web_test_case.php on line 545"

Adding for search.

Crell’s picture

alexpott’s picture

David_Rothstein’s picture

Issue summary:View changes
Status:Reviewed & tested by the community» Fixed
David_Rothstein’s picture

Issue tags:+7.25 release notes

Status:Fixed» Closed (fixed)

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