#1754216: Meta: Add one test that tests each plugin type intensive

CommentFileSizeAuthor
#8 1754248-8.patch7.43 KBdamiankloip
#5 1754248-5.patch7.32 KBdamiankloip
#2 views-1754248.patch6.8 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: Unassigned » damiankloip

Assigning to me, I will look at this in the morning on my train.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Active » Needs review
FileSize
views-1754248.patch6.8 KB

Delayed, but here are some tests I did yesterday. Modified for the new views test module structure.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: damiankloip » Unassigned

Sorry, removing me, so people actually look :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,134 @@
+    $plugin = views_get_plugin('filter', 'test_filter');
+    $this->assertTrue($plugin instanceof FilterPlugin, 'Test filter plugin found.');

I see what you talked about before, maybe this would make sense to be moved to a general test for all plugin types, but just maybe.

The rest of the patch looks great.

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,134 @@
+  protected function viewsData() {

Just nitpicking, this should have a documentation that there is an override

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,134 @@
+    $this->assertTrue(!empty($view->query->where));

Shouldn't we check even more then just the fact that it got added?

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,134 @@
+    $plugin = views_get_plugin('filter', 'test_filter');
+    $this->assertTrue($plugin instanceof FilterPlugin, 'Test filter plugin found.');

I see what you talked about before, maybe this would make sense to be moved to a general test for all plugin types, but just maybe.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
1754248-5.patch7.32 KB

Thanks, updated patch.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Needs work

This looks awesome!

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,144 @@
+    $this->assertTrue($where[0]['conditions'][0]['field'] == 'views_test.name', 'Where condition field matches');
+    $this->assertTrue($where[0]['conditions'][0]['value'] == 'John', 'Where condition value matches');
+    $this->assertTrue($where[0]['conditions'][0]['operator'] == '=', 'Where condition operator matches');

These should use assertIdentical

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,144 @@
+    // Check that our operator and value match on the filter.
+    $this->assertEqual($view->filter['test_filter']->operator, '=');
+    $this->assertEqual($view->filter['test_filter']->value, 'John');

These should use assertIdentical

+++ b/lib/Drupal/views/Tests/Plugin/FilterTest.phpundefined
@@ -0,0 +1,144 @@
+    // Check that our operator and value match on the filter.
+    $this->assertEqual($view->filter['test_filter']->operator, "<>");
+    $this->assertEqual($view->filter['test_filter']->value, "John");

These should use assertIdentical, and single quotes around John

+++ b/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/filter/FilterTest.phpundefined
@@ -0,0 +1,59 @@
+ *   base = {"node"},

What's with the {}? Haven't seen that

+++ b/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/filter/FilterTest.phpundefined
@@ -0,0 +1,59 @@
+  public function query() {

Missing a docblock

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: Unassigned » damiankloip

All looks reasonable, I'll post a new patch in a few hours.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: damiankloip » Unassigned
Status: Needs work » Needs review
FileSize
1754248-8.patch7.43 KB

Here are all of tim.plunkett's comments too.

dawehner’s picture

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

And go!

dawehner’s picture

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

So it's committed now

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