Task to expand phpunit test coverage for \Drupal\Component\Utility\NestedArray to 100%.

See #1938068: Convert UnitTestBase to PHPUnit.

CommentFileSizeAuthor
#10 nestedarray-phpunit-2050535-10.patch924 bytesjhedstrom
#5 interdiff.txt924 bytesjhedstrom
#5 nestedarray-phpunit-2050535-05.patch3.43 KBjhedstrom
#1 nestedarray-phpunit-2050535-01.patch3.88 KBjhedstrom

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Active » Needs review
StatusFileSize
new nestedarray-phpunit-2050535-01.patch3.88 KB

This patch gets 100% coverage. I also cleaned up the function declarations, and moved the test into the correct directory.

Status: Needs review » Needs work

The last submitted patch, nestedarray-phpunit-2050535-01.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

This looks pretty good, but I don't know yet why this is failing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Ah, I bet it's the test that looks for an exception that's causing this:

PHP Fatal error:  Cannot create references to/from string offsets nor overloaded objects in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Component/Utility/NestedArray.php on line 158
Random data generation tests 7 passes, 0 fails, and 0 exceptions
+  /**
+   * Tests setting that an exception is thrown without force.
+   *
+   * @expectedException Exception
+   */
+  public function testSetValueException() {
+    $new_value = array(
+      'one',
+    );
+    $this->form['details']['non-array-parent'] = 'string';
+    $parents = array('details', 'non-array-parent', 'child');
+    // Without force, this should throw an exception.
+    NestedArray::setValue($this->form, $parents, $new_value);
+  }
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new nestedarray-phpunit-2050535-05.patch3.43 KB
new interdiff.txt924 bytes

That exception test isn't strictly necessary, it was basically testing default php behavior.

dawehner’s picture

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

Ah I see.

alexpott’s picture

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

Committed 7ec526e and pushed to 8.x. Thanks!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Fixed » Needs work

Since this is such a quick issue so far, can we get a follow-up for the following, instead of a new issue? Thanks!

+++ b/core/tests/Drupal/Tests/Component/Utility/NestedArrayTest.php
@@ -144,5 +157,7 @@ function testMergeDeepArray() {
+    $this->assertSame(NestedArray::mergeDeep($link_options_1, $link_options_2), $expected, 'NestedArray::mergeDeepArray() returned a properly merged array.');

The message uses the wrong method name.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Novice

add tag.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new nestedarray-phpunit-2050535-10.patch924 bytes

Patch for #8.

dawehner’s picture

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

Seems valid.

alexpott’s picture

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

Committed b0c8255 and pushed to 8.x. Thanks!

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

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Issue summary: View changes
Parent issue: » #1938068: Convert UnitTestBase to PHPUnit