Problem/Motivation

In #2022931: Move drupal_var_export() to \Drupal\Component\Utility the Variable static class was created.

In #2002514: Deprecate debug(); remove references to _drupal_debug_message()., due to scope change, the unit tests were never written.

Proposed resolution

Write the tests.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#14 interdiff.txt1 KBMile23
#14 2163245_14.patch2.69 KBMile23
#12 interdiff.txt2.06 KBMile23
#12 2163245_12.patch2.49 KBMile23
#9 2163245_9.patch2.83 KBMile23
#6 interdiff.txt1.67 KBMile23
#6 2163245_5.patch2.83 KBMile23
#3 2163245_3.patch2.86 KBMile23
#1 2163245_1.patch2.74 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review
FileSize
2163245_1.patch2.74 KB

Le Patch.

Note that this test results in 100% code coverage, even though it shouldn't.

The bit at the end of Variable::export() that adds the prefix is always run, due to the way the $prefix default is set.

  public static function export($var, $prefix = '') {
    // ...
    if ($prefix) {
      $output = str_replace("\n", "\n$prefix", $output);
    }

It should either set $prefix to NULL, or just always run str_replace() which is what's happening anyway.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Needs work

looks good, thanks for moving this here

+++ b/core/tests/Drupal/Tests/Component/Utility/VariableTest.php
@@ -0,0 +1,114 @@
+      array( // non-associative.
...
+      array( // associative.
...
+      array( // 2 backslashes. \\
...
+      array( // Double-quote "
...
+      array( // Single-quote '
...
+      array( // stdClass
...
+      array( // not stdClass

lets just move those inline comments to a separate line and its good to go

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review
FileSize
2163245_3.patch2.86 KB

Moved the comments, plus a few other superficial coding standards changes.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Reviewed & tested by the community

thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/tests/Drupal/Tests/Component/Utility/VariableTest.php
@@ -0,0 +1,124 @@
+/**
+ * Test unicode handling features implemented in Unicode component.
+ *
+ * @see \Drupal\Component\Utility\Unicode
+ *
+ * @group Drupal
+ * @group Component
+ * @group Variable
+ */
+class VariableTest extends UnitTestCase {

Unicode?

And can we get some @covers annotations too?

+++ b/core/tests/Drupal/Tests/Component/Utility/VariableTest.php
@@ -0,0 +1,124 @@
+   *     - (optional) Boolean indicating invalid status. Defaults to FALSE.

This is never used.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
FileSize
2163245_5.patch2.83 KB
interdiff.txt1.67 KB

How's that?

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work 
+ * @coversDefaultClass \Drupal\Component\Utility\Export

This class does not exists.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review
FileSize
2163245_9.patch2.83 KB

Doh! :-)

Changed to \Drupal\Component\Utility\Variable

Mile23 queued 9: 2163245_9.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 9: 2163245_9.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review
FileSize
2163245_12.patch2.49 KB
interdiff.txt2.06 KB

Removed getInfo(), changed non-stdClass object to be a locally-defined class in order to avoid dependency on PHPUnit's mocking system, changed @covers to reflect current standards.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/tests/Drupal/Tests/Component/Utility/VariableTest.php
@@ -0,0 +1,117 @@
+/**
+ * No-op test class for

for what? :)

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
FileSize
2163245_14.patch2.69 KB
interdiff.txt1 KB

Ewps.

Improved docs, renamed test class to be more obvious.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Cool, thank you

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 55240a0 and pushed to 8.x. Thanks!

  • alexpott committed 55240a0 on 8.x 
    Issue #2163245 by Mile23: Unit testing for \Drupal\Component\Utility\...

Status: Fixed » Closed (fixed)

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