Task to convert modules/system/lib/Drupal/system/Tests/Common to phpunit where possible.

Common/JsonUnitTest.php
Common/ValidUrlUnitTest.php
Common/ParseInfoFileUnitTest.php
Common/HtmlIdentifierUnitTest.php
Common/AutocompleteTagsUnitTest.php
Common/AttributesUnitTest.php
Common/SizeUnitTest.php
Common/TableSortExtenderUnitTest.php
Common/CascadingStylesheetsUnitTest.php
Common/DiffArrayUnitTest.php
Common/ValidNumberStepUnitTest.php
Common/ColorTest.php

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

Files: 
CommentFileSizeAuthor
#17 common-2003568-17.patch23.8 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,902 pass(es).
[ View ]
#17 interdiff.txt6.21 KBdawehner
#13 drupal-2003568-13.patch23.2 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 57,723 pass(es).
[ View ]
#13 interdiff.txt10.79 KBdawehner
#11 drupal-2003568-11.patch22.85 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 56,303 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#11 interdiff.txt1.32 KBdawehner
#9 drupal-2003568-9.patch22.18 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,757 pass(es).
[ View ]
#1 system-common-phpunit-2002568-01.patch35.8 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-common-phpunit-2002568-01.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new35.8 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch system-common-phpunit-2002568-01.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

This patch is quite involved compared to some of the others. A lot of these tests required constants and functions defined in includes/bootstrap.inc, so that got added to tests/bootstrap.php--not sure if there's currently a way around that.

I should have mentioned, Common/CascadingStylesheetsUnitTest.php could not be easily converted since it relies on comparing CSS files contents loaded from the file system, with those fetched from $GLOBALS['base_url'] via HTTP.

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

The last submitted patch, system-common-phpunit-2002568-01.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, system-common-phpunit-2002568-01.patch, failed testing.

Status:Needs work» Needs review

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

The last submitted patch, system-common-phpunit-2002568-01.patch, failed testing.

