minor #66 Github actions setup and test fixes (acrobat)

This PR was merged into the 3.0-dev branch.

Discussion
----------

Github actions setup and test fixes

Fixes #59 and closes #61

- This PR adds a github action setup, so that the test suite is actually executed on pr's (and pushes)
- Test fixes as there are quite some failing or risky tests

The github actions result can be viewed here before merge (https://github.com/acrobat/security-acl/actions)

There are 2 risky tests left, but I don't know what to do with them or how to fix them. The test names indicate that there should be an exception thrown but that doesn't happen and there are no asserts performed in those tests.

The tests/fix for an issue were added in symfony/symfony#9485

Commits
-------

8b94a32 Github actions setup and test fixes

.github/workflows/ci.yml

@@ -0,0 +1,43 @@
+name: CI
+
+on:
+    [push, pull_request]
+
+jobs:
+    test:
+        name: Test on PHP ${{ matrix.php }}
+        runs-on: ubuntu-latest
+
+        strategy:
+            fail-fast: false
+            matrix:
+                php: ['7.1', '7.2', '7.3', '7.4', '8.0']
+
+        steps:
+            -   uses: actions/checkout@v2
+
+            -   name: Setup PHP
+                uses: shivammathur/setup-php@v2
+                with:
+                    php-version: ${{ matrix.php }}
+                    coverage: none
+
+            -   name: Determine composer cache directory
+                id: composer-cache
+                run: echo "::set-output name=directory::$(composer config cache-dir)"
+
+            -   name: Cache composer dependencies
+                uses: actions/cache@v2
+                with:
+                    path: ${{ steps.composer-cache.outputs.directory }}
+                    key: ${{ matrix.php }}-composer-${{ hashFiles('**/composer.lock') }}
+                    restore-keys: ${{ matrix.php }}-composer-
+
+            -   name: Install dependencies
+                run: composer update --no-progress --ansi
+
+            -   name: Install simple-phpunit
+                run: vendor/bin/simple-phpunit install
+
+            -   name: Run phpunit
+                run: vendor/bin/simple-phpunit

.gitignore

@@ -1,3 +1,4 @@
 vendor/
 composer.lock
 phpunit.xml
+.phpunit.result.cache

Tests/Dbal/MutableAclProviderTest.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Security\Acl\Tests\Dbal;
 
+use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\DriverManager;
 use Symfony\Component\Security\Acl\Dbal\AclProvider;
 use Symfony\Component\Security\Acl\Dbal\MutableAclProvider;
@@ -103,11 +104,10 @@ public function testDeleteAclDeletesChildren()
         $provider->updateAcl($acl);
         $provider->deleteAcl($parentAcl->getObjectIdentity());
 
-        try {
-            $provider->findAcl(new ObjectIdentity(1, 'Foo'));
-            $this->fail('Child-ACLs have not been deleted.');
-        } catch (AclNotFoundException $e) {
-        }
+        $this->expectException(AclNotFoundException::class);
+        $this->expectExceptionMessage('There is no ACL for the given object identity.');
+
+        $provider->findAcl(new ObjectIdentity(1, 'Foo'));
     }
 
     public function testFindAclsAddsPropertyListener()
@@ -252,7 +252,8 @@ public function testUpdateAclDoesNotAcceptUntrackedAcls()
 
     public function testUpdateDoesNothingWhenThereAreNoChanges()
     {
-        $con = $this->getMock('Doctrine\DBAL\Connection', [], [], '', false);
+        $con = $this->createMock(Connection::class);
+
         $con
             ->expects($this->never())
             ->method('beginTransaction')
@@ -287,11 +288,11 @@ public function testUpdateAclThrowsExceptionOnConcurrentModificationOfSharedProp
 
         $acl1->insertClassAce($sid, 3);
         $acl2->insertClassAce($sid, 5);
-        try {
-            $provider->updateAcl($acl1);
-            $this->fail('Provider failed to detect a concurrent modification.');
-        } catch (ConcurrentModificationException $e) {
-        }
+
+        $this->expectException(ConcurrentModificationException::class);
+        $this->expectExceptionMessage('The "classAces" property has been modified concurrently.');
+
+        $provider->updateAcl($acl1);
     }
 
     public function testUpdateAcl()

Tests/Domain/AclTest.php

@@ -16,6 +16,7 @@
 use Symfony\Component\Security\Acl\Domain\PermissionGrantingStrategy;
 use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity;
 use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
