From 4a116db6a3918d7ecc36823f839960c1d8277dde Mon Sep 17 00:00:00 2001 From: Frank Fleige Date: Mon, 17 Jul 2017 14:18:33 +0200 Subject: [PATCH 1/3] new sample 3 --- .idea/PHPUnitPlayground.iml | 28 +++---- ReadMe.md | 14 +++- composer.lock | 24 +++--- src/Sample3/A.php | 76 ++++++++++++++++++ src/Sample3/A1.php | 75 ++++++++++++++++++ src/Sample3/B.php | 67 ++++++++++++++++ src/Sample3/C.php | 54 +++++++++++++ src/Sample3/ReadMe.md | 150 ++++++++++++++++++++++++++++++++++++ src/Sample3/Sample3.php | 29 +++++++ src/Sample3/c_config.php | 10 +++ test/Sample3/A1Test.php | 70 +++++++++++++++++ test/Sample3/ATest.php | 48 ++++++++++++ test/Sample3/BTest.php | 61 +++++++++++++++ 13 files changed, 677 insertions(+), 29 deletions(-) create mode 100644 src/Sample3/A.php create mode 100644 src/Sample3/A1.php create mode 100644 src/Sample3/B.php create mode 100644 src/Sample3/C.php create mode 100644 src/Sample3/ReadMe.md create mode 100644 src/Sample3/Sample3.php create mode 100644 src/Sample3/c_config.php create mode 100644 test/Sample3/A1Test.php create mode 100644 test/Sample3/ATest.php create mode 100644 test/Sample3/BTest.php diff --git a/.idea/PHPUnitPlayground.iml b/.idea/PHPUnitPlayground.iml index 3449561..d85cc61 100644 --- a/.idea/PHPUnitPlayground.iml +++ b/.idea/PHPUnitPlayground.iml @@ -37,20 +37,6 @@ - - - - - - - - - - - - - - @@ -127,5 +113,19 @@ + + + + + + + + + + + + + + \ No newline at end of file diff --git a/ReadMe.md b/ReadMe.md index c39eadb..b5febfa 100644 --- a/ReadMe.md +++ b/ReadMe.md @@ -3,17 +3,25 @@ ## about This is a project to play around with unit tests. -## articles about unit tests +## articles/discussions about unit tests - [Steve Sanderson - Writing Great Unit Tests: Best and Worst Practices](http://blog.stevensanderson.com/2009/08/24/writing-great-unit-tests-best-and-worst-practises/) ## how unit tests impact code + +### depencency injection + +- [replacing methods in class to test](https://stackoverflow.com/questions/11399600/is-it-possible-to-phpunit-mock-object-to-replace-one-created-in-class) +- [replacing objects in class to test](https://stackoverflow.com/questions/279493/phpunit-avoid-constructor-arguments-for-mock) + + +### static + - [static methods should be avoided](https://stackoverflow.com/questions/34380247/php-how-to-make-testable-static-methods) -- [dependency injection](https://stackoverflow.com/questions/11399600/is-it-possible-to-phpunit-mock-object-to-replace-one-created-in-class) -## Frameworks +## useful frameworks - [PHP dependency injection container](http://php-di.org/) diff --git a/composer.lock b/composer.lock index 80255bd..b96d108 100644 --- a/composer.lock +++ b/composer.lock @@ -433,22 +433,22 @@ }, { "name": "phpdocumentor/reflection-docblock", - "version": "3.1.1", + "version": "3.2.0", "source": { "type": "git", "url": "https://github.com/phpDocumentor/ReflectionDocBlock.git", - "reference": "8331b5efe816ae05461b7ca1e721c01b46bafb3e" + "reference": "46f7e8bb075036c92695b15a1ddb6971c751e585" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpDocumentor/ReflectionDocBlock/zipball/8331b5efe816ae05461b7ca1e721c01b46bafb3e", - "reference": "8331b5efe816ae05461b7ca1e721c01b46bafb3e", + "url": "https://api.github.com/repos/phpDocumentor/ReflectionDocBlock/zipball/46f7e8bb075036c92695b15a1ddb6971c751e585", + "reference": "46f7e8bb075036c92695b15a1ddb6971c751e585", "shasum": "" }, "require": { "php": ">=5.5", "phpdocumentor/reflection-common": "^1.0@dev", - "phpdocumentor/type-resolver": "^0.2.0", + "phpdocumentor/type-resolver": "^0.4.0", "webmozart/assert": "^1.0" }, "require-dev": { @@ -474,24 +474,24 @@ } ], "description": "With this component, a library can provide support for annotations via DocBlocks or otherwise retrieve information that is embedded in a DocBlock.", - "time": "2016-09-30T07:12:33+00:00" + "time": "2017-07-15T11:38:20+00:00" }, { "name": "phpdocumentor/type-resolver", - "version": "0.2.1", + "version": "0.4.0", "source": { "type": "git", "url": "https://github.com/phpDocumentor/TypeResolver.git", - "reference": "e224fb2ea2fba6d3ad6fdaef91cd09a172155ccb" + "reference": "9c977708995954784726e25d0cd1dddf4e65b0f7" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpDocumentor/TypeResolver/zipball/e224fb2ea2fba6d3ad6fdaef91cd09a172155ccb", - "reference": "e224fb2ea2fba6d3ad6fdaef91cd09a172155ccb", + "url": "https://api.github.com/repos/phpDocumentor/TypeResolver/zipball/9c977708995954784726e25d0cd1dddf4e65b0f7", + "reference": "9c977708995954784726e25d0cd1dddf4e65b0f7", "shasum": "" }, "require": { - "php": ">=5.5", + "php": "^5.5 || ^7.0", "phpdocumentor/reflection-common": "^1.0" }, "require-dev": { @@ -521,7 +521,7 @@ "email": "me@mikevanriel.com" } ], - "time": "2016-11-25T06:54:22+00:00" + "time": "2017-07-14T14:27:02+00:00" }, { "name": "phpspec/prophecy", diff --git a/src/Sample3/A.php b/src/Sample3/A.php new file mode 100644 index 0000000..365cbdb --- /dev/null +++ b/src/Sample3/A.php @@ -0,0 +1,76 @@ +entitlements = []; + $this->initEntitlements(); + } + + /** + * will init the entitlement keys + */ + private function initEntitlements() + { + $this->entitlements = $this->getConfigurationRepository()->getConfig('entitlements'); + } + + /** + * will get an instance of the configuration repository + * @return C + */ + public function getConfigurationRepository() + { + return C::getInstance(); + } + + /** + * will return the keys of all entitlements + * @return string[] + */ + public function getEntitlements() + { + return $this->entitlements; + } + + /** + * will set the internal entitlements storage + * @param string[] $entitlements array with entitlement keys + */ + public function setEntitlements($entitlements) + { + $this->entitlements = $entitlements; + } + + /** + * checks if the given key is within the list of entitlements + * @param string $key + * @return boolean + */ + public function isEntitledTo($key) + { + return in_array($key, $this->entitlements); + } +} \ No newline at end of file diff --git a/src/Sample3/A1.php b/src/Sample3/A1.php new file mode 100644 index 0000000..4ddc7e4 --- /dev/null +++ b/src/Sample3/A1.php @@ -0,0 +1,75 @@ +entitlements = []; + } + + /** + * will init the entitlement keys + */ + public function initEntitlements() + { + $this->entitlements = $this->getConfigurationRepository()->getConfig('entitlements'); + } + + /** + * will get an instance of the configuration repository + * @return C + */ + public function getConfigurationRepository() + { + return C::getInstance(); + } + + /** + * will return the keys of all entitlements + * @return string[] + */ + public function getEntitlements() + { + return $this->entitlements; + } + + /** + * will set the internal entitlements storage + * @param string[] $entitlements array with entitlement keys + */ + public function setEntitlements($entitlements) + { + $this->entitlements = $entitlements; + } + + /** + * checks if the given key is within the list of entitlements + * @param string $key + * @return boolean + */ + public function isEntitledTo($key) + { + return in_array($key, $this->entitlements); + } +} \ No newline at end of file diff --git a/src/Sample3/B.php b/src/Sample3/B.php new file mode 100644 index 0000000..6302ba2 --- /dev/null +++ b/src/Sample3/B.php @@ -0,0 +1,67 @@ +configurationRepository = $cr; + $this->entitlements = []; + $this->initEntitlements(); + } + + /** + * will init the entitlement keys + */ + private function initEntitlements() + { + $this->entitlements = $this->configurationRepository->getConfig('entitlements'); + } + + /** + * will return the keys of all entitlements + * @return string[] + */ + public function getEntitlements() + { + return $this->entitlements; + } + + /** + * checks if the given key is within the list of entitlements + * @param string $key + * @return boolean + */ + public function isEntitledTo($key) + { + return in_array($key, $this->entitlements); + } +} \ No newline at end of file diff --git a/src/Sample3/C.php b/src/Sample3/C.php new file mode 100644 index 0000000..85b6120 --- /dev/null +++ b/src/Sample3/C.php @@ -0,0 +1,54 @@ +config = include_once(__DIR__ . DIRECTORY_SEPARATOR . 'c_config.php'); + } + + /** + * returns a singleton instance of C + * @return C + */ + public static function getInstance() + { + if (!static::$singleton) { + static::$singleton = new static(); + } + return static::$singleton; + } + + /** + * gets the configuration for a given key + * @param string $key + * @return mixed|null + */ + public function getConfig($key) + { + return (isset($this->config[$key])) ? $this->config[$key] : null; + } +} \ No newline at end of file diff --git a/src/Sample3/ReadMe.md b/src/Sample3/ReadMe.md new file mode 100644 index 0000000..2b71ded --- /dev/null +++ b/src/Sample3/ReadMe.md @@ -0,0 +1,150 @@ +# Sample 3 + +## introduction + +| class | purpose | +| ----- |-------------| +| A | a service class to provide and check entitlement keys. The keys are read from the configuration repository. | +| A1 | class A refactored (see explanation) | +| B | Same as A, but the configuration repository is injected as a dependency | +| C | A repository that provides key/value pairs. The key value pairs are read from a persistent storage. We use here the file system for simplicity, but i could also be a database. | + + +## the goals + +1. Check that `Sample3::getEntitlements()` will return an array. +1. Check that `Sample3::getEntitlements()` will return an empty array when no entitlements are present in the storage. +1. Check that `Sample3::getEntitlements()` will return two keys when two entitlements are present in the storage. +1. Check that `Sample3::isEntitledTo()` will return false when given key is not within the list of entitlements. +1. Check that `Sample3::isEntitledTo()` will return true when given key is within the list of entitlements. +1. Ensure that behaviour of unit test won't change when the storage changes. + + +## writing the unit tests + +### the challange + ```php + public function testGetEntitlements() { + $a = new A(); + $k = $a->getEntitlements(); + $this::assertCount(2, $k); + } + ``` + `$a->getEntitlements()` will return a list of all entitlements, read from the persitant store. + The unit test will pass as long as there are excatly two entitlements persisted in the storage, + but when it changes the test will fail. So we would. miss our goal #6. + + So whe have to mock the repository that provides the key/value pairs from the entitlement store: + +### mocking the repository (class C) + +```php + $cr = $this->getMockBuilder(C::class) + ->disableOriginalConstructor() + ->setMethods(['getConfig']) + ->getMock(); + $cr->expects($this::once()) + ->method('getConfig') + ->with('entitlements') + ->willReturn(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2']); +``` +First we create a mocked instance, but without calling the original constructor of A. +Thats because the original constructor would initialize the key/value pairs from the persistant storage: + +Second we tell the unit test that we expect exactly one call of the method `getConfig($key)` with the key `entitlements`. +This call will return a fi`xed array with two entitlement keys. + +### replacing the original with the mock + +This is where trouble begins when writing the unit test for A. +Whe have to replace the original class A with its mocked one. + +```php + /** + * will get an instance of the configuration repository + * @return C + */ + public function getConfigurationRepository() { + return C::getInstance(); + } +``` + +To to that we have to replace the method `getConfigurationRepository`, so +that it will return the mocked instance of C instead of the original one. + +```php + // $cr is the mocked instance of C (see above) + $a = $this->getMockBuilder(A::class) + ->disableOriginalConstructor() + ->setMethods(['getConfigurationRepository']) + ->getMock(); + $a->expects(self::once()) + ->method('getConfigurationRepository') + ->willReturn($cr); +``` +So when we now run our unit test, ... + +```php + /** @var Sample3 $a */ + $k = $a->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertCount(2, $k); +``` + +... it will fail! Why? Let's have a look at the constructor of A: + +```php + public function __construct() { + $this->keys = []; + $this->initEntitlements(); + } +``` + +As you can see, there is a call of the `initEntitlements` method: + +```php + private function initEntitlements() { + $this->entitlements = $this->getConfigurationRepository()->getConfig('entitlements'); + } +``` + +Because we disabled the original constructor when creating the mocked instance of A, +the internal entitlement storage of A has not been initialized. +How get we out of this? Well, there is no other way than to refactor the code: + +1. Removing the initialization of the internal entitlement storage from the constructor. + +```php + public function __construct() { + $this->entitlements = []; + } +``` + +2. Changing the scope of `initEntitlements()` to `public` + +```php + public function initEntitlements() { + $this->entitlements = $this->getConfigurationRepository()->getConfig('entitlements'); + } +``` + +3. Extend all instantiations of `A` with a call of `initEntitlements()` + +```php + $a = new A(); + $a->initEntitlements(); + // ... do something else with a public interface of $a ... +``` + +With the refactored class A1 we can achieve all our goals for the unit test. + +### how dependency injection would help + +When C is injected as a dependency, things will get a lot easier +(regarding writing unit tests... ;-)). We only have to create an instance of mocked class C (see above) +and inject it to B when creating an instance of it: + +```php + // $cr is an instance of the mocked class C + $b = new B($cr); +``` \ No newline at end of file diff --git a/src/Sample3/Sample3.php b/src/Sample3/Sample3.php new file mode 100644 index 0000000..102c6c6 --- /dev/null +++ b/src/Sample3/Sample3.php @@ -0,0 +1,29 @@ + ['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2'] +]; \ No newline at end of file diff --git a/test/Sample3/A1Test.php b/test/Sample3/A1Test.php new file mode 100644 index 0000000..810e77b --- /dev/null +++ b/test/Sample3/A1Test.php @@ -0,0 +1,70 @@ +getMockBuilder(C::class) + ->disableOriginalConstructor() + ->setMethods(['getConfig']) + ->getMock(); + $cr->expects($this::once()) + ->method('getConfig') + ->with('entitlements') + ->willReturn($e); + // mock a itself + $a = $this->getMockBuilder(A1::class) + ->setMethods(['getConfigurationRepository']) + ->getMock(); + $a->expects(self::once()) + ->method('getConfigurationRepository') + ->willReturn($cr); + /** @var A1 $a */ + $a->initEntitlements(); + return $a; + } + + /** + * @covers A1::getEntitlements() + */ + public function testGetEntitlements() { + // no entitlements + $a = $this->getMockedInstanceWithEntitlements([]); + $k = $a->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertEmpty($k); + // two entitlements + $a = $this->getMockedInstanceWithEntitlements(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2']); + $k = $a->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertCount(2, $k); + } + + /** + * @covers A1::isEntitledTo() + */ + public function testIsEntitledTo() { + $a = $this->getMockedInstanceWithEntitlements([]); + $this::assertFalse($a->isEntitledTo('de.frankfleige.phpunit.sample3.1')); + $a = $this->getMockedInstanceWithEntitlements(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2']); + $this::assertFalse($a->isEntitledTo('de.frankfleige.phpunit.sample3.3')); + $this::assertTrue($a->isEntitledTo('de.frankfleige.phpunit.sample3.2')); + $this::assertTrue($a->isEntitledTo('de.frankfleige.phpunit.sample3.1')); + } +} diff --git a/test/Sample3/ATest.php b/test/Sample3/ATest.php new file mode 100644 index 0000000..63d4570 --- /dev/null +++ b/test/Sample3/ATest.php @@ -0,0 +1,48 @@ +getMockBuilder(C::class) + ->disableOriginalConstructor() + ->setMethods(['getConfig']) + ->getMock(); + $cr->expects($this::once()) + ->method('getConfig') + ->with('entitlements') + ->willReturn(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2']); + // mock a itself + $a = $this->getMockBuilder(A::class) + ->disableOriginalConstructor() + ->setMethods(['getConfigurationRepository']) + ->getMock(); + /* + * bad! + * We have to call the (mocked) configuration repository externally within + * our unit test. So we cannot unit test if a configuration respository + */ + /*$a->expects(self::once()) + ->method('getConfigurationRepository') + ->willReturn($cr);*/ + /** @var A $a */ + /** @var C $cr */ + $a->setEntitlements($cr->getConfig('entitlements')); + $k = $a->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertCount(2, $k); + } + +} diff --git a/test/Sample3/BTest.php b/test/Sample3/BTest.php new file mode 100644 index 0000000..0d4e558 --- /dev/null +++ b/test/Sample3/BTest.php @@ -0,0 +1,61 @@ +getMockBuilder(C::class) + ->disableOriginalConstructor() + ->setMethods(['getConfig']) + ->getMock(); + $cr->expects($this::once()) + ->method('getConfig') + ->with('entitlements') + ->willReturn($e); + return $cr; + } + + /** + * @covers B::getEntitlements() + */ + public function testGetEntitlements() { + // no entitlements + $b = new B($this->getMockedInstanceWithEntitlements([])); + $k = $b->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertEmpty($k); + // two entitlements + $b = new B($this->getMockedInstanceWithEntitlements(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2'])); + $k = $b->getEntitlements(); + $this::assertInternalType("array", $k); + $this::assertCount(2, $k); + } + + /** + * @covers B::isEntitledTo() + */ + public function testIsEntitledTo() { + $b = new B($this->getMockedInstanceWithEntitlements([])); + $this::assertFalse($b->isEntitledTo('de.frankfleige.phpunit.sample3.1')); + $b = new B($this->getMockedInstanceWithEntitlements(['de.frankfleige.phpunit.sample3.1', 'de.frankfleige.phpunit.sample3.2'])); + $this::assertFalse($b->isEntitledTo('de.frankfleige.phpunit.sample3.3')); + $this::assertTrue($b->isEntitledTo('de.frankfleige.phpunit.sample3.2')); + $this::assertTrue($b->isEntitledTo('de.frankfleige.phpunit.sample3.1')); + } +} From 6b2e78cfae02b559fbd6f6b1b607dbc1472e4b23 Mon Sep 17 00:00:00 2001 From: Frank Fleige Date: Mon, 17 Jul 2017 15:02:38 +0200 Subject: [PATCH 2/3] added index file for Sample 3 --- src/Sample3/index.php | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/Sample3/index.php diff --git a/src/Sample3/index.php b/src/Sample3/index.php new file mode 100644 index 0000000..d352279 --- /dev/null +++ b/src/Sample3/index.php @@ -0,0 +1,30 @@ +initEntitlements(); +var_dump($a->getEntitlements()); + +// B +$containerBuilder = new ContainerBuilder(); +/** @noinspection PhpUnusedParameterInspection */ +$containerBuilder->addDefinitions( + [ + \FrankFleige\PHPUnitPlayground\Sample3\C::class => function(ContainerBuilder $c) { + return \FrankFleige\PHPUnitPlayground\Sample3\C::getInstance(); + } + ] +); +$container = $containerBuilder->build(); +$b = $container->get('FrankFleige\PHPUnitPlayground\Sample3\B'); +var_dump($b->getEntitlements()); From 3adab1e713d873f3fe38d0743346b7c2850c473d Mon Sep 17 00:00:00 2001 From: Frank Fleige Date: Tue, 18 Jul 2017 08:16:23 +0200 Subject: [PATCH 3/3] fixed typo --- src/Sample3/ReadMe.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sample3/ReadMe.md b/src/Sample3/ReadMe.md index 2b71ded..8b616ce 100644 --- a/src/Sample3/ReadMe.md +++ b/src/Sample3/ReadMe.md @@ -22,7 +22,7 @@ ## writing the unit tests -### the challange +### the challenge ```php public function testGetEntitlements() { $a = new A(); @@ -147,4 +147,4 @@ and inject it to B when creating an instance of it: ```php // $cr is an instance of the mocked class C $b = new B($cr); -``` \ No newline at end of file +```