+++ b/core/tests/Drupal/Tests/Core/Common/AutocompleteTagsUnitTest.phpundefined
@@ -33,7 +34,7 @@ public static function getInfo() {
   function testDrupalExplodeTags() {
...
@@ -44,8 +45,8 @@ function testDrupalImplodeTags() {

The test methods match the old function names. they should probably be renamed. Technically not strictly necessary but keeping them would be kinda weird I think.

+++ b/core/tests/Drupal/Tests/Core/Common/ColorTest.phpundefined
@@ -5,15 +5,15 @@
-class ColorTest extends UnitTestBase {
+class ColorTest extends UnitTestCase {

Wondering if I haven't seen this already somewhere, didn't that get in yesterday or so?

+++ b/core/tests/Drupal/Tests/Core/Common/ParseInfoFileUnitTest.phpundefined
@@ -0,0 +1,34 @@
+class ParseInfoFileUnitTest extends UnitTestCase {
...
+      'name' => 'Parsing .info.yml files',
...
+    $path = __DIR__ . '/../../../../../modules/system/tests/common_test_info.txt';
+    $info_values = drupal_parse_info_file($path);

Not sure if we even still need this, given that we converted everything to yml, including the name of this test :p

And including bootstrap.inc/common.inc is a no-go I think, I'd rather convert those tests to DUBT or rewrite the code to use classes and static methods like we did for other examples.

Title:Convert system module's Common unit tests to phpunitConvert tags,attributes, diff and url validation unit tests to phpunit
Status:Needs work» Needs review
StatusFileSize
new22.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,757 pass(es).
[ View ]

Another classical example that you should never do too much in one issue.

I removed the conversions which didn't had conversions in bootstrap.inc and moved the explode/implode to a tags class.

On a few of the other component conversions, the @deprecated tag has been added to the wrapper function. I think we should do that in this case as well.

Also, great idea on reducing the scope of this issue.

StatusFileSize
new1.32 KB
new22.85 KB
FAILED: [[SimpleTest]]: [MySQL] 56,303 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Good idea, let's do it here.

+++ b/core/includes/common.incundefined
@@ -772,6 +774,7 @@ function valid_email_address($mail) {
+ *
  * Verifies the syntax of the given URL.
@@ -5581,47 +5586,28 @@ function watchdog_severity_levels() {
+ *
  * Explodes a string of tags into an array.

unneeded changes

+++ b/core/tests/Drupal/Tests/Core/Common/AttributesTest.phpundefined
@@ -0,0 +1,56 @@
+    $this->assertSame((string) new Attribute(array('title' => '&"\'<>')), ' title="&amp;&quot;&#039;&lt;&gt;"', 'HTML encode attribute values.');
...
+    $this->assertSame((string) new Attribute(array('class' => array('first', 'last'))), ' class="first last"', 'Concatenate multi-value attributes.');
...
+    $this->assertSame((string) new Attribute(array('alt' => '')), ' alt=""', 'Empty attribute value #1.');
+    $this->assertSame((string) new Attribute(array('alt' => NULL)), ' alt=""', 'Empty attribute value #2.');
...
+    $this->assertSame((string) new Attribute($attributes), ' id="id-test" class="first last" alt="Alternate"', 'Multiple attributes.');
...
+    $this->assertSame((string) new Attribute(array()), '', 'Empty attributes array.');
...
+      $this->assertSame((string) $value, 'value1', 'Iterate over attribute.');

this should use a dataprovider

+++ b/core/tests/Drupal/Tests/Core/Common/AutocompleteTagsTest.phpundefined
@@ -31,21 +33,21 @@ public static function getInfo() {
+  function testImplodeTags() {

needs visibility

+++ b/core/tests/Drupal/Tests/Core/Common/ValidUrlTest.phpundefined
@@ -0,0 +1,154 @@
+  public function validAbsoluteProvider() {

lets prefix this with provider:)

+++ b/core/tests/Drupal/Tests/Core/Common/ValidUrlTest.phpundefined
@@ -0,0 +1,154 @@
+    $url_schemes = array('http', 'https', 'ftp');
+    foreach ($url_schemes as $scheme) {

those could be merged into the provider

+++ b/core/tests/Drupal/Tests/Core/Common/ValidUrlTest.phpundefined
@@ -0,0 +1,154 @@
+  public function invalidAbsoluteProvider() {

provider prefix instead too

+++ b/core/tests/Drupal/Tests/Core/Common/ValidUrlTest.phpundefined
@@ -0,0 +1,154 @@
+    $url_schemes = array('http', 'https', 'ftp');
...
+    foreach ($url_schemes as $scheme) {

same as well

and all test below in same pattern

StatusFileSize
new10.79 KB
new23.2 KB
PASSED: [[SimpleTest]]: [MySQL] 57,723 pass(es).
[ View ]

Great review! Fixed a couple of more points like renaming the test classes etc.

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

The last submitted patch, drupal-2003568-13.patch, failed testing.

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

#13: drupal-2003568-13.patch queued for re-testing.

+++ b/core/tests/Drupal/Tests/Core/Common/AttributesTest.phpundefined
@@ -0,0 +1,76 @@
+  public function provideAttributeData() {

you missed an 'r'

also, i am sorry i failed to mention above..we tend to add the Test..
so providerTestAttributeData

+++ b/core/tests/Drupal/Tests/Core/Common/UrlValidatorTest.phpundefined
@@ -0,0 +1,174 @@
+  public function providerValidAbsolute() {

providerTestValideAbsolute
likewise

+++ b/core/tests/Drupal/Tests/Core/Common/UrlValidatorTest.phpundefined
@@ -0,0 +1,174 @@
+  public function providerInvalidAbsolute() {

providerTestInvalidAbsolute

+++ b/core/tests/Drupal/Tests/Core/Common/UrlValidatorTest.phpundefined
@@ -0,0 +1,174 @@
+    foreach (array('', '/') as $front) {
...
+    foreach (array('', '/') as $front) {

we should move this foreach to the provider instead.

+++ b/core/tests/Drupal/Tests/Core/Common/UrlValidatorTest.phpundefined
@@ -0,0 +1,174 @@
+  public function dataEnhanceWithScheme($urls) {

should be protected right?

StatusFileSize
new6.21 KB
new23.8 KB
PASSED: [[SimpleTest]]: [MySQL] 57,902 pass(es).
[ View ]

Fixed all the points

Status:Needs review» Reviewed & tested by the community

cool, its ready to go, thanks!

Status:Reviewed & tested by the community» Fixed

Committed 573068f and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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

seems this test fails on 5.5 #2068593: JsonTest fails with pecl-json