Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Pol created an issue. See original summary.

Pol’s picture

mfb’s picture

Filtering the memory limit doesn't work as it could be specified in bytes, GB, etc.

Instead, we need to either parse the memory limit using parse_size(), or manually find the size, multiply, and recombine with any unit.

mfb’s picture

Oops that wasn't quite right, because MB was being tacked on to the byte size, which meant that the memory limit was multiplied by 1024 * 1024 * 2 instead of 2 :)

Pol’s picture

Status: Needs review » Reviewed & tested by the community
Fabianx’s picture

Title: [PHP 7.3] Fix BootstrapMiscTestCase::testCheckMemoryLimit() notice » [PP-D8] [PHP 7.3] Fix BootstrapMiscTestCase::testCheckMemoryLimit() notice
Status: Reviewed & tested by the community » Postponed

Unless I miss something the same problem exists in Drupal 8 in this file:

core/tests/Drupal/Tests/Component/Utility/EnvironmentTest.php

It needs to be fixed there first, before we can commit it to Drupal 7.

Pol’s picture

Assigned: Unassigned » Pol

Thanks for your input Fabian, I will provide a patch for D8 asap as well.

Pol’s picture

alexpott’s picture

+++ b/modules/simpletest/tests/bootstrap.test
@@ -735,7 +735,7 @@ class BootstrapMiscTestCase extends DrupalUnitTestCase {
-    $twice_avail_memory = ($memory_limit * 2) . 'MB';
+    $twice_avail_memory = parse_size($memory_limit) * 2;

As commented on the D8 issue. On DrupalCi this value is -1 in CLI. So this test is not testing what we think it is. I would replace $twice_avail_memory with something absurd like 3000000YB

mfb’s picture

Hahaha I am trying to imagine what is 3,000,000 YB of RAM and have been unable to. That is way beyond the largest supercomputers or the 64-bit architecture limit. If every human being on the planet had like 100 billion smartphones each, then that would get you in the ballpark.

Pol’s picture

Status: Postponed » Active
Pol’s picture

Status: Active » Needs review
FileSize
1.35 KB

Updated patch.

Pol’s picture

Title: [PP-D8] [PHP 7.3] Fix BootstrapMiscTestCase::testCheckMemoryLimit() notice » [PHP 7.3] Fix BootstrapMiscTestCase::testCheckMemoryLimit() notice
Pol’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Pending Drupal 7 commit

All good for me locally!

Fabianx’s picture

+++ b/modules/simpletest/tests/bootstrap.test
@@ -729,16 +729,12 @@ class BootstrapMiscTestCase extends DrupalUnitTestCase {
+    // Exceed a custom (unlimited) memory limit.

Let's sync this with the D8 changes made before committing.

Pol’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.34 KB

Updated patch - just changed the comment, it's now in sync with Drupal 8 comment.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC

  • Pol committed a5c07d4 on 7.x
    Issue #3023066 by Pol, mfb: [PHP 7.3] Fix BootstrapMiscTestCase::...
Pol’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Thanks all !!!

Status: Fixed » Closed (fixed)

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