These methods are more or less the heart of all the options of a view so they should get respect with a proper unit test :)

CommentFileSizeAuthor
#6 durpal-1849814-5.patch8.24 KBdawehner
#3 1849814-2.patch8.23 KBdamiankloip
#2 interdiff.txt2.3 KBdamiankloip
#1 drupal-1849814-1.patch8.21 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new drupal-1849814-1.patch8.21 KB

I think it's okay to bypass the actual plugin system by just creating an instance of the object.

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff.txt2.3 KB

Awesome, The only thing is maybe have a helper? like this.

damiankloip’s picture

damiankloip commented
StatusFileSize
new 1849814-2.patch8.23 KB

Huh? where did that patch go!

dawehner’s picture

dawehner
Primary language German
commented

I li

+++ b/core/modules/views/lib/Drupal/views/Tests/PluginBaseUnitTest.phpundefined
@@ -0,0 +1,257 @@
+  /**
+   * Sets up and returns a basic instance of a plugin.
+   *
+   * @return \Drupal\views\Tests\TestHelperPlugin
+   *   A test plugin instance.
+   */
+  protected function getTestPlugin() {
+    $discovery = new StaticDiscovery();
+    $discovery->setDefinition('default', array());
+    return new TestHelperPlugin(array(), 'default', $discovery);

Good idea!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/modules/views/lib/Drupal/views/Tests/PluginBaseUnitTest.phpundefined
@@ -0,0 +1,257 @@
+        'key' => array('default' => 'value2')
...
+        'key' => array('default' => 'value3')
...
+        'key' => array('default' => 'value2')

Missing trailing comma

+++ b/core/modules/views/lib/Drupal/views/Tests/PluginBaseUnitTest.phpundefined
@@ -0,0 +1,257 @@
+      $this->assertEqual($parameter['storage'], $parameter['expected']);
+    }
+
+  }

Extra blank line

+++ b/core/modules/views/lib/Drupal/views/Tests/PluginBaseUnitTest.phpundefined
@@ -0,0 +1,257 @@
+    return new TestHelperPlugin(array(), 'default', $discovery);
+  }
+}
diff --git a/core/modules/views/lib/Drupal/views/Tests/TestHelperPlugin.php b/core/modules/views/lib/Drupal/views/Tests/TestHelperPlugin.php

Missing a blank line before the end of the class

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new durpal-1849814-5.patch8.24 KB

Thank you for the review, fixed those.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

Unit tests++

Looks good!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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