Merge pull request #54 from thephpleague/feature/test-cleanup

Test cleanup + instability research
thephpleague · Aug 3, 2017 · 0e30e58 · 0e30e58
2 parents db359bf + 08249ba
commit 0e30e58
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 56 deletions.
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -7,7 +7,7 @@
          convertErrorsToExceptions="true"
          convertNoticesToExceptions="true"
          convertWarningsToExceptions="true"
-         processIsolation="true"
+         beStrictAboutTestsThatDoNotTestAnything="false"
          stopOnFailure="false">
     <testsuites>
         <testsuite name="TacticianBundle Test Suite">

diff --git a/src/Security/Voter/HandleCommandVoter.php b/src/Security/Voter/HandleCommandVoter.php
@@ -25,7 +25,7 @@ class HandleCommandVoter extends Voter
      *
      * @var array
      */
-    private $commandRoleMapping;
+    private $commandRoleMapping = [];
 
     /**
      * Create a new HandleCommandVoter.

diff --git a/tests/Integration/BasicCommandAndBusMappingTest.php b/tests/Integration/BasicCommandAndBusMappingTest.php
@@ -3,8 +3,6 @@
 namespace League\Tactician\Bundle\Tests\Integration;
 
 /**
- * To ensure cache is isolated from each test.
- *
  * @runTestsInSeparateProcesses
  */
 class BasicCommandAndBusMappingTest extends IntegrationTest

diff --git a/tests/Integration/SecurityTest.php b/tests/Integration/SecurityTest.php
@@ -7,19 +7,17 @@
 use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use Symfony\Component\Security\Core\Role\Role;
