From f3f313d34fdf7d5702fa4c038c31c0f09ded58dd Mon Sep 17 00:00:00 2001 From: David Fox Date: Thu, 9 May 2019 15:12:28 -0400 Subject: [PATCH 1/7] MockBuilder method to verify mock method existence --- src/Framework/MockObject/MockBuilder.php | 25 +++++++++++++++++ .../Framework/MockObject/MockBuilderTest.php | 28 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index 5b2c6fcd69c..4f9cd1d40db 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -189,6 +189,31 @@ public function setMethods(array $methods = null): self return $this; } + /** + * Specifies the subset of methods to mock, requiring each to exist in the class + */ + public function setRealMethods(array $methods = null): self + { + if ($methods) { + $reflection = new \ReflectionClass($this->type); + + foreach ($methods as $method) { + if (!$reflection->hasMethod($method)) { + throw new RuntimeException( + \sprintf( + 'Trying to set mock method "%s" which cannot be configured because it does not exist', + $method + ) + ); + } + } + } + + $this->methods = $methods; + + return $this; + } + /** * Specifies the subset of methods to not mock. Default is to mock all of them. */ diff --git a/tests/unit/Framework/MockObject/MockBuilderTest.php b/tests/unit/Framework/MockObject/MockBuilderTest.php index 3d22478c2ba..c5d683f41d5 100644 --- a/tests/unit/Framework/MockObject/MockBuilderTest.php +++ b/tests/unit/Framework/MockObject/MockBuilderTest.php @@ -51,6 +51,34 @@ public function testMethodExceptionsToMockCanBeSpecified(): void $this->assertNull($mock->anotherMockableMethod()); } + public function testSetMethodsAllowsNonExistentMethodNames(): void + { + $mock = $this->getMockBuilder(Mockable::class) + ->setMethods(['mockableMethodWithCrazyName']) + ->getMock(); + + $this->assertNull($mock->mockableMethodWithCrazyName()); + } + + public function testSetRealMethodsWithNonExistentMethodNames(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->setRealMethods(['mockableMethodWithCrazyName']) + ->getMock(); + } + + public function testSetRealMethodsWithExistingMethodNames(): void + { + $mock = $this->getMockBuilder(Mockable::class) + ->setRealMethods(['mockableMethod']) + ->getMock(); + + $this->assertNull($mock->mockableMethod()); + $this->assertTrue($mock->anotherMockableMethod()); + } + public function testEmptyMethodExceptionsToMockCanBeSpecified(): void { $mock = $this->getMockBuilder(Mockable::class) From d8a10da6351b9989752e61ea741a1a7d3e0379af Mon Sep 17 00:00:00 2001 From: David Fox Date: Thu, 9 May 2019 16:31:01 -0400 Subject: [PATCH 2/7] Add ArgumentTypeCoercion type psalm error to baseline --- .psalm/baseline.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml index c39b22fc015..2a1443dd805 100644 --- a/.psalm/baseline.xml +++ b/.psalm/baseline.xml @@ -201,6 +201,9 @@ null + + $this->type + From 097bb47ef40506ef1073905523147b0a8fd12f95 Mon Sep 17 00:00:00 2001 From: David Fox Date: Sat, 11 May 2019 09:39:26 -0400 Subject: [PATCH 3/7] Some changes based on feedback (doc, null) --- src/Framework/MockObject/MockBuilder.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index 4f9cd1d40db..9d9e8dae2be 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -191,8 +191,10 @@ public function setMethods(array $methods = null): self /** * Specifies the subset of methods to mock, requiring each to exist in the class + * + * @param string[] $methods */ - public function setRealMethods(array $methods = null): self + public function setRealMethods(array $methods): self { if ($methods) { $reflection = new \ReflectionClass($this->type); From d2f1f5075453e31d80bdde94e3e668d460f2c3fe Mon Sep 17 00:00:00 2001 From: David Fox Date: Sat, 11 May 2019 12:25:40 -0400 Subject: [PATCH 4/7] Add throws, change error message + add class name --- src/Framework/MockObject/MockBuilder.php | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index 9d9e8dae2be..fc4f2180782 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -193,21 +193,22 @@ public function setMethods(array $methods = null): self * Specifies the subset of methods to mock, requiring each to exist in the class * * @param string[] $methods + * + * @throws RuntimeException */ public function setRealMethods(array $methods): self { - if ($methods) { - $reflection = new \ReflectionClass($this->type); - - foreach ($methods as $method) { - if (!$reflection->hasMethod($method)) { - throw new RuntimeException( - \sprintf( - 'Trying to set mock method "%s" which cannot be configured because it does not exist', - $method - ) - ); - } + $reflection = new \ReflectionClass($this->type); + + foreach ($methods as $method) { + if (!$reflection->hasMethod($method)) { + throw new RuntimeException( + \sprintf( + 'Trying to set mock method "%s", but it does not exist in class "%s"', + $method, + $this->type + ) + ); } } From a606bf89f4486de4bad25486de70b24eb5bc4eda Mon Sep 17 00:00:00 2001 From: David Fox Date: Wed, 15 May 2019 10:04:58 -0400 Subject: [PATCH 5/7] Add onlyMethods, addMethods + deprecate setMethods --- .psalm/baseline.xml | 2 +- src/Framework/MockObject/MockBuilder.php | 34 +++++++++++++++++-- .../Framework/MockObject/MockBuilderTest.php | 33 ++++++++++++++---- 3 files changed, 59 insertions(+), 10 deletions(-) diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml index 2a1443dd805..9f63d36fb39 100644 --- a/.psalm/baseline.xml +++ b/.psalm/baseline.xml @@ -201,7 +201,7 @@ null - + $this->type diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index fc4f2180782..36aa9bd2338 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -181,6 +181,8 @@ public function getMockForTrait(): MockObject /** * Specifies the subset of methods to mock. Default is to mock none of them. + * + * @deprecated https://github.com/sebastianbergmann/phpunit/pull/3687 */ public function setMethods(array $methods = null): self { @@ -196,7 +198,7 @@ public function setMethods(array $methods = null): self * * @throws RuntimeException */ - public function setRealMethods(array $methods): self + public function onlyMethods(array $methods): self { $reflection = new \ReflectionClass($this->type); @@ -204,7 +206,35 @@ public function setRealMethods(array $methods): self if (!$reflection->hasMethod($method)) { throw new RuntimeException( \sprintf( - 'Trying to set mock method "%s", but it does not exist in class "%s"', + 'Trying to set mock method "%s" with onlyMethods, but it does not exist in class "%s". Use addMethods() for methods that don\'t exist in the class.', + $method, + $this->type + ) + ); + } + } + + $this->methods = $methods; + + return $this; + } + + /** + * Specifies methods that don't exist in the class which you want to mock + * + * @param string[] $methods + * + * @throws RuntimeException + */ + public function addMethods(array $methods): self + { + $reflection = new \ReflectionClass($this->type); + + foreach ($methods as $method) { + if ($reflection->hasMethod($method)) { + throw new RuntimeException( + \sprintf( + 'Trying to set mock method "%s" with addMethod, but it exists in class "%s". Use onlyMethods() for methods that exist in the class.', $method, $this->type ) diff --git a/tests/unit/Framework/MockObject/MockBuilderTest.php b/tests/unit/Framework/MockObject/MockBuilderTest.php index c5d683f41d5..e03f6e8986d 100644 --- a/tests/unit/Framework/MockObject/MockBuilderTest.php +++ b/tests/unit/Framework/MockObject/MockBuilderTest.php @@ -60,30 +60,49 @@ public function testSetMethodsAllowsNonExistentMethodNames(): void $this->assertNull($mock->mockableMethodWithCrazyName()); } - public function testSetRealMethodsWithNonExistentMethodNames(): void + public function testOnlyMethodsWithNonExistentMethodNames(): void { $this->expectException(RuntimeException::class); $this->getMockBuilder(Mockable::class) - ->setRealMethods(['mockableMethodWithCrazyName']) + ->onlyMethods(['mockableMethodWithCrazyName']) ->getMock(); } - public function testSetRealMethodsWithExistingMethodNames(): void + public function testOnlyMethodsWithExistingMethodNames(): void { $mock = $this->getMockBuilder(Mockable::class) - ->setRealMethods(['mockableMethod']) - ->getMock(); + ->onlyMethods(['mockableMethod']) + ->getMock(); $this->assertNull($mock->mockableMethod()); $this->assertTrue($mock->anotherMockableMethod()); } + public function testAddMethodsWithNonExistentMethodNames(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->addMethods(['mockableMethod']) + ->getMock(); + } + + public function testAddMethodsWithExistingMethodNames(): void + { + $mock = $this->getMockBuilder(Mockable::class) + ->addMethods(['mockableMethodWithFakeMethod']) + ->getMock(); + + $this->assertNull($mock->mockableMethodWithFakeMethod()); + $this->assertTrue($mock->anotherMockableMethod()); + } + public function testEmptyMethodExceptionsToMockCanBeSpecified(): void { $mock = $this->getMockBuilder(Mockable::class) - ->setMethodsExcept() - ->getMock(); + ->setMethodsExcept() + ->getMock(); $this->assertNull($mock->mockableMethod()); $this->assertNull($mock->anotherMockableMethod()); From 27de9a570a316c8a155d1d48ff2cc328195fa078 Mon Sep 17 00:00:00 2001 From: David Fox Date: Wed, 15 May 2019 10:36:40 -0400 Subject: [PATCH 6/7] Only allow new mock methods to configure once --- src/Framework/MockObject/MockBuilder.php | 17 ++++++++++++++++ .../Framework/MockObject/MockBuilderTest.php | 20 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index 36aa9bd2338..7297cedaf36 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -86,6 +86,11 @@ final class MockBuilder */ private $generator; + /** + * @var bool + */ + private $alreadyUsedMockMethodConfiguration = false; + /** * @param string|string[] $type * @@ -200,6 +205,12 @@ public function setMethods(array $methods = null): self */ public function onlyMethods(array $methods): self { + if ($this->alreadyUsedMockMethodConfiguration) { + throw new RuntimeException('Can\'t use onlyMethods after already configuring mock methods on the mock.'); + } + + $this->alreadyUsedMockMethodConfiguration = true; + $reflection = new \ReflectionClass($this->type); foreach ($methods as $method) { @@ -228,6 +239,12 @@ public function onlyMethods(array $methods): self */ public function addMethods(array $methods): self { + if ($this->alreadyUsedMockMethodConfiguration) { + throw new RuntimeException('Can\'t use addMethods after already configuring mock methods on the mock.'); + } + + $this->alreadyUsedMockMethodConfiguration = true; + $reflection = new \ReflectionClass($this->type); foreach ($methods as $method) { diff --git a/tests/unit/Framework/MockObject/MockBuilderTest.php b/tests/unit/Framework/MockObject/MockBuilderTest.php index e03f6e8986d..7c31c86d49a 100644 --- a/tests/unit/Framework/MockObject/MockBuilderTest.php +++ b/tests/unit/Framework/MockObject/MockBuilderTest.php @@ -108,6 +108,26 @@ public function testEmptyMethodExceptionsToMockCanBeSpecified(): void $this->assertNull($mock->anotherMockableMethod()); } + public function testNotAbleToUseAddMethodsAfterOnlyMethods(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->onlyMethods(['mockableMethod']) + ->addMethods(['mockableMethodWithFakeMethod']) + ->getMock(); + } + + public function testNotAbleToUseOnlyMethodsAfterAddMethods(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->addMethods(['mockableMethodWithFakeMethod']) + ->onlyMethods(['mockableMethod']) + ->getMock(); + } + public function testByDefaultDoesNotPassArgumentsToTheConstructor(): void { $mock = $this->getMockBuilder(Mockable::class)->getMock(); From 9f4764fb3a338e7c2461a0aa57921173e31d7104 Mon Sep 17 00:00:00 2001 From: David Fox Date: Wed, 15 May 2019 11:30:54 -0400 Subject: [PATCH 7/7] Warn of non-existent methods in createPartialMock --- .psalm/baseline.xml | 3 +- src/Framework/MockObject/MockBuilder.php | 16 +++++- src/Framework/TestCase.php | 20 +++++++ tests/_files/TestWithDifferentStatuses.php | 11 ++++ .../Framework/MockObject/MockBuilderTest.php | 52 ++++++++++++++++--- tests/unit/Framework/TestCaseTest.php | 20 +++++++ 6 files changed, 113 insertions(+), 9 deletions(-) diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml index 9f63d36fb39..c8ad440f9da 100644 --- a/.psalm/baseline.xml +++ b/.psalm/baseline.xml @@ -220,7 +220,8 @@ - + + $class_name $this->expectedException diff --git a/src/Framework/MockObject/MockBuilder.php b/src/Framework/MockObject/MockBuilder.php index 7297cedaf36..96dd1a662b4 100644 --- a/src/Framework/MockObject/MockBuilder.php +++ b/src/Framework/MockObject/MockBuilder.php @@ -193,6 +193,8 @@ public function setMethods(array $methods = null): self { $this->methods = $methods; + $this->alreadyUsedMockMethodConfiguration = true; + return $this; } @@ -206,7 +208,12 @@ public function setMethods(array $methods = null): self public function onlyMethods(array $methods): self { if ($this->alreadyUsedMockMethodConfiguration) { - throw new RuntimeException('Can\'t use onlyMethods after already configuring mock methods on the mock.'); + throw new RuntimeException( + \sprintf( + 'Can\'t use onlyMethods on "%s" mock because mocked methods were already configured.', + $this->type + ) + ); } $this->alreadyUsedMockMethodConfiguration = true; @@ -240,7 +247,12 @@ public function onlyMethods(array $methods): self public function addMethods(array $methods): self { if ($this->alreadyUsedMockMethodConfiguration) { - throw new RuntimeException('Can\'t use addMethods after already configuring mock methods on the mock.'); + throw new RuntimeException( + \sprintf( + 'Can\'t use addMethods on "%s" mock because mocked methods were already configured.', + $this->type + ) + ); } $this->alreadyUsedMockMethodConfiguration = true; diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php index b583fcc9570..eb252f5c658 100644 --- a/src/Framework/TestCase.php +++ b/src/Framework/TestCase.php @@ -1521,6 +1521,26 @@ protected function createConfiguredMock($originalClassName, array $configuration */ protected function createPartialMock($originalClassName, array $methods): MockObject { + $class_names = \is_array($originalClassName) ? $originalClassName : [$originalClassName]; + + foreach ($class_names as $class_name) { + $reflection = new \ReflectionClass($class_name); + + $mockedMethodsThatDontExist = \array_filter($methods, function (string $method) use ($reflection) { + return !$reflection->hasMethod($method); + }); + + if ($mockedMethodsThatDontExist) { + $this->addWarning( + \sprintf( + 'createPartialMock called with method(s) %s that do not exist in %s. This will not be allowed in future versions of PHPUnit.', + \implode(', ', $mockedMethodsThatDontExist), + $class_name + ) + ); + } + } + return $this->getMockBuilder($originalClassName) ->disableOriginalConstructor() ->disableOriginalClone() diff --git a/tests/_files/TestWithDifferentStatuses.php b/tests/_files/TestWithDifferentStatuses.php index e680f6ce2d3..da170332349 100644 --- a/tests/_files/TestWithDifferentStatuses.php +++ b/tests/_files/TestWithDifferentStatuses.php @@ -45,4 +45,15 @@ public function testThatAddsAWarning(): void { $this->addWarning('Sorry, Dave!'); } + + public function testWithCreatePartialMockWarning(): void + { + $this->createPartialMock(\Mockable::class, ['mockableMethod', 'fakeMethod1', 'fakeMethod2']); + } + + public function testWithCreatePartialMockPassesNoWarning(): void + { + $mock = $this->createPartialMock(\Mockable::class, ['mockableMethod']); + $this->assertNull($mock->mockableMethod()); + } } diff --git a/tests/unit/Framework/MockObject/MockBuilderTest.php b/tests/unit/Framework/MockObject/MockBuilderTest.php index 7c31c86d49a..f7034ac8981 100644 --- a/tests/unit/Framework/MockObject/MockBuilderTest.php +++ b/tests/unit/Framework/MockObject/MockBuilderTest.php @@ -54,8 +54,8 @@ public function testMethodExceptionsToMockCanBeSpecified(): void public function testSetMethodsAllowsNonExistentMethodNames(): void { $mock = $this->getMockBuilder(Mockable::class) - ->setMethods(['mockableMethodWithCrazyName']) - ->getMock(); + ->setMethods(['mockableMethodWithCrazyName']) + ->getMock(); $this->assertNull($mock->mockableMethodWithCrazyName()); } @@ -65,8 +65,8 @@ public function testOnlyMethodsWithNonExistentMethodNames(): void $this->expectException(RuntimeException::class); $this->getMockBuilder(Mockable::class) - ->onlyMethods(['mockableMethodWithCrazyName']) - ->getMock(); + ->onlyMethods(['mockableMethodWithCrazyName']) + ->getMock(); } public function testOnlyMethodsWithExistingMethodNames(): void @@ -101,8 +101,8 @@ public function testAddMethodsWithExistingMethodNames(): void public function testEmptyMethodExceptionsToMockCanBeSpecified(): void { $mock = $this->getMockBuilder(Mockable::class) - ->setMethodsExcept() - ->getMock(); + ->setMethodsExcept() + ->getMock(); $this->assertNull($mock->mockableMethod()); $this->assertNull($mock->anotherMockableMethod()); @@ -128,6 +128,46 @@ public function testNotAbleToUseOnlyMethodsAfterAddMethods(): void ->getMock(); } + public function testAbleToUseSetMethodsAfterOnlyMethods(): void + { + $mock = $this->getMockBuilder(Mockable::class) + ->onlyMethods(['mockableMethod']) + ->setMethods(['mockableMethodWithCrazyName']) + ->getMock(); + + $this->assertNull($mock->mockableMethodWithCrazyName()); + } + + public function testAbleToUseSetMethodsAfterAddMethods(): void + { + $mock = $this->getMockBuilder(Mockable::class) + ->addMethods(['notAMethod']) + ->setMethods(['mockableMethodWithCrazyName']) + ->getMock(); + + $this->assertNull($mock->mockableMethodWithCrazyName()); + } + + public function testNotAbleToUseAddMethodsAfterSetMethods(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->setMethods(['mockableMethod']) + ->addMethods(['mockableMethodWithFakeMethod']) + ->getMock(); + } + + public function testNotAbleToUseOnlyMethodsAfterSetMethods(): void + { + $this->expectException(RuntimeException::class); + + $this->getMockBuilder(Mockable::class) + ->setMethods(['mockableMethodWithFakeMethod']) + ->onlyMethods(['mockableMethod']) + ->getMock(); + } + public function testByDefaultDoesNotPassArgumentsToTheConstructor(): void { $mock = $this->getMockBuilder(Mockable::class)->getMock(); diff --git a/tests/unit/Framework/TestCaseTest.php b/tests/unit/Framework/TestCaseTest.php index 29e1ab5dd73..fe6e7d087b0 100644 --- a/tests/unit/Framework/TestCaseTest.php +++ b/tests/unit/Framework/TestCaseTest.php @@ -857,6 +857,26 @@ public function testCreatePartialMockCanMockNoMethods(): void $this->assertTrue($mock->anotherMockableMethod()); } + public function testCreatePartialMockWithFakeMethods(): void + { + $test = new \TestWithDifferentStatuses('testWithCreatePartialMockWarning'); + + $test->run(); + + $this->assertSame(BaseTestRunner::STATUS_WARNING, $test->getStatus()); + $this->assertFalse($test->hasFailed()); + } + + public function testCreatePartialMockWithRealMethods(): void + { + $test = new \TestWithDifferentStatuses('testWithCreatePartialMockPassesNoWarning'); + + $test->run(); + + $this->assertSame(BaseTestRunner::STATUS_PASSED, $test->getStatus()); + $this->assertFalse($test->hasFailed()); + } + public function testCreateMockSkipsConstructor(): void { /** @var \Mockable $mock */