-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable PHPStan in tests #3260
Enable PHPStan in tests #3260
Conversation
); | ||
|
||
self::assertCount(1, $results); | ||
self::assertInstanceOf(__NAMESPACE__.'\\MyFetchClass', $results[0]); | ||
self::assertInstanceOf(MyFetchClass::class, $results[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly needed, but affected by phpstan/phpstan-phpunit#27.
6fd682b
to
0fc324b
Compare
composer.json
Outdated
@@ -21,6 +21,7 @@ | |||
"doctrine/coding-standard": "^4.0", | |||
"jetbrains/phpstorm-stubs": "^2018.1.2", | |||
"phpstan/phpstan": "^0.10.1", | |||
"phpstan/phpstan-phpunit": "dev-GetMockBuilderDynamicReturnTypeExtension-with-undefined-class as 0.10.0.999", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: make sure this patch is released first.
@@ -24,7 +24,7 @@ class PDOExceptionTest extends DbalTestCase | |||
/** | |||
* The wrapped PDO exception mock. | |||
* | |||
* @var \PDOException|\PHPUnit_Framework_MockObject_MockObject | |||
* @var \PDOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay!
@@ -15,7 +16,7 @@ class MySqlSchemaManagerTest extends \PHPUnit\Framework\TestCase | |||
*/ | |||
private $manager; | |||
|
|||
/** @var Connection */ | |||
/** @var Connection|MockObject */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the opposite of https://github.com/doctrine/dbal/pull/3260/files#diff-90748caec80a98aa0fcfef640916661bL27.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the referenced code isn't acutally a mock at all, see https://github.com/doctrine/dbal/pull/3260/files#diff-90748caec80a98aa0fcfef640916661bR39. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Makes sense.
I thought, using phpstan-phpunit
would eliminate the necessity to explicitly annotate mocks as such. Looks like it's only the case for the return values of methods like createMock()
which automatically get the traits of the object under test and the mock. But for properties, you still have to specify the type explictly. Does it sound about right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it changes return types of these factory methods to return intersections of the mock type and the mocked type.
Also, because intersection support sucks in IDEs, it should convert unions like Type|MockObject
to an intersection Type&MockObject
(not sure how this is implemented).
Additional constraints are there for assertion methods (i.e. assertInstanceOf
), see readme: https://github.com/phpstan/phpstan-phpunit#phpstan-phpunit-extensions-and-rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it converts them in this case, currently hardcoded in PHPStan, but will be cleaner and part of the phpunit extension in 0.11.
@@ -24,7 +25,7 @@ class DateImmutableTypeTest extends \PHPUnit\Framework\TestCase | |||
protected function setUp() | |||
{ | |||
$this->type = Type::getType('date_immutable'); | |||
$this->platform = $this->prophesize(AbstractPlatform::class); | |||
$this->platform = $this->getMockBuilder(AbstractPlatform::class)->getMock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably could use $this->createMock()
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, forgot to change this file. 😃
* TaskMock used for testing the CLI interface. | ||
* @author Nils Adermann <naderman@naderman.de> | ||
*/ | ||
class TaskMock extends \Doctrine\Common\Cli\Tasks\AbstractTask |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this one was REALLY old. ;)
https://github.com/doctrine/dbal/commits/319e20f847efecc8a2727c8dac5ceb592498b81b/tests/Doctrine/Tests/Mocks/TaskMock.php
385b730
to
6a2b788
Compare
…having to replicate the \PDOStatement interface in ResultStatement
Changed the type of $char in AbstractPlatform::getTrimExpression()`from string|false to ?string
…pression() signatures
Modified AbstractPlatform::getLocateExpression() and ::getSubstringExpression() signatures
Reworked AbstractPlatform::get*Expression() methods
Enable strict types
…n-key-rule-def Removed DB2SchemaManager::_getPortableForeignKeyRuleDef()
Reworked driver exceptions
…moved `::removeDoctrineTypeFromComment()`
Removed errorCode() and errorInfo() methods from Connection and Statement APIs
Reworked AbstractSchemaManager::extractDoctrineTypeFromComment(), removed ::removeDoctrineTypeFromComment()
Rebased on top of develop, green on level 3, 65 more errors on level 7. |
We can wrap it up at level 3 and continue to improve later in order to avoid future conflicts and new issues on Level 3 and lower. BTW, why |
Picked wrong target when rebasing and didn't want to start again on top of master. :| |
Could you at least roll it back? I would like to have it done in |
Summary
Enabling PHPStan in tests/ at level 3, just like for lib/. 🎉
Rewritten some Prophecy-based unit tests into PHPUnit mock system to avoid having another (unofficial) extension for Prophecy.
Replaces incomplete #3203 with no changes like (Class -> mixed).
Currently depends on my fork of phpstan-phpunit due to phpstan/phpstan-phpunit#28.