+use stdClass;
 
 /**
  * Integration test for security middleware.
  *
  * @author Ron Rademaker
+ *
+ * @runTestsInSeparateProcesses
  */
 class SecurityTest extends IntegrationTest
 {
-    /**
-     * Tests if the kernel is bootable with security middleware.
-     *
-     * @return void
-     */
     public function testCanBootKernelWithSecurityMiddleware()
     {
         $this->loadSecurityConfiguration();
@@ -35,12 +33,7 @@ public function testCanBootKernelWithSecurityMiddleware()
         $this->assertTrue(true);
     }
 
-    /**
-     * Tests if the kernel is not bootable without security settings (but with security middleware).
-     *
-     * @return void
-     */
-    public function testCanNotBootKernelWithoutSecurity()
+    public function testCanNotBootKernelIfLoadingSecurityMiddlewareWithoutSecurityBeingTurnedOn()
     {
         $this->expectException(UnknownMiddlewareException::class);
         $this->givenConfig('tactician', <<<'EOF'
@@ -53,27 +46,19 @@ public function testCanNotBootKernelWithoutSecurity()
         static::$kernel->boot();
     }
 
-    /**
-     * Tests if the kernel is bootable without security middleware and without security settings.
-     */
-    public function testCanBootKernelWithoutSecurity()
+    public function testCanBootKernelWithoutSecurityOrSecurityMiddleware()
     {
         static::$kernel->boot();
         $this->assertTrue(true);
     }
 
     /**
-     * Tests security middleware.
-     *
      * @dataProvider provideTestData
-     *
-     * @param string $role
-     * @param bool $allowed
      */
-    public function testSecurityMiddleware(string $role, bool $allowed)
+    public function testSecurityMiddleware($command, string $role, string $expectedExceptionClassName = null)
     {
-        if (false === $allowed) {
-            $this->expectException(AccessDeniedException::class);
+        if ($expectedExceptionClassName) {
+            $this->expectException($expectedExceptionClassName);
         }
 
         $this->loadSecurityConfiguration();
@@ -89,10 +74,9 @@ public function testSecurityMiddleware(string $role, bool $allowed)
         );
 
         static::$kernel->boot();
-        static::$kernel->getContainer()->get('security.token_storage')->setToken(new AnonymousToken('test', 'anon', [new Role($role)]));
-        static::$kernel->getContainer()->get('tactician.commandbus.default')->handle(new FakeCommand());
+        $this->setUserRole($role);
 
-        $this->assertTrue($allowed);
+        static::$kernel->getContainer()->get('tactician.commandbus.default')->handle($command);
     }
 
     /**
@@ -103,9 +87,10 @@ public function testSecurityMiddleware(string $role, bool $allowed)
     public function provideTestData(): array
     {
         return [
-            'Role may handle the command' => ['ROLE_ADMIN', true],
-            'Test role hierarchy' => ['ROLE_SUPER_ADMIN', true],
-            'Role may not handle the command' => ['ROLE_USER', false],
+            'Role may handle the command' => [new FakeCommand(), 'ROLE_ADMIN'],
+            'Test role hierarchy' => [new FakeCommand(), 'ROLE_SUPER_ADMIN'],
+            'Role may not handle the command' => [new FakeCommand(), 'ROLE_USER', AccessDeniedException::class],
+            'Deny access if command is not in the mapping' => [new stdClass(), 'ROLE_SUPER_ADMIN', AccessDeniedException::class],
         ];
     }
 
@@ -132,4 +117,16 @@ private function loadSecurityConfiguration()
 EOF
         );
     }
+
+    /**
+     * @param string $role
+     */
+    protected function setUserRole(string $role)
+    {
+        static::$kernel->getContainer()
+            ->get('security.token_storage')
+            ->setToken(
+                new AnonymousToken('test', 'anon', [new Role($role)])
+            );
+    }
 }
diff --git a/tests/Security/Voter/HandleCommandVoterTest.php b/tests/Security/Voter/HandleCommandVoterTest.php
@@ -5,6 +5,7 @@
 use League\Tactician\Bundle\Security\Voter\HandleCommandVoter;
 use League\Tactician\Bundle\Tests\Fake\FakeCommand;
 use Mockery;
+use Mockery\MockInterface;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
@@ -18,25 +19,41 @@
 class HandleCommandVoterTest extends TestCase
 {
     /**
-     * The decision manager mock.
+     * @var AccessDecisionManager|MockInterface
      */
     private $decisionManager;
 
-    /**
-     * Set up.
-     */
     public function setUp()
     {
         $this->decisionManager = Mockery::mock(AccessDecisionManager::class);
     }
 
+    public function testAKnownCommandWillBeDelegatedToTheDecisionManager()
+    {
+        $tokenMock = Mockery::mock(TokenInterface::class);
+
+        $voter = new HandleCommandVoter($this->decisionManager, [FakeCommand::class => ['ROLE_USER']]);
+
+        $this->decisionManager
+            ->shouldReceive('decide')
+            ->with($tokenMock, ['ROLE_USER'])
+            ->andReturn(true)
+            ->once();
+
+        $this->assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($tokenMock, new FakeCommand(), ['handle']));
+    }
+
     /**
      * @dataProvider provideTestVoteData
      */
-    public function testVote($attribute, $subject, $decision, $mapping, $expected)
+    public function testAbstainOrDenyScenarios($attribute, $subject, $expected)
     {
-        $this->decisionManager->shouldReceive('decide')->andReturn($decision);
-        $voter = new HandleCommandVoter($this->decisionManager, $mapping);
+        // In the test cases provided, we either abstain from voting or refuse
+        // to do it since there's no declared mapping. Therefore, it would be
+        // an error to ever call the decision manager.
+        $this->decisionManager->shouldReceive('decide')->never();
+
+        $voter = new HandleCommandVoter($this->decisionManager, []);
         $tokenMock = Mockery::mock(TokenInterface::class);
 
         $this->assertEquals($expected, $voter->vote($tokenMock, $subject, [$attribute]));
@@ -50,22 +67,26 @@ public function testVote($attribute, $subject, $decision, $mapping, $expected)
     public function provideTestVoteData()
     {
         return [
-            // Testcase: default access is false
-            ['handle', new FakeCommand, true, [], VoterInterface::ACCESS_DENIED],
-            // Testcase: abstain when not handling a command, but using the handle attribute
-            ['handle', null, true, [], VoterInterface::ACCESS_ABSTAIN],
-            // Testcase: abstain when not handling a command and not using the handle attribute
-            ['create', null, true, [], VoterInterface::ACCESS_ABSTAIN],
-            // Testcase: abstain when not handling a command
-            ['create', new FakeCommand, true, [], VoterInterface::ACCESS_ABSTAIN],
-            // Testcase: default is unrelated to decision manager
-            ['handle', new FakeCommand, false, [], VoterInterface::ACCESS_DENIED],
-            // Testcase: deny access if decision manager returns false
-            ['handle', new FakeCommand, false, [FakeCommand::class => ['ROLE_USER']], VoterInterface::ACCESS_DENIED],
-            // Testcase: grant access if decision manager returns true and the command is in the mapping
-            ['handle', new FakeCommand, true, [FakeCommand::class => ['ROLE_USER']], VoterInterface::ACCESS_GRANTED],
-            // Testcase: deny access if the command is not in the mapping (i.e. a default deny access case)
-            ['handle', new FakeCommand, false, ['someOtherCommand' => ['ROLE_USER']], VoterInterface::ACCESS_DENIED],
+            'abstain when not handling a command, but using the handle attribute' => [
+                'handle',
+                null,
+                VoterInterface::ACCESS_ABSTAIN
+            ],
+            'abstain when not handling a command and not using the handle attribute' => [
+                'create',
+                null,
+                VoterInterface::ACCESS_ABSTAIN
+            ],
+            'abstain when handling a command and not using the handle attribute' => [
+                'create',
+                new FakeCommand,
+                VoterInterface::ACCESS_ABSTAIN
+            ],
+            'default access is false' => [
+                'handle',
+                new FakeCommand,
+                VoterInterface::ACCESS_DENIED
+            ],
         ];
     }
 }