+use Doctrine\Persistence\PropertyChangedListener;
 
 class AclTest extends \PHPUnit\Framework\TestCase
 {
@@ -497,20 +498,21 @@ protected function getListener($expectedChanges)
     {
         $aceProperties = ['aceOrder', 'mask', 'strategy', 'auditSuccess', 'auditFailure'];
 
-        $listener = $this->createMock('Doctrine\Persistence\PropertyChangedListener;');
+        $arguments = [];
+        $listener = $this->createMock(PropertyChangedListener::class);
         foreach ($expectedChanges as $index => $property) {
             if (\in_array($property, $aceProperties)) {
                 $class = 'Symfony\Component\Security\Acl\Domain\Entry';
             } else {
                 $class = 'Symfony\Component\Security\Acl\Domain\Acl';
             }
 
-            $listener
-                ->expects($this->at($index))
-                ->method('propertyChanged')
-                ->with($this->isInstanceOf($class), $this->equalTo($property))
-            ;
+            $arguments[] = [$this->isInstanceOf($class), $this->equalTo($property)];
         }
+        $listener
+            ->method('propertyChanged')
+            ->withConsecutive(...$arguments)
+        ;
 
         return $listener;
     }

Tests/Domain/ObjectIdentityTest.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Acl\Tests\Domain
 {
     use Symfony\Component\Security\Acl\Domain\ObjectIdentity;
+    use Symfony\Component\Security\Acl\Model\DomainObjectInterface;
 
     class ObjectIdentityTest extends \PHPUnit\Framework\TestCase
     {
@@ -34,17 +35,17 @@ public function testConstructorWithProxy()
 
         public function testFromDomainObjectPrefersInterfaceOverGetId()
         {
-            $domainObject = $this->createMock('Symfony\Component\Security\Acl\Model\DomainObjectInterface');
-            $domainObject
-                ->expects($this->once())
-                ->method('getObjectIdentifier')
-                ->willReturn('getObjectIdentifier()')
-            ;
-            $domainObject
-                ->expects($this->never())
-                ->method('getId')
-                ->willReturn('getId()')
-            ;
+            $domainObject = new class() implements DomainObjectInterface {
+                public function getObjectIdentifier()
+                {
+                    return 'getObjectIdentifier()';
+                }
+
+                public function getId()
+                {
+                    return 'getId()';
+                }
+            };
 
             $id = ObjectIdentity::fromDomainObject($domainObject);
             $this->assertEquals('getObjectIdentifier()', $id->getIdentifier());

Tests/Domain/PermissionGrantingStrategyTest.php

@@ -150,11 +150,10 @@ public function testIsGrantedStrategies($maskStrategy, $aceMask, $requiredMask,
         $acl->insertObjectAce($sid, $aceMask, 0, true, $maskStrategy);
 
         if (false === $result) {
-            try {
-                $strategy->isGranted($acl, [$requiredMask], [$sid]);
-                $this->fail('The ACE is not supposed to match.');
-            } catch (NoAceFoundException $e) {
-            }
+            $this->expectException(NoAceFoundException::class);
+            $this->expectExceptionMessage('No applicable ACE was found.');
+
+            $strategy->isGranted($acl, [$requiredMask], [$sid]);
         } else {
             $this->assertTrue($strategy->isGranted($acl, [$requiredMask], [$sid]));
         }

Tests/Domain/RoleSecurityIdentityTest.php

@@ -26,6 +26,10 @@ public function testConstructor()
 
     public function testConstructorWithRoleInstance()
     {
+        if (!class_exists(\Symfony\Component\Security\Core\Role\Role::class)) {
+            $this->markTestSkipped();
+        }
+
         $id = new RoleSecurityIdentity(new Role('ROLE_FOO'));
 
         $this->assertEquals('ROLE_FOO', $id->getRole());
@@ -43,11 +47,24 @@ public function testEquals($id1, $id2, $equal)
         }
     }
 
+    /**
+     * @group legacy
+     */
+    public function testDeprecatedRoleClassEquals()
+    {
+        if (!class_exists(Role::class)) {
+            $this->markTestSkipped();
+        }
+
+        $id1 = new RoleSecurityIdentity('ROLE_FOO');
+        $id2 = new RoleSecurityIdentity(new Role('ROLE_FOO'));
+        $this->assertTrue($id1->equals($id2));
+    }
+
     public function getCompareData()
     {
         return [
             [new RoleSecurityIdentity('ROLE_FOO'), new RoleSecurityIdentity('ROLE_FOO'), true],
-            [new RoleSecurityIdentity('ROLE_FOO'), new RoleSecurityIdentity(new Role('ROLE_FOO')), true],
             [new RoleSecurityIdentity('ROLE_USER'), new RoleSecurityIdentity('ROLE_FOO'), false],
             [new RoleSecurityIdentity('ROLE_FOO'), new UserSecurityIdentity('ROLE_FOO', 'Foo'), false],
         ];

Tests/Domain/SecurityIdentityRetrievalStrategyTest.php

@@ -15,6 +15,10 @@
 use Symfony\Component\Security\Acl\Domain\SecurityIdentityRetrievalStrategy;
 use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
 use Symfony\Component\Security\Core\Role\Role;
+use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
 
 class SecurityIdentityRetrievalStrategyTest extends \PHPUnit\Framework\TestCase
 {
@@ -24,18 +28,18 @@ class SecurityIdentityRetrievalStrategyTest extends \PHPUnit\Framework\TestCase
     public function testGetSecurityIdentities($user, array $roles, $authenticationStatus, array $sids)
     {
         if ('anonymous' === $authenticationStatus) {
-            $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\AnonymousToken')
-                                ->disableOriginalConstructor()
-                                ->getMock();
+            $token = $this->getMockBuilder(AnonymousToken::class)
+                ->disableOriginalConstructor()
+                ->getMock();
         } else {
             $class = '';
             if (\is_string($user)) {
                 $class = 'MyCustomTokenImpl';
             }
 
-            $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')
-                        ->setMockClassName($class)
-                        ->getMock();
+            $token = $this->getMockBuilder(TokenInterface::class)
+                ->setMockClassName($class)
+                ->getMock();
         }
 
         if (method_exists($token, 'getRoleNames')) {
@@ -122,7 +126,10 @@ public function getSecurityIdentityRetrievalTests()
 
     protected function getAccount($username, $class)
     {
-        $account = $this->getMock('Symfony\Component\Security\Core\User\UserInterface', [], [], $class);
+        $account = $this->getMockBuilder(UserInterface::class)
+            ->setMockClassName($class)
+            ->getMock()
+        ;
         $account
             ->expects($this->any())
             ->method('getUsername')
@@ -158,10 +165,9 @@ protected function getStrategy(array $roles = [], $authenticationStatus = 'fullF
                 ->willReturn($roles);
         }
 
-        $trustResolver = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface', [], ['', '']);
+        $trustResolver = $this->createMock(AuthenticationTrustResolverInterface::class);
 
         $trustResolver
-            ->expects($this->at(0))
             ->method('isAnonymous')
             ->willReturn('anonymous' === $authenticationStatus)
         ;
@@ -189,7 +195,6 @@ protected function getStrategy(array $roles = [], $authenticationStatus = 'fullF
             ;
         } else {
             $trustResolver
-                ->expects($this->at(1))
                 ->method('isAnonymous')
                 ->willReturn(true)
             ;

composer.json

@@ -20,7 +20,7 @@
         "symfony/security-core": "^3.4|^4.4|^5.0"
     },
     "require-dev": {
-        "symfony/phpunit-bridge": "^3.4|^4.4|^5.0",
+        "symfony/phpunit-bridge": "^5.2",
         "doctrine/common": "~2.2",
         "doctrine/persistence": "^1.3.3",
         "doctrine/dbal": "^2.13",
@@ -35,7 +35,6 @@
     "conflict": {
         "doctrine/dbal": "<2.13.0"
     },
-    "minimum-stability": "dev",
     "extra": {
         "branch-alias": {
             "dev-main": "3.0-dev"

phpunit.xml.dist

@@ -1,15 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <phpunit backupGlobals="false"
-         backupStaticAttributes="false"
          colors="true"
-         convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
-         convertWarningsToExceptions="true"
-         processIsolation="false"
-         stopOnFailure="false"
-         syntaxCheck="false"
          bootstrap="vendor/autoload.php"
+         failOnRisky="true"
+         failOnWarning="true"
 >
     <php>
         <ini name="error_reporting" value="-1" />
@@ -31,4 +26,12 @@
             </exclude>
         </whitelist>
     </filter>
+    <groups>
+        <exclude>
+            <group>benchmark</group>
+        </exclude>
+    </groups>
+    <listeners>
+        <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener"/>
+    </listeners>
 </phpunit>