If you call update.php at the moment you fail with a message like

Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "plugin.manager.views.display" does not exist. in Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition() (line 691 of /var/www/d8/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).

let's try to reproduce this in a test.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
2.11 KB

This just works if you reset the container (i'm not sure whether this is actually the right way to do that).

Status: Needs review » Needs work

The last submitted patch, views-1783692-1.patch, failed testing.

xjm’s picture

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -19,27 +20,7 @@ class PluginInstanceTest extends ViewTestBase {
-  protected $pluginTypes = array(
-    'access',
-    'area',
-    'argument',
-    'argument_default',
-    'argument_validator',
-    'cache',
-    'display_extender',
-    'display',
-    'exposed_form',
-    'field',
-    'filter',
-    'join',
-    'pager',
-    'query',
-    'relationship',
-    'row',
-    'sort',
-    'style',
-    'wizard',
-  );
+  protected $pluginTypes;

@@ -61,6 +42,7 @@ class PluginInstanceTest extends ViewTestBase {
+    $this->pluginTypes = View::getPluginTypes();

If we're going to remove the expected list of the correct plugin types and replace it with View::getPluginTypes(), let's also add a unit test for that method that checks the expected list. Tim filed #1784072: Add a unit test for View::getPluginTypes().

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -87,7 +69,7 @@ class PluginInstanceTest extends ViewTestBase {
-    $container = drupal_container();
+    $container = drupal_container(NULL, TRUE);

Should add a comment for this (something like "Inject a fresh container for testing to reduce over." or something). Whatever is accurate.

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -106,4 +88,17 @@ class PluginInstanceTest extends ViewTestBase {
+   * Tests the views bundle after clearing all caches.

Tests what about the view bundle, precisely? (That it's not broken nor obliterated from the container, but... what specifically? Let's add some detail.)

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -106,4 +88,17 @@ class PluginInstanceTest extends ViewTestBase {
+    drupal_flush_all_caches();
+    $container = drupal_container();

So the idea here is that the container is getting reset when the cache is flushed. So, let's say something about, "Do not explicitly rebuild the container; instead, use the container built during the cache clear." above the drupal_container() call.

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -106,4 +88,17 @@ class PluginInstanceTest extends ViewTestBase {
+      $this->assertTrue($manager instanceof \Drupal\views\Plugin\Type\ViewsPluginManager);

Let's add a comment and assertion message here. Value true is TRUE isn't the most informative message in the world :) and what we're checking is that the stuff is still available from the container.

fastangel’s picture

Status: Needs work » Needs review
FileSize
2.72 KB

Hi,

I attached one patch with test #1784072: Add a unit test for View::getPluginTypes() and other changes:
Drupal\views\ViewExecutable
Is need this change.
ViewExecutable::getPluginTypes();
Now is need call viewExecutable... instead Views.
$this->assertTrue($manager instanceof \Drupal\views\Plugin\Type\PluginManager);
The type of manager now is PluginManager.

Regards.

tim.plunkett’s picture

Status: Needs review » Needs work

Thanks for working on this @fastangel!

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -86,7 +68,7 @@ class PluginInstanceTest extends ViewTestBase {
-    $container = drupal_container();
+    $container = drupal_container(NULL, TRUE);

This should have a comment.

EDIT: This was pointed out in #3, it looks like none of that was addressed.

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -105,4 +87,45 @@ class PluginInstanceTest extends ViewTestBase {
+      $this->assertTrue($manager instanceof \Drupal\views\Plugin\Type\PluginManager);

instanceof respects use statements, you can add that to the top and just have instanceof PluginManager

#1784072: Add a unit test for View::getPluginTypes() should still happen on its own as this class is not a unit test.

fastangel’s picture

Status: Needs work » Needs review
FileSize
2.19 KB

Ok. Attached new patch.

dawehner’s picture

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -86,7 +69,8 @@ class PluginInstanceTest extends ViewTestBase {
+    //Inject a fresh container for testing to reduce over.

Maybe i'm not into that topic/english language, but i don't get what "to reduce over" means here.

+++ b/lib/Drupal/views/Tests/PluginInstanceTest.phpundefined
@@ -105,4 +89,16 @@ class PluginInstanceTest extends ViewTestBase {
+  public function testClearCache() {

I really like this test function, simple as it is.

tim.plunkett’s picture

Status: Needs review » Needs work

The last submitted patch, views-1783692-6.patch, failed testing.

fastangel’s picture

Status: Needs work » Needs review
FileSize
2.55 KB

Ok. I attached a new patch. In this new patch I changed the description of drupal_container and fixed testClearCache with the special case join and wizar. I talked with @dawehner__ for find one solution for these cases.

NOTE: Only I get this error in tests Symfony\Component\DependencyInjection\Exception\InvalidArgumentException: The service definition "plugin.manager.views.access" does not exist. in Symfony\Component\DependencyInjection\ContainerBuilder->getDefinition() (line 690 of /Users/jacintocapote/Sites/drupal8/drupal/core/vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/ContainerBuilder.php).

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

The last submitted patch, views-1783692-9.patch, failed testing.

fastangel’s picture

Status: Needs work » Needs review

#10: views-1783692-9.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, views-1783692-9.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

#10: views-1783692-9.patch queued for re-testing.

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

The last submitted patch, views-1783692-9.patch, failed testing.

fastangel’s picture

dawehner’s picture

Do we still require this to be opened, it's clear that not views is the problem.

dawehner’s picture

Status: Needs work » Fixed

We have proven that

Status: Fixed » Closed (fixed)

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

xjm’s picture

Status: Closed (fixed) » Closed (duplicate)

More correct status.