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

CommentFileSizeAuthor
#11 views-1754260-11.patch6.36 KBdawehner
#9 views-1754260-9.patch6.69 KBdawehner
#7 views-1754260-7.patch6.53 KBdawehner
#5 views-1754260-5.patch6.25 KBdawehner
#1 views-1754260-1.patch6.32 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new views-1754260-1.patch6.32 KB

A spin off caused by reading some code: #1770726: Simplify the code for adding a relationship form element to plugin forms

Added tests:

  • Make sure that either the base table or the base table of the relationship is added to the query
  • Make sure that the relationship option is available
  • Make sure that the row options link disappear when no option exists.
aspilicious’s picture

aspilicious commented

Generated by hand?

+tag: ''
\ No newline at end of file

dawehner’s picture

dawehner
Primary language German
commented

yes ... it's so easy nowadays :)

damiankloip’s picture

damiankloip commented
Status: Needs review » Needs work

Looks pretty good.

+++ b/lib/Drupal/views/Tests/Plugin/RowTest.phpundefined
@@ -0,0 +1,115 @@
+    $this->adminUser = $this->drupalCreateUser(array('administer views'));
+
+    $views_admin = $this->drupalCreateUser(array('administer views'));

Do we want to create two users with the same permissions here?

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new views-1754260-5.patch6.25 KB

Ups!

aspilicious’s picture

aspilicious commented
Status: Needs review » Needs work 
+++ b/tests/views_test/config/views.view.test_row_plugin.ymlundefined
@@ -0,0 +1,25 @@
+            cache:
+                type: none
+            row_plugin: test_row
+            relationships:
+                vid:

2 space yaml files

+++ b/tests/views_test/config/views.view.test_row_plugin.ymlundefined
@@ -0,0 +1,25 @@
+tag: ''
\ No newline at end of file

Newline needed

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new views-1754260-7.patch6.53 KB

Fixed the spaces and added a new test to be sure that removing all possible relationships also removes the option in the UI.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work 
+++ b/lib/Drupal/views/Tests/Plugin/RowTest.phpundefined
@@ -0,0 +1,118 @@
+ * Tests general row plugin functionality..

two periods.

+++ b/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/row/RowTest.phpundefined
@@ -0,0 +1,65 @@
+ *   base = {"node"},

I *still* don't understand these curl braces. What are they?

+++ b/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/row/RowTest.phpundefined
@@ -0,0 +1,65 @@
+  public $base_table = 'node';
+
+  public $base_field = 'nid';

Need docblocks, and should be baseTable and baseField

+++ b/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/row/RowTest.phpundefined
@@ -0,0 +1,65 @@
+    $options['test_option'] = array('default' => TRUE, 'bool' => TRUE);

Don't need the bool anymore.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new views-1754260-9.patch6.69 KB

Thanks for the review!!

I *still* don't understand these curl braces. What are they?

views_fetch_plugin_names allows you to filter plugin by base table they are written for.
You can write a plugin against multiple base tables, so arrays are used here.

function views_fetch_plugin_names($type, $key = NULL, $base = array()) {
  ...
  if (empty($plugin['no_ui']) && (empty($base) || empty($plugin['base']) || array_intersect($base, $plugin['base']))) {
Need docblocks, and should be baseTable and baseField

Fixed the docblocks, though these variables are used in RowPluginBase::query() so no change here.

Fixed the other issues as well.

dawehner’s picture

dawehner
Primary language German
commented

#9: views-1754260-9.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new views-1754260-11.patch6.36 KB

The style tests added a test row plugin in the meantime. Here is a reroll of the patch.

Status: Needs review » Needs work

The last submitted patch, views-1754260-11.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Ah i see the problem, the style test assumes that there is no field added to the query,
but the row test uses a lot of information around relationships which aren't available on views_test_data, mh ...

xjm’s picture

xjm
she/her
Primary language English
commented
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Issue summary: View changes
Status: Needs work » Fixed

Seems this is already included in core:

https://api.drupal.org/api/drupal/core!modules!views!tests!modules!views...

Status: Fixed » Closed (fixed)

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