- Patch #267333 by cwgordon7, David_Rothstein, lilou, et al: fixed batch api rounding.

2 files changed, 120 insertions, 2 deletions

diff --git a/includes/batch.inc b/includes/batch.inc
index 1e506abaa..ed29b2b23 100644
--- a/includes/batch.inc
+++ b/includes/batch.inc
@@ -294,8 +294,9 @@ function _batch_process() {
       $progress_message = $old_set['progress_message'];
     }
 
-    $current    = $total - $remaining + $finished;
-    $percentage = $total ? floor($current / $total * 100) : 100;
+    $current = $total - $remaining + $finished;
+    $percentage = _batch_api_percentage($total, $current);
+
     $elapsed    = $current_set['elapsed'];
     // Estimate remaining with percentage in floating format.
     $estimate   = $elapsed * ($total - $current) / $current;
@@ -324,6 +325,34 @@ function _batch_process() {
 }
 
 /**
+ * Helper function for _batch_process(): returns the formatted percentage.
+ *
+ * @param $total
+ *   The total number of operations.
+ * @param $current
+ *   The number of the current operation.
+ * @return
+ *   The properly formatted percentage, as a string. We output percentages
+ *   using the correct number of decimal places so that we never print "100%"
+ *   until we are finished, but we also never print more decimal places than
+ *   are meaningful.
+ */
+function _batch_api_percentage($total, $current) {
+  if (!$total || $total == $current) {
+    // If $total doesn't evaluate as true or is equal to the current set, then
+    // we're finished, and we can return "100".
+    $percentage = "100";
+  }
+  else {
+    // We add a new digit at 200, 2000, etc. (since, for example, 199/200
+    // would round up to 100% if we didn't).
+    $decimal_places = max(0, floor(log10($total / 2.0)) - 1);
+    $percentage = sprintf('%01.' . $decimal_places . 'f', round($current / $total * 100, $decimal_places));
+  }
+  return $percentage;
+}
+
+/**
  * Return the batch set being currently processed.
  */
 function &_batch_current_set() {
diff --git a/modules/simpletest/tests/batch.test b/modules/simpletest/tests/batch.test
new file mode 100644
index 000000000..bc0f5ea34
--- /dev/null
+++ b/modules/simpletest/tests/batch.test
@@ -0,0 +1,89 @@
+<?php
+// $Id$
+
+/**
+ * @file
+ * Unit tests for the Drupal Batch API.
+ */
+
+/**
+ * Tests the function _batch_api_percentage() to make sure that the rounding
+ * works properly in all cases.
+ */
+class BatchAPIPercentagesTestCase extends DrupalWebTestCase {
+  protected $testCases = array();
+
+  public static function getInfo() {
+    return array(
+      'name' => t('Batch API percentages'),
+      'description' => t('Tests the handling of percentage rounding in the Drupal batch API. This is critical to Drupal user experience.'),
+      'group' => t('Batch API'),
+    );
+  }
+
+  function setUp() {
+    // Set up an array of test cases, where the expected values are the keys,
+    // and the values are arrays with the keys 'total' and 'current',
+    // corresponding with the function parameters of _batch_api_percentage().
+    $this->testCases = array(
+      // 1/2 is 50%.
+      '50' => array('total' => 2, 'current' => 1),
+      // Though we should never encounter a case where the current set is set
+      // 0, if we did, we should get 0%.
+      '0' => array('total' => 3, 'current' => 0),
+      // 1/3 is closer to 33% than to 34%.
+      '33' => array('total' => 3, 'current' => 1),
+      // 2/3 is closer to 67% than to 66%.
+      '67' => array('total' => 3, 'current' => 2),
+      // A full 3/3 should equal 100%.
+      '100' => array('total' => 3, 'current' => 3),
+      // 1/199 should round up to 1%.
+      '1' => array('total' => 199, 'current' => 1),
+      // 198/199 should round down to 99%.
+      '99' => array('total' => 199, 'current' => 198),
+      // 199/200 would have rounded up to 100%, which would give the false
+      // impression of being finished, so we add another digit and should get
+      // 99.5%.
+      '99.5' => array('total' => 200, 'current' => 199),
+      // The same logic holds for 1/200: we should get 0.5%.
+      '0.5' => array('total' => 200, 'current' => 1),
+      // Numbers that come out evenly, such as 50/200, should be forced to have
+      // extra digits for consistancy.
+      '25.0' => array('total' => 200, 'current' => 50),
+      // Regardless of number of digits we're using, 100% should always just be
+      // 100%.
+      '100' => array('total' => 200, 'current' => 200),
+      // 1998/1999 should similarly round down to 99.9%.
+      '99.9' => array('total' => 1999, 'current' => 1998),
+      // 1999/2000 should add another digit and go to 99.95%.
+      '99.95' => array('total' => 2000, 'current' => 1999),
+      // 19999/20000 should add yet another digit and go to 99.995%.
+      '99.995' => array('total' => 20000, 'current' => 19999),
+    );
+
+    parent::setUp();
+  }
+
+  /**
+   * Test the _batch_api_percentage() function with the data stored in the
+   * testCases class variable.
+   */
+  function testBatchAPIPercentages() {
+    // Include batch.inc if it's not already included.
+    drupal_function_exists('_batch_api_percentage');
+    foreach ($this->testCases as $expected_result => $arguments) {
+      // PHP sometimes casts numeric strings that are array keys to integers,
+      // cast them back here.
+      $expected_result = (string)$expected_result;
+      $total = $arguments['total'];
+      $current = $arguments['current'];
+      $actual_result = _batch_api_percentage($total, $current);
+      if ($actual_result === $expected_result) {
+        $this->pass(t('Expected the batch api percentage at the state @numerator/@denominator to be @expected%, and got @actual%.', array('@numerator' => $current, '@denominator' => $total, '@expected' => $expected_result, '@actual' => $actual_result)));
+      }
+      else {
+        $this->fail(t('Expected the batch api percentage at the state @numerator/@denominator to be @expected%, but got @actual%.', array('@numerator' => $current, '@denominator' => $total, '@expected' => $expected_result, '@actual' => $actual_result)));
+      }
+    }
+  }
+}
\ No newline at end of file