Enable PHPStan in tests #3203

carusogabriel · 2018-07-01T17:49:30Z

Q	A
Type	improvement
BC Break	no
Fixed issues	-

Summary

Enable PHPStan on test at level 3.

carusogabriel · 2018-07-06T01:01:49Z

@Majkl578 @morozov There're a few errors left. Can you help me with them? 😇

morozov

@carusogabriel as for the remaining

 ------ ---------------------------------------------------------------------- 
  Line   tests/Doctrine/Tests/DBAL/Functional/Driver/PDOSqlsrv/DriverTest.php  
 ------ ---------------------------------------------------------------------- 
  72     Call to an undefined method                                           
         Doctrine\DBAL\Driver\Connection::getAttribute().                      
 ------ ----------------------------------------------------------------------

You can add a type hint like @var \PDO $connection.

morozov · 2018-07-06T04:56:47Z

tests/Doctrine/Tests/DBAL/ConnectionTest.php

@@ -138,9 +138,11 @@ public function testGetEventManager()

    public function testConnectDispatchEvent()
    {
+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


Is there a problem detecting the type here? The code looks straightforward. AFAIK, PHPUnit will use namespaced class names if future versions, so it makes sense to use \PHPUnit\Framework\... instead.

Looks like it still PHPUnit_... https://github.com/sebastianbergmann/phpunit/blob/master/src/Framework/MockObject/MockObject.php#L21

It's also https://github.com/sebastianbergmann/phpunit/blob/0b52fd1df743e51b32becee1a4276c012c7b51d7/src/Framework/MockObject/ForwardCompatibility/MockObject.php

morozov · 2018-07-06T04:59:16Z

tests/Doctrine/Tests/DBAL/Functional/Driver/IBMDB2/DB2StatementTest.php

@@ -26,6 +26,7 @@ protected function setUp()

    public function testExecutionErrorsAreNotSuppressed()
    {
+        /** @var \Doctrine\DBAL\Statement $stmt */


This looks redundant. The @return annotation on the method declares the type.

morozov · 2018-07-06T05:00:45Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -34,7 +34,7 @@
 class SchemaManagerFunctionalTestCase extends \Doctrine\Tests\DbalFunctionalTestCase
 {
    /**
-     * @var \Doctrine\DBAL\Schema\AbstractSchemaManager
+     * @var mixed


Is it supposed to be anything else than a schema manager?

morozov · 2018-07-06T05:01:11Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -342,10 +342,12 @@ public function testListTableColumnsDispatchEvent()

        $this->_sm->dropAndCreateTable($table);

+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


This shouldn't be needed.

I tried to remove, but, no success 😢

It still doesn't look right. PhpStorm unambiguously detects the $listenerMockType as PHPUnit\Framework\MockObject\MockObject based on the @return annotation of PHPUnit\Framework\MockObject\MockBuilder::getMock(). So should do PHPStan. Instead of annotating every call like this, we need to find what the underlying problem is and fix it.

morozov · 2018-07-06T05:01:21Z

tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php

@@ -370,10 +372,12 @@ public function testListTableIndexesDispatchEvent()

        $this->_sm->dropAndCreateTable($table);

+        /** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */


Shouldn't be needed.

morozov · 2018-07-06T05:10:02Z

tests/Doctrine/Tests/DBAL/Types/DateImmutableTypeTest.php

@@ -44,6 +44,7 @@ public function testReturnsBindingType()

    public function testConvertsDateTimeImmutableInstanceToDatabaseValue()
    {
+        /** @var \DateTimeImmutable|\Prophecy\Prophecy\ObjectProphecy $date */


Does phpstan/phpstan-phpunit handle prophecy mocks? Or there's a separate extension for them? In any event, it's better to have this type detection automated rather than manually entered every time.

morozov · 2018-07-06T05:10:20Z

tests/Doctrine/Tests/DBAL/Types/DateIntervalTest.php

@@ -16,7 +16,7 @@
    private $platform;

    /**
-     * @var \Doctrine\DBAL\Types\DateIntervalType
+     * @var \Doctrine\DBAL\Types\Type


Please revert.

morozov · 2018-07-06T05:10:26Z

tests/Doctrine/Tests/DBAL/Types/DateTimeImmutableTypeTest.php

@@ -17,7 +17,7 @@ class DateTimeImmutableTypeTest extends \PHPUnit\Framework\TestCase
    private $platform;

    /**
-     * @var DateTimeImmutableType
+     * @var Type


Please revert.

Should this parent/child issue be reported to upstream?

------ ----------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Types/DateTimeImmutableTypeTest.php ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------- 26 Property Doctrine\Tests\DBAL\Types\DateTimeImmutableTypeTest::$type (Doctrine\DBAL\Types\DateTimeImmutableType) does not accept Doctrine\DBAL\Types\Type. ------ -----------------------------------------------------------------------------------------------------------------------------------------------------------

No. Please try using new DateTimeImmutableType() instead of the factory.

morozov · 2018-07-06T05:10:47Z

tests/Doctrine/Tests/DbalFunctionalTestCase.php

     */
    private static $_sharedConn;

    /**
-     * @var \Doctrine\DBAL\Connection
+     * @var mixed


Please revert.

It breaks a lot of places:

------ ------------------------------------------------------------------------------------ Line tests/Doctrine/Tests/DBAL/Functional/Driver/IBMDB2/DB2StatementTest.php ------ ------------------------------------------------------------------------------------ 32 Call to an undefined method Doctrine\DBAL\Driver\Statement::getWrappedStatement(). ------ ------------------------------------------------------------------------------------ ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Functional/Driver/OCI8/OCI8ConnectionTest.php ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 29 Property Doctrine\Tests\DBAL\Functional\Driver\OCI8\OCI8ConnectionTest::$driverConnection (Doctrine\DBAL\Driver\OCI8\OCI8Connection) does not accept Doctrine\DBAL\Driver\Connection. ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Line tests/Doctrine/Tests/DBAL/Functional/Driver/PDOConnectionTest.php ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 27 Property Doctrine\Tests\DBAL\Functional\Driver\PDOConnectionTest::$driverConnection (Doctrine\DBAL\Driver\PDOConnection) does not accept Doctrine\DBAL\Driver\Connection. ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------ ------------------------------------------------------------------------------ Line tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL630Test.php ------ ------------------------------------------------------------------------------ 41 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 75 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 99 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). 123 Call to an undefined method Doctrine\DBAL\Driver\Connection::setAttribute(). ------ ------------------------------------------------------------------------------

Those breakages look valid though, looks PDO related. Explicitly mixed only disables some checks (so better to avoid mixed).

morozov · 2018-07-06T05:11:26Z

tests/Doctrine/Tests/Mocks/DriverMock.php

 use Doctrine\DBAL\Schema\AbstractSchemaManager;

 class DriverMock implements \Doctrine\DBAL\Driver
 {
    /**
-     * @var DatabasePlatformMock
+     * @var AbstractPlatform|DatabasePlatformMock


DatabasePlatformMock extends AbstractPlatform. The former is enough.

carusogabriel · 2018-07-09T02:44:18Z

phpcbf f*cked us in this review 🤦‍♂️

carusogabriel · 2018-07-09T16:17:30Z

@morozov Some comments that now are folded but I'd like your reply to continue to fix:

Majkl578 · 2018-08-02T13:06:07Z

@carusogabriel Hi, any progress?

carusogabriel · 2018-08-02T13:24:43Z

Closing as I won't be able to finish it. Anyone is encouraged to finish it 😊

Majkl578 · 2018-08-10T01:32:47Z

Great job.

morozov requested changes Jul 6, 2018

View reviewed changes

carusogabriel changed the title ~~[WIP] Enable PHPStan in tests~~ Enable PHPStan in tests Jul 9, 2018

Enable PHPStan in tests

82ee39f

carusogabriel closed this Aug 10, 2018

carusogabriel deleted the phpstan-tests branch August 10, 2018 00:21

Majkl578 added Improvement Incomplete Static Analysis labels Aug 10, 2018

Majkl578 mentioned this pull request Aug 18, 2018

Enable PHPStan in tests #3260

Closed

morozov assigned carusogabriel Oct 13, 2018

github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable PHPStan in tests #3203

Enable PHPStan in tests #3203

carusogabriel commented Jul 1, 2018 •

edited

Loading

carusogabriel commented Jul 6, 2018

morozov left a comment

morozov Jul 6, 2018

carusogabriel Jul 9, 2018

morozov Jul 9, 2018

morozov Jul 6, 2018

morozov Jul 6, 2018

morozov Jul 6, 2018

carusogabriel Jul 9, 2018

morozov Jul 9, 2018

morozov Jul 6, 2018

morozov Jul 6, 2018

morozov Jul 6, 2018

morozov Jul 6, 2018

carusogabriel Jul 9, 2018

morozov Jul 9, 2018

morozov Jul 6, 2018

carusogabriel Jul 9, 2018

Majkl578 Jul 9, 2018

morozov Jul 6, 2018

carusogabriel commented Jul 9, 2018

carusogabriel commented Jul 9, 2018

Majkl578 commented Aug 2, 2018

carusogabriel commented Aug 2, 2018 •

edited

Loading

Majkl578 commented Aug 10, 2018

		@@ -342,10 +342,12 @@ public function testListTableColumnsDispatchEvent()

		$this->_sm->dropAndCreateTable($table);

		/** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */

		@@ -370,10 +372,12 @@ public function testListTableIndexesDispatchEvent()

		$this->_sm->dropAndCreateTable($table);

		/** @var \PHPUnit_Framework_MockObject_MockObject $listenerMock */

Enable PHPStan in tests #3203

Enable PHPStan in tests #3203

Conversation

carusogabriel commented Jul 1, 2018 • edited Loading

Summary

carusogabriel commented Jul 6, 2018

morozov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carusogabriel commented Jul 9, 2018

carusogabriel commented Jul 9, 2018

Majkl578 commented Aug 2, 2018

carusogabriel commented Aug 2, 2018 • edited Loading

Majkl578 commented Aug 10, 2018

carusogabriel commented Jul 1, 2018 •

edited

Loading

carusogabriel commented Aug 2, 2018 •

edited

Loading