Add unit test for AccessSubscriber

Some people follow the phpunit tag, so you potentially get more reviews on that. Thank you for writing a unit test.

1. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   + * Contains \Drupal\Core\EventSubscriber\AccessSubscriberUnitTest.
   ...
   +namespace Drupal\Tests\Core\EventSubscriber;
   

   Let's document the proper namespace.

2. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   +use Drupal\Core\EventSubscriber\AccessSubscriber;
   +use Drupal\Core\Access\AccessManager;
   +use Symfony\Component\HttpKernel\Event\GetResponseEvent;
   +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
   +use Symfony\Component\HttpFoundation\Request;
   +use Symfony\Component\HttpFoundation\ParameterBag;
   +use Symfony\Component\Routing\Route;
   +use Symfony\Cmf\Component\Routing\RouteObjectInterface;
   +use Drupal\Tests\UnitTestCase;
   

   Let's order them properly ...

3. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   + *
   + * @group System
   + */
   

   Let's also add a @group Drupal so you can run just all the drupal tests. In addition it would be nice to have a @see to the actual class, just for convenience.

4. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   +class AccessSubscriberUnitTest extends UnitTestCase {
   

   If you name it like AccessSubscriberTest PhpStorm makes it really easy to switch between the class and the implementation. It is also sort of standard to name the testclass like that.

5. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   +
   +  private $event;
   +  private $request;
   +  private $parameter_bag;
   +  private $route;
   +  private $access_manager;
   

   For the sake of autocompletion it would be great to document all these with the mocked class and the actual mock class (so for example @var \Symfony\Component\HttpKernel\GetResponseEvent|PHPUnit_Framework_MockObject_MockObject

6. +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberUnitTest.php
   @@ -0,0 +1,127 @@
   +  /**
   ...
   +  public function testAccessSubscriberThrowsAccessDeniedException() {
   

   Some single line describing what is tested here would be nice. PS: I really like that you actually split up the tests as much as possible!

Perfect!

Awesome work! Thanks for the tests. One hopefully quick thing to fix:

+++ b/core/tests/Drupal/Tests/Core/EventSubscriber/AccessSubscriberTest.php
@@ -0,0 +1,162 @@
+  /**
+   * Tests that AccessSubscriber does not alter the request if AccessManager grants access.
+   */
+  public function testAccessSubscriberDoesNotAlterRequestIfAccessManagerGrantsAccess() {

This doc is just a restatement of what the method name is... while that's true of the method above it as well, that one's easier to parse out what it's actually doing.

Could we get a quick line of english here that explains what this test is testing? Is it something like "Tests that if access is granted, no further access checks are done"?

You don't have to check explicit that an exception is thrown. If you have an uncatched exception
phpunit will complain.

Try to always provide an interdiff: https://drupal.org/documentation/git/interdiff
This makes it easier for other people to follow your changes.

Let's git it in now.

14: 2099239-accesssubscriber-test-14.patch queued for re-testing.

Committed and pushed to 8.x. Thanks!