One test to rule them all,
One test to find them,
one test to bring them all and in the darkness of the testbot bind them.

I'm a bit feared that the global time continuum breaks if this code is executed.
Someone else should decide whether this code could/should be executed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Status: Needs review » Needs work

The last submitted patch, one-test.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
1.62 KB

Setting to needs review so you can see it explode

aspilicious’s picture

Status: Needs review » Needs work

What I found in the original patch. I changed some of that in my.

+++ b/lib/Drupal/views/Tests/Handler/HandlerAllTest.phpundefined
@@ -0,0 +1,55 @@
+    foreach (views_fetch_base_tables() as $base_table) {

should be $base_table =>$info (or something like that)

+++ b/lib/Drupal/views/Tests/Handler/HandlerAllTest.phpundefined
@@ -0,0 +1,55 @@
+              $view->addItem('default', $type, $field);

This is lacking the $base_table. AND default is not always a valid plugin ID is it?

dawehner’s picture

You are so right, let's try this one.

dawehner’s picture

Status: Needs work » Needs review

.

aspilicious’s picture

Status: Needs review » Needs work
+++ b/lib/Drupal/views/Tests/Handler/HandlerAllTest.phpundefined
@@ -0,0 +1,57 @@
+              $view->addItem('default', $type, $field);

Is still going to fail... It needs 4 items

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

This one just produces an usual exception ;)

Status: Needs review » Needs work

The last submitted patch, one_test_rule_them_all-8.patch, failed testing.

dawehner’s picture

dawehner’s picture

Status: Needs work » Needs review

As this got in let's retest it.

aspilicious’s picture

Issue tags: -VDC

#8: one_test_rule_them_all-8.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, one_test_rule_them_all-8.patch, failed testing.

aspilicious’s picture

+++ b/lib/Drupal/views/Tests/Handler/HandlerAllTest.phpundefined
@@ -0,0 +1,57 @@
+    $object_types = array_keys(View::viewsObjectTypes());

Offcourse this fails ;)

viewsObjectTypes...

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

He!

Status: Needs review » Needs work

The last submitted patch, one_test_rule_them_all-8.patch, failed testing.

aspilicious’s picture

I did some serious debugging and I think this test catches a couple of bugs.
I used the "debug" script on the bottom.

Everything goes wrong in $view->execute.
Something is wrong with the filter plugins.
In my test I changed:

if (isset($field_info[$type]['id'])) {
to
if (isset($field_info[$type]['id']) && $type != 'filter') {

That makes the system happy but some other errors appear (with base table: users)

1) 'Missing handler: users langcode argument'
2) Undefined offset: 1 Notice GroupwiseMax.php 210 Drupal\views\Plugin\views\relationship\GroupwiseMax->left_query()
3) 'Missing handler: sort'

It's posisble they are all related

      debug('New loop');
      debug('Base table: ' . $base_table);

      $view = entity_create('view', array('base_table' => $base_table));

      // Go through all fields and there through all handler types.
      foreach ($info as $field => $field_info) {
        // Table is a reserved key for the metainformation.
        if ($field != 'table') {
          foreach ($object_types as $type) {
            if (isset($field_info[$type]['id']) && $type != 'filter') {
              debug('type and field: ' . $type . ' and ' . $field);
              $view->addItem('default', $type, $base_table, $field);
            }
          }
        }
      }

      debug('done adding');
aspilicious’s picture

Ok one bug found:

  // language
  $data['users']['language']['moved to'] = array('users', 'langcode');
  $data['users']['langcode'] = array(
    'title' => t('Language'), // The item it appears as on the UI,
    'help' => t('Language of the user'),
    'field' => array(
      'id' => 'user_language',
      'click sortable' => TRUE,
    ),
    'sort' => array(
      'id' => 'standard',
    ),
    'filter' => array(
      'id' => 'node_language',
    ),
    'argument' => array(
      'id' => 'node_language',
    ),
  );

The node_language argument plugin doesn't exist.
That line is added in: http://drupalcode.org/project/views.git/commitdiff/5203f9c5ddf2e5703a97e...
But I'm not sure I understand why it has to be there if there is no actual plugin handling it...

aspilicious’s picture

