Task to convert ValidNumberStepUnitTest to phpunit.

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

Files: 
CommentFileSizeAuthor
#20 interdiff.txt884 bytesjhedstrom
#20 system-validnumber-phpunit-2030173-20.patch12.17 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 58,870 pass(es).
[ View ]
#18 system-validnumber-phpunit-2030173-18.patch12.17 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 58,892 pass(es).
[ View ]
#16 interdiff.txt751 bytesjhedstrom
#16 system-validnumber-phpunit-2030173-16.patch12.17 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 57,555 pass(es).
[ View ]
#13 interdiff.txt1.83 KBjhedstrom
#13 system-validnumber-phpunit-2030173-13.patch12.09 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 57,090 pass(es).
[ View ]
#11 system-validnumber-phpunit-2030173.patch12.09 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 57,090 pass(es).
[ View ]
#8 codecoveragewithpatch.png334.7 KBYesCT
#5 system-validnumber-phpunit-2030173-05.patch12.1 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-validnumber-phpunit-2030173-05.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 system-validnumber-phpunit-2030173-01.patch9.7 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new9.7 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

This converts valid_number_step() to a component (Number::validStep()), and moves the unit test to phpunit, with dataproviders.

Status:Needs review» Needs work

The last submitted patch, system-validnumber-phpunit-2030173-01.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, system-validnumber-phpunit-2030173-01.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new12.1 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-validnumber-phpunit-2030173-05.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Forgot to add the new Number component file.

Status:Needs review» Reviewed & tested by the community

woo, looks good thanks:)

Issue tags:+phpunit, +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

StatusFileSize
new334.7 KB

This increases phpunit test code coverage from

patch Lines Functions and Methods Classes and Traits
no patch 18.22% 2742 / 15050 19.93% 368 / 1846 19.32% 34 / 176
patch 18.25% 2748 / 15056 19.98% 369 / 1847 19.77% 35 / 177

codecoveragewithpatch.png

Issue tags:-phpunit, -RTBC July 1

Status:Reviewed & tested by the community» Needs work
Issue tags:+phpunit, +RTBC July 1

The last submitted patch, system-validnumber-phpunit-2030173-05.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new12.09 KB
PASSED: [[SimpleTest]]: [MySQL] 57,090 pass(es).
[ View ]

Re-rolled #5.

That is a really nice test!

+++ b/core/lib/Drupal/Component/Utility/Number.phpundefined
@@ -0,0 +1,60 @@
+    // The fractional part of a double has 53 bits. The greatest number that could
...
+    // $step * 2^53, then dividing by $step will result in a very small remainder.
...
+    // fractional part of a float has 24 bits. That means remainders smaller than

Some lines exceed the 80 chars, so lets fix them.

+++ b/core/tests/Drupal/Tests/Component/Utility/NumberTest.phpundefined
@@ -0,0 +1,125 @@
+class NumberTest extends UnitTestCase {
+  public static function getInfo() {

Lets add an empty line here.

StatusFileSize
new12.09 KB
PASSED: [[SimpleTest]]: [MySQL] 57,090 pass(es).
[ View ]
new1.83 KB

Should address #12.

Status:Needs review» Reviewed & tested by the community

Thank you

Status:Reviewed & tested by the community» Needs work

+++ b/core/includes/common.incundefined
@@ -628,47 +629,13 @@ function valid_url($url, $absolute = FALSE) {
/**
  * Verifies that a number is a multiple of a given step.
  *
- * The implementation assumes it is dealing with IEEE 754 double precision
- * floating point numbers that are used by PHP on most systems.
+ * @see \Drupal\Component\Utility\Number::validStep()
  *
- * This is based on the number/range verification methods of webkit.
- *
- * @param $value
- *   The value that needs to be checked.
- * @param $step
- *   The step scale factor. Must be positive.
- * @param $offset
- *   (optional) An offset, to which the difference must be a multiple of the
- *   given step.
- *
- * @return bool
- *   TRUE if no step mismatch has occured, or FALSE otherwise.
- *
- * @see http://opensource.apple.com/source/WebCore/WebCore-1298/html/NumberInputType.cpp
+ * @deprecated as of Drupal 8.0. Use
+ *   \Drupal\Component\Utility\Number::validStep() directly instead
  */
function valid_number_step($value, $step, $offset = 0.0) {

I'm not sure that we should be removing the documentation of the parameters and return value here... we can remove the the details and replace with the @see...

So I think this should look like this...

+++ b/core/includes/common.incundefined
@@ -628,47 +629,13 @@ function valid_url($url, $absolute = FALSE) {
/**
  * Verifies that a number is a multiple of a given step.
  *
  * @see \Drupal\Component\Utility\Number::validStep()
  *
  * @param $value
  *   The value that needs to be checked.
  * @param $step
  *   The step scale factor. Must be positive.
  * @param $offset
  *   (optional) An offset, to which the difference must be a multiple of the
  *   given step.
  *
  * @return bool
  *   TRUE if no step mismatch has occured, or FALSE otherwise.
  *
  * @deprecated as of Drupal 8.0. Use
  *   \Drupal\Component\Utility\Number::validStep() directly instead
  */
function valid_number_step($value, $step, $offset = 0.0) {

... I know I committed a patch by @jhedstrom that removed docs like this is the past but I now consider that a mistake...

Status:Needs work» Needs review
StatusFileSize
new12.17 KB
PASSED: [[SimpleTest]]: [MySQL] 57,555 pass(es).
[ View ]
new751 bytes

This adds those params back. The reason I'd been removing them was due to some really large docblocks (not including the params/return), that seemed silly to duplicate on both the deprecated function and the new component method.

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

oh everyone forgot this issue i guess:P

Status:Needs work» Needs review
StatusFileSize
new12.17 KB
PASSED: [[SimpleTest]]: [MySQL] 58,892 pass(es).
[ View ]

Re-roll of #16.

Issue tags:-Needs reroll

+++ b/core/tests/Drupal/Tests/Component/Utility/NumberTest.php
@@ -0,0 +1,126 @@
+    $this->assertEquals($return, $expected);
...
+    $this->assertEquals($return, $expected);

$expected should be the first argument

StatusFileSize
new12.17 KB
PASSED: [[SimpleTest]]: [MySQL] 58,870 pass(es).
[ View ]
new884 bytes

Flipped the argument order as per #19.

Status:Needs review» Reviewed & tested by the community

ty

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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