Issue #2298667 by Mile23, neclimdul: Fixed Code review for report generation.

core/modules/migrate_drupal/tests/src/source/d6/BlockTest.php

@@ -82,13 +82,6 @@ protected function setUp() {
     parent::setUp();
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function testRetrieval() {
-    // FakeSelect does not support multiple source identifiers, can not test.
-  }
-
 }
 
 namespace Drupal\migrate_drupal\Tests\source\d6;

core/modules/migrate_drupal/tests/src/source/d6/FieldInstanceTest.php

@@ -94,13 +94,6 @@ protected function setUp() {
     parent::setUp();
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function testRetrieval() {
-    // FakeSelect does not support multiple source identifiers, can not test.
-  }
-
 }
 namespace Drupal\migrate_drupal\Tests\source\d6;
 

core/modules/migrate_drupal/tests/src/source/d6/NodeRevisionTest.php

@@ -329,7 +329,8 @@ protected function setUp() {
    * {@inheritdoc}
    */
   public function testRetrieval() {
-    // FakeSelect does not support multiple source identifiers, can not test.
+    // @todo: Fix this as per https://www.drupal.org/node/2299795
+    $this->markTestIncomplete('FakeSelect does not support multiple source identifiers, can not test.');
   }
 
 }

core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php

@@ -13,7 +13,7 @@
 use Drupal\Tests\UnitTestCase;
 
 /**
- * @coversDefaultClass \Drupal\Component\Utility\Url
+ * @coversDefaultClass \Drupal\Component\Utility\UrlHelper
  * @group Utility
  */
 class UrlHelperTest extends UnitTestCase {

core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php

@@ -264,7 +264,8 @@ public function testPreSaveRevision() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
     $record = new \stdClass();
-    $this->entity->preSaveRevision($storage, $record);
+    // Our mocked entity->preSaveRevision() returns NULL, so assert that.
+    $this->assertNull($this->entity->preSaveRevision($storage, $record));
   }
 
   /**
@@ -352,7 +353,8 @@ public function testGetParent() {
   public function testSetContext() {
     $name = $this->randomName();
     $parent = $this->getMock('\Drupal\Core\TypedData\TypedDataInterface');
-    $this->entity->setContext($name, $parent);
+    // Our mocked entity->setContext() returns NULL, so assert that.
+    $this->assertNull($this->entity->setContext($name, $parent));
   }
 
   /**

core/tests/Drupal/Tests/Core/Entity/EntityResolverManagerTest.php

@@ -60,7 +60,6 @@ class EntityResolverManagerTest extends UnitTestCase {
    * {@inheritdoc}
    *
    * @covers ::__construct()
-   * @covers ::setContainer()
    */
   protected function setUp() {
     $this->entityManager = $this->getMock('Drupal\Core\Entity\EntityManagerInterface');

core/tests/Drupal/Tests/Core/Entity/EntityUnitTest.php

@@ -530,7 +530,8 @@ public function testGetEntityTypeId() {
   public function testPreSave() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
-    $this->entity->preSave($storage);
+    // Our mocked entity->preSave() returns NULL, so assert that.
+    $this->assertNull($this->entity->preSave($storage));
   }
 
   /**
@@ -566,7 +567,8 @@ public function testPreCreate() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
     $values = array();
-    $this->entity->preCreate($storage, $values);
+    // Our mocked entity->preCreate() returns NULL, so assert that.
+    $this->assertNull($this->entity->preCreate($storage, $values));
   }
 
   /**
@@ -575,7 +577,8 @@ public function testPreCreate() {
   public function testPostCreate() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
-    $this->entity->postCreate($storage);
+    // Our mocked entity->postCreate() returns NULL, so assert that.
+    $this->assertNull($this->entity->postCreate($storage));
   }
 
   /**
@@ -584,7 +587,8 @@ public function testPostCreate() {
   public function testPreDelete() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
-    $this->entity->preDelete($storage, array($this->entity));
+    // Our mocked entity->preDelete() returns NULL, so assert that.
+    $this->assertNull($this->entity->preDelete($storage, array($this->entity)));
   }
 
   /**
@@ -617,7 +621,8 @@ public function testPostLoad() {
     // This method is internal, so check for errors on calling it only.
     $storage = $this->getMock('\Drupal\Core\Entity\EntityStorageInterface');
     $entities = array($this->entity);
-    $this->entity->postLoad($storage, $entities);
+    // Our mocked entity->postLoad() returns NULL, so assert that.
+    $this->assertNull($this->entity->postLoad($storage, $entities));
   }
 
   /**

core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php

@@ -91,9 +91,9 @@ public function setUp() {
   }
 
   /**
-   * Tests access denied throws a Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException exception.
+   * Tests access denied throws an exception.
    *
-   * @expectedException Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+   * @expectedException \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    */
   public function testAccessSubscriberThrowsAccessDeniedException() {
 
@@ -132,25 +132,27 @@ public function testAccessSubscriberOnlyChecksForRequestsWithRouteObject() {
   }
 
   /**
-   * Tests that if access is granted, AccessSubscriber will not throw a Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException exception.
+   * Tests that if access is granted, AccessSubscriber will not throw an exception.
    */
   public function testAccessSubscriberDoesNotAlterRequestIfAccessManagerGrantsAccess() {
-    $this->parameterBag->expects($this->any())
+    $this->parameterBag->expects($this->once())
       ->method('has')
       ->with(RouteObjectInterface::ROUTE_OBJECT)
       ->will($this->returnValue(TRUE));
 
-    $this->parameterBag->expects($this->any())
+    $this->parameterBag->expects($this->once())
       ->method('get')
       ->with(RouteObjectInterface::ROUTE_OBJECT)
       ->will($this->returnValue($this->route));
 
-    $this->accessManager->expects($this->any())
+    $this->accessManager->expects($this->once())
       ->method('check')
-      ->with($this->anything())
+      ->with($this->equalTo($this->route))
       ->will($this->returnValue(TRUE));
 
     $subscriber = new AccessSubscriber($this->accessManager, $this->currentUser);
+    // We're testing that no exception is thrown in this case. There is no
+    // return.
     $subscriber->onKernelRequestAccessCheck($this->event);
   }
 

core/tests/Drupal/Tests/Core/EventSubscriber/ModuleRouteSubscriberTest.php

@@ -42,7 +42,8 @@ public function setUp() {
   /**
    * Tests that removeRoute() removes routes when the module is not enabled.
    *
-   * @dataProvider testRemoveRouteProvider
+   * @dataProvider providerTestRemoveRoute
+   * @covers ::onAlterRoutes
    *
    * @param string $route_name
    *   The machine name for the route.
@@ -71,7 +72,7 @@ public function testRemoveRoute($route_name, array $requirements, $removed) {
   /**
    * Data provider for testRemoveRoute().
    */
-  public function testRemoveRouteProvider() {
+  public function providerTestRemoveRoute() {
     return array(
       array('enabled', array('_module_dependencies' => 'enabled'), FALSE),
       array('disabled', array('_module_dependencies' => 'disabled'), TRUE),