2) and 3) are related to the same bug. That bug is caused by this piece of code in users.views.inc

  // uid
  $data['users']['uid_representative'] = array(
    'relationship' => array(
      'title' => t('Representative node'),
      'label'  => t('Representative node'),
      'help' => t('Obtains a single representative node for each user, according to a chosen sort criterion.'),
      'id' => 'groupwise_max',
      'relationship field' => 'uid',
      'outer field' => 'users.uid',
      'argument table' => 'users',
      'argument field' =>  'uid',
      'base'   => 'node',
      'field'  => 'nid',
    ),
  );

In "function left_query($options) {" the printed $options array equals

array (
  'id' => 'uid_representative',
  'table' => 'users',
  'field' => 'uid_representative',
  'relationship' => 'none',
  'group_type' => 'group',
  'admin_label' => '',
  'label' => 'Representative node',
  'required' => false,
  'subquery_sort' => NULL,
  'subquery_order' => 'DESC',
  'subquery_regenerate' => false,
  'subquery_view' => false,
  'subquery_namespace' => false,
)

subquery sort is NULL which is bad as we do:

      // Add the sort from the options to the default display.
      // This is broken, in that the sort order field also gets added as a
      // select field. See http://drupal.org/node/844910.
      // We work around this further down.
      $sort = $options['subquery_sort'];
      list($sort_table, $sort_field) = explode('.', $sort);

I hope this enough info :)

dawehner’s picture

Status: Needs work » Needs review
Issue tags: -VDC

#15: one_test_rule_them_all-8.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, one_test_rule_them_all-8.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.99 KB

Hacked around the fact that INOperator filters require some way of valid input, even this maybe be a real bug.
If there is no input there should be nothing adding to the query.

Status: Needs review » Needs work

The last submitted patch, views-1777194-21.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
3.39 KB

Status: Needs review » Needs work

The last submitted patch, views-1777194-23.patch, failed testing.

dawehner’s picture

dawehner’s picture

Status: Needs work » Needs review
Issue tags: -VDC

#24: views-1777194-23.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, views-1777194-23.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#24: views-1777194-23.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, views-1777194-23.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +VDC

#24: views-1777194-23.patch queued for re-testing.

dawehner’s picture

FileSize
3.37 KB

Rerole the patch without the debug message.

aspilicious’s picture

For me this is rtbc but dawehner wants input from the others. As this is a duplicate of #1754206: Add one test that enables all modules and adds all filters Tim must be into this as well. I ran these tests locally and they go lightning fast so I don't see any performance issues on this one.

I'll rtbc this in 7 days if no one else did it.

aspilicious’s picture

Issue tags: -VDC

#32: views-1777194-32.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, views-1777194-32.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
3.39 KB

Rereole even something else is broken.

Status: Needs review » Needs work

The last submitted patch, views-1777194-36.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
4.71 KB

Let's try this one.

Status: Needs review » Needs work

The last submitted patch, views-1777194-38.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
7.33 KB

We really should have got this one in earlier :( In general this will still not run

Status: Needs review » Needs work

The last submitted patch, views-1777194-40.patch, failed testing.

Lars Toomre’s picture

Small nit from reading proposed patch...

+++ b/lib/Drupal/views/Plugin/views/relationship/GroupwiseMax.phpundefined
@@ -231,7 +233,7 @@ class GroupwiseMax extends RelationshipPluginBase {
       'argument',
-      $this->definition['argument table'], // eg 'term_node',
+      $this->definition['argument table'], // eg 'taxonomy_index',
       $this->definition['argument field'] //  eg 'tid'

Simple question... Does core documentation allow for in-line comments at the end of a line? I thought that the comment was required to go on the line above, ideally be a sentence and end in a period.

Also, my understanding is that 'eg' should be written as 'e.g.'.

dawehner’s picture

Assigned: Unassigned » tim.plunkett
Status: Needs work » Needs review
FileSize
9.54 KB

We should get this in, to find more bugs, before they get committed.

dawehner’s picture

FileSize
6.31 KB

Let's remove the fixes for the groupwise handler and fix it there: #1799040: Fixed groupwise max relationship

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Let's do this

damiankloip’s picture

This looks good, a nice partner to the PluginInstanceTests!

dawehner’s picture

FileSize
739 bytes
6.63 KB

Just had an idea to check for the class ... but this adds 360 additional assertions.

damiankloip’s picture

Even so, I think this is still a good idea. Tests should maybe have some assertions?! :)

tim.plunkett’s picture

#47: views-1777194-47.patch queued for re-testing.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Status: Reviewed & tested by the community » Fixed

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