The LocalTaskManagerTest is only working because of:

    $this->manager = $this
      ->getMockBuilder('Drupal\Core\Menu\LocalTaskManager')
      ->disableOriginalConstructor()
      ->setMethods(array('enforcePluginInterface'))
      ->getMock();

Which means the only method we are mocking is the non existing enforcePluginInterface method.

Let's just create a proper LocalTaskManager to test.

CommentFileSizeAuthor
d8.local-task-manager-test.patch5.29 KBalexpott

Comments

dawehner’s picture

dawehner
Primary language German
commented 
  1. +++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
@@ -89,7 +90,7 @@ class LocalTaskManagerTest extends UnitTestCase {
-    $this->controllerResolver = $this->getMock('Symfony\Component\HttpKernel\Controller\ControllerResolverInterface');
+    $this->controllerResolver = $this->getMock('Drupal\Core\Controller\ControllerResolverInterface');

    I don't really get why its typehinted in the main code. We don't use getControllerFromDefinition in the LocalTaskManager code 

  2. +++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
@@ -243,29 +244,19 @@ public function testGetTitle() {
-    $this->manager = $this
-      ->getMockBuilder('Drupal\Core\Menu\LocalTaskManager')
-      ->disableOriginalConstructor()
-      ->setMethods(array('enforcePluginInterface'))
-      ->getMock();
-

    Does that mean that we should NOT use the getMockBuilder for the tested class at all, but otherwise, if needed use the TestFoo approach? 

  3. +++ b/core/tests/Drupal/Tests/Core/Menu/LocalTaskManagerTest.php
@@ -243,29 +244,19 @@ public function testGetTitle() {
-    $property = new \ReflectionProperty('Drupal\Core\Menu\LocalTaskManager', 'controllerResolver');
-    $property->setAccessible(TRUE);
-    $property->setValue($this->manager, $this->controllerResolver);

    +1 for getting rid of as many as possible ugly setAccessible calls.

dawehner’s picture

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

.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 2aa54f7 on 8.0.x 
    Issue #2359005 by alexpott: Fixed LocalTaskManagerTest only works...

Status: Fixed » Closed (fixed)

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