Task to convert Batch/PercentagesUnitTest.php to phpunit.

See #1938068: [Meta] Convert UnitTestBase to PHPUnit.

Files: 
CommentFileSizeAuthor
#20 batch-2003736-20.patch12.58 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,073 pass(es).
[ View ]
#20 interdiff.txt873 bytesdawehner
#18 drupal-2003736-18.patch14.32 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,667 pass(es).
[ View ]
#18 interdiff.txt1.47 KBdawehner
#16 drupal-2003736-15.patch12.57 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,402 pass(es).
[ View ]
#15 drupal-2003736-15.patch6.41 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#15 interdiff.txt6.41 KBdawehner
#11 drupal-2003736-10.patch13 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,980 pass(es).
[ View ]
#6 drupal-2003736-6.patch13.03 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 56,033 pass(es).
[ View ]
#6 interdiff.txt4.12 KBdawehner
#5 system-batch-phpunit-2003736-05.patch12.43 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 57,131 pass(es).
[ View ]
#4 system-batch-phpunit-2003736-04.patch12.44 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#3 system-batch-phpunit-2003736-03.patch12.45 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#1 system-batch-phpunit-2003736.patch2.11 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 55,729 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.11 KB
PASSED: [[SimpleTest]]: [MySQL] 55,729 pass(es).
[ View ]

This one is quite simple as well.

Status:Needs review» Needs work

PercentagesUnitTest should be PercentagesTest, no point having Unit in the name for PHPUnit tests.

Also:

<?php
$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),
     
// 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),
     
// The next five test cases simulate a batch with a single operation
      // ('total' equals 1) that takes several steps to complete. Within the
      // operation, we imagine that there are 501 items to process, and 100 are
      // completed during each step. The percentages we get back should be
      // rounded the usual way for the first few passes (i.e., 20%, 40%, etc.),
      // but for the last pass through, when 500 out of 501 items have been
      // processed, we do not want to round up to 100%, since that would
      // erroneously indicate that the processing is complete.
     
'20' => array('total' => 1, 'current' => 100/501),
     
'40' => array('total' => 1, 'current' => 200/501),
     
'60' => array('total' => 1, 'current' => 300/501),
     
'80' => array('total' => 1, 'current' => 400/501),
     
'99.8' => array('total' => 1, 'current' => 500/501),
    );
?>

This should be a dataprovider

require_once DRUPAL_ROOT . '/core/includes/batch.inc';

This is kinda of showstopper i think:(
not sure if we plan to convert batch to a service or something

Status:Needs work» Needs review
StatusFileSize
new12.45 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

This patch uses dataProviders, and has an initial Batch component (just moved the _batch_api_percentage function for now).

StatusFileSize
new12.44 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Oops, forgot to change test method name as per #2.

StatusFileSize
new12.43 KB
PASSED: [[SimpleTest]]: [MySQL] 57,131 pass(es).
[ View ]

Forgot to rename the file to match the new class name.

StatusFileSize
new4.12 KB
new13.03 KB
PASSED: [[SimpleTest]]: [MySQL] 56,033 pass(es).
[ View ]

Just a couple of nitpicks which makes it easier to use storm (for example match classname + test classname)

Status:Needs review» Needs work
Issue tags:-phpunit

The last submitted patch, drupal-2003736-6.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+phpunit

#6: drupal-2003736-6.patch queued for re-testing.

+++ b/core/tests/Drupal/Tests/Component/Batch/PercentagesTest.phpundefined
@@ -0,0 +1,100 @@
+  public function provider() {

maybe this name is too general and will bring troubles if we decide to move more tests in this class..behind this i believe batch have a potential as a component:)

but, after seeing that we have batch.storage service i am not quite sure we want to go towards this way...unless we rename the component to Percentage component..because in the end we might end up with something very confusing

StatusFileSize
new13 KB
PASSED: [[SimpleTest]]: [MySQL] 55,980 pass(es).
[ View ]

I think as the batch is entirely part of core not just a component this helper method should also just be in core. I don't think it makes sense to move this methods to the batch storage as it's an orthogonal part of the code.

Status:Needs review» Needs work
Issue tags:-phpunit

The last submitted patch, drupal-2003736-10.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+phpunit

#11: drupal-2003736-10.patch queued for re-testing.

Status:Needs review» Needs work

+++ b/core/tests/Drupal/Tests/Core/Batch/PercentagesTest.phpundefined
@@ -0,0 +1,100 @@
+  public function formatData() {

should be called providerTestPercentages

besides that, as we already discussed in irc with @dawehner we should move the batch.storage inside the Drupal\Core\Batch namespace

Status:Needs work» Needs review
StatusFileSize
new6.41 KB
new6.41 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

besides that, as we already discussed in irc with @dawehner we should move the batch.storage inside the Drupal\Core\Batch namespace

Opened a new issue for that: #2018257: Move batch storage to Drupal\Core\Batch

StatusFileSize
new12.57 KB
PASSED: [[SimpleTest]]: [MySQL] 57,402 pass(es).
[ View ]

urgs

Status:Needs review» Needs work

#14 comment still applies

Status:Needs work» Needs review
StatusFileSize
new1.47 KB
new14.32 KB
PASSED: [[SimpleTest]]: [MySQL] 57,667 pass(es).
[ View ]

That's easy.

+++ b/core/tests/Drupal/Tests/Core/Batch/PercentagesTest.phpundefined
@@ -0,0 +1,100 @@
+  public function testPercentages($total, $current, $expected_result) {
...
+  public function providerFormatData() {

no, i mean the provider method name..should be providerTestPercentages

StatusFileSize
new873 bytes
new12.58 KB
PASSED: [[SimpleTest]]: [MySQL] 58,073 pass(es).
[ View ]

Let's skip the moving for now and just rename the provider method.

Status:Needs review» Reviewed & tested by the community

great, thanks good to go

Status:Reviewed & tested by the community» Fixed

Committed 69f2cef and pushed to 8.x. Thanks!

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