Add a unit test for View::getPluginTypes() [#1784072]

Comment	File	Size	Author
#11	views-1784072-11.patch	1.32 KB	fastangel

#9	views-1784072-9.patch	1.43 KB	fastangel

#7	views-1784072-7.patch	1.4 KB	fastangel

#5	views-1784072-5.patch	1.57 KB	fastangel

Comment #1

xjm

she/her

English

CreditAttribution: xjm commented 13 September 2012 at 21:34

Issue tags:

+Novice

Nice novice task. If any of tim.plunkett, dawehner, damiankloip, or aspilicious do this, I will tease you mercilessly.

Log in or register to post comments

Comment #2

damiankloip CreditAttribution: damiankloip commented 24 September 2012 at 08:36

Must..try...to...resist.... Need... more...views..tests...

Log in or register to post comments

Comment #3

fastangel CreditAttribution: fastangel commented 10 October 2012 at 20:30

I done this unit Test in #1783692-4: Test the bundle manager to show the fail on update.php

Log in or register to post comments

Comment #4

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 10 October 2012 at 20:38

That's a WebTest, not a UnitTest. This should still be done separately.

Log in or register to post comments

Comment #5

fastangel CreditAttribution: fastangel commented 11 October 2012 at 19:10

Status:

Active

» Needs review

File	Size
views-1784072-5.patch	1.57 KB

I attached the path with the test.

Log in or register to post comments

Comment #6

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 11 October 2012 at 19:28

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,68 @@
+namespace Drupal\views\Tests;
+use Drupal\views\ViewExecutable;

Need space between lines

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,68 @@
+use Drupal\simpletest\WebTestBase;
...
+class PluginWebTest extends WebTestBase {

This should be UnitTestBase

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,68 @@
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('views');
...
+  protected function setUp() {
+    parent::setUp();
+    $this->pluginTypes = ViewExecutable::getPluginTypes();
+  }
+  /**
+   * Tests the plugins list is correct.

Do we need this in setUp? I think we should just use the method in the class.

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,68 @@
+    $diff = array_diff($plugin_list, $this->pluginTypes);
+    $this->assertTrue(empty($diff), 'The plugin list is correct'); ¶
+  }

Needs a blank line before the final }, after the method

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,68 @@
+    $this->assertTrue(empty($diff), 'The plugin list is correct'); ¶

Trailing whitespace

Log in or register to post comments

Comment #7

fastangel CreditAttribution: fastangel commented 11 October 2012 at 19:56

File	Size
views-1784072-7.patch	1.4 KB

New patch with all changes.

Log in or register to post comments

Comment #8

damiankloip CreditAttribution: damiankloip commented 17 October 2012 at 14:36

Status:

Needs review

» Needs work

Patch and code style is looking good. Just a couple o' things...

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,61 @@
+class PluginWebTest extends UnitTestBase {

Not sure I like the name PluginWebTest, and none of our other tests use this naming. Maybe we could go with PluginTypeTest or PluginTypeListTest maybe? I think I would favour the latter, as we also have other tests testing plugin types.

+++ b/lib/Drupal/views/Tests/PluginWebTest.phpundefined
@@ -0,0 +1,61 @@
+    );
+    $diff = array_diff($plugin_list, ViewExecutable::getPluginTypes());

I usually like a space in between stuff like this. Really nitpicky I know!

Log in or register to post comments

Comment #9

fastangel CreditAttribution: fastangel commented 17 October 2012 at 15:30

Status:

Needs work

» Needs review

File	Size
views-1784072-9.patch	1.43 KB

Done. Change the name to PluginTypeListTest and modified the space.

Log in or register to post comments

Comment #10

dawehner

German

CreditAttribution: dawehner commented 17 October 2012 at 16:40

Status:

Needs review

» Needs work

+++ b/lib/Drupal/views/Tests/PluginTypeListTest.phpundefined
@@ -0,0 +1,62 @@
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('views');

You don't need this part, as the module will not be enabled at all.

The rest looks perfect.