From 071980460edb88cda266b7476f49a10b7a05c1c7 Mon Sep 17 00:00:00 2001 From: Tavo Nieves J Date: Tue, 18 Aug 2020 02:08:49 -0500 Subject: [PATCH 1/3] improved code quality, fixed incomplete bug test, added chained methods, added new BDD methods --- .gitignore | 1 + composer.json | 4 + src/Codeception/Specify.php | 227 +++----------------- src/Codeception/Specify/SpecifyBoostrap.php | 227 ++++++++++++++++++++ 4 files changed, 266 insertions(+), 193 deletions(-) create mode 100644 src/Codeception/Specify/SpecifyBoostrap.php diff --git a/.gitignore b/.gitignore index 285be54..87dc28f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ vendor .idea +.phpunit.result.cache composer.phar \ No newline at end of file diff --git a/composer.json b/composer.json index e3848cb..a2821f2 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,10 @@ { "name": "Michael Bodnarchuk", "email": "davert@codeception.com" + }, + { + "name": "Gustavo Nieves", + "homepage": "https://medium.com/@ganieves" } ], "require": { diff --git a/src/Codeception/Specify.php b/src/Codeception/Specify.php index 5c0d74a..fbab72b 100644 --- a/src/Codeception/Specify.php +++ b/src/Codeception/Specify.php @@ -1,224 +1,65 @@ -currentSpecifyTest; - } - - public function should($specification, \Closure $callable = null, $params = []) - { - $this->specify("should " . $specification, $callable, $params); - } - - public function it($specification, \Closure $callable = null, $params = []) - { - $this->specify($specification, $callable, $params); - } - - public function describe($specification, \Closure $callable = null) - { - $this->specify($specification, $callable); - } - - public function specify($specification, \Closure $callable = null, $params = []) - { - if (!$callable) { - return; - } - - /** @var $this TestCase **/ - if (!$this->copier) { - $this->copier = new \DeepCopy\DeepCopy(); - $this->copier->skipUncloneable(); - } - - $properties = $this->getSpecifyObjectProperties(); - - // prepare for execution - $examples = $this->getSpecifyExamples($params); - $showExamplesIndex = $examples !== [[]]; - - $specifyName = $this->specifyName; - $this->specifyName .= ' ' . $specification; - - foreach ($examples as $idx => $example) { - $test = new SpecifyTest($callable->bindTo($this)); - $this->currentSpecifyTest = $test; - $test->setName($this->getName() . ' |' . $this->specifyName); - $test->setExample($example); - if ($showExamplesIndex) { - $test->setName($this->getName() . ' |' . $this->specifyName . ' # example ' . $idx); - } - - // copy current object properties - $this->specifyCloneProperties($properties); - - if (!empty($this->beforeSpecify) && is_array($this->beforeSpecify)) { - foreach ($this->beforeSpecify as $closure) { - if ($closure instanceof \Closure) $closure->__invoke(); - } - } - - $test->run($this->getTestResultObject()); - $this->specifyCheckMockObjects(); - - // restore object properties - $this->specifyRestoreProperties($properties); - - if (!empty($this->afterSpecify) && is_array($this->afterSpecify)) { - foreach ($this->afterSpecify as $closure) { - if ($closure instanceof \Closure) $closure->__invoke(); - } - } - } - - // revert specify name - $this->specifyName = $specifyName; - } - - /** - * @param $params - * @return array - * @throws \RuntimeException - */ - private function getSpecifyExamples($params) - { - if (isset($params['examples'])) { - if (!is_array($params['examples'])) throw new \RuntimeException("Examples should be an array"); - return $params['examples']; - } - return [[]]; + use SpecifyBoostrap { + afterSpecify as public; + beforeSpecify as public; + cleanSpecify as public; + getCurrentSpecifyTest as public; } - /** - * @return \ReflectionClass|null - */ - private function specifyGetPhpUnitReflection() + public function specify(string $thing, Closure $code = null, $examples = []): ?self { - if ($this instanceof \PHPUnit\Framework\TestCase) { - return new \ReflectionClass(\PHPUnit\Framework\TestCase::class); + if ($code instanceof Closure) { + $this->runSpec($thing, $code, $examples); + return null; } + TestCase::markTestIncomplete(); + return $this; } - private function specifyCheckMockObjects() + public function describe(string $feature, Closure $code = null): ?self { - if (($phpUnitReflection = $this->specifyGetPhpUnitReflection()) !== null) { - $verifyMockObjects = $phpUnitReflection->getMethod('verifyMockObjects'); - $verifyMockObjects->setAccessible(true); - $verifyMockObjects->invoke($this); + if ($code instanceof Closure) { + $this->runSpec($feature, $code); + return null; } + return $this; } - function beforeSpecify(\Closure $callable = null) - { - $this->beforeSpecify[] = $callable->bindTo($this); - } - - function afterSpecify(\Closure $callable = null) - { - $this->afterSpecify[] = $callable->bindTo($this); - } - - function cleanSpecify() + public function it(string $specification, Closure $code = null, $examples = []): self { - $this->beforeSpecify = $this->afterSpecify = array(); - } - - /** - * @param ObjectProperty[] $properties - */ - private function specifyRestoreProperties($properties) - { - foreach ($properties as $property) { - $property->restoreValue(); + if ($code instanceof Closure) { + $this->runSpec($specification, $code, $examples); } + return $this; } - /** - * @return ObjectProperty[] - */ - private function getSpecifyObjectProperties() + public function its(string $specification, Closure $code = null, $examples = []): self { - $objectReflection = new \ReflectionObject($this); - $properties = $objectReflection->getProperties(); - - if (($classProperties = $this->specifyGetClassPrivateProperties()) !== []) { - $properties = array_merge($properties, $classProperties); - } - - $clonedProperties = []; - - foreach ($properties as $property) { - /** @var $property \ReflectionProperty **/ - $docBlock = $property->getDocComment(); - if (!$docBlock) { - continue; - } - if (preg_match('~\*(\s+)?@specify\s?~', $docBlock)) { - $property->setAccessible(true); - $clonedProperties[] = new ObjectProperty($this, $property); - } - } - - // isolate mockObjects property from PHPUnit\Framework\TestCase - if ($classReflection = $this->specifyGetPhpUnitReflection()) { - $property = $classReflection->getProperty('mockObjects'); - // remove all mock objects inherited from parent scope(s) - $clonedProperties[] = new ObjectProperty($this, $property); - $property->setValue($this, []); - } - - return $clonedProperties; + return $this->it($specification, $code, $examples); } - private function specifyGetClassPrivateProperties() + public function should(string $behavior, Closure $code = null, $examples = []): self { - static $properties = []; - - if (!isset($properties[__CLASS__])) { - $reflection = new \ReflectionClass(__CLASS__); - - $properties[__CLASS__] = (get_class($this) !== __CLASS__) - ? $reflection->getProperties(\ReflectionProperty::IS_PRIVATE) : []; + if ($code instanceof Closure) { + $this->runSpec('should ' . $behavior, $code, $examples); } - - return $properties[__CLASS__]; + return $this; } - /** - * @param ObjectProperty[] $properties - */ - private function specifyCloneProperties($properties) + public function shouldNot(string $behavior, Closure $code = null, $examples = []): self { - foreach ($properties as $property) { - $propertyValue = $property->getValue(); - $property->setValue($this->copier->copy($propertyValue)); + if ($code instanceof Closure) { + $this->runSpec('should not ' . $behavior, $code, $examples); } + return $this; } } diff --git a/src/Codeception/Specify/SpecifyBoostrap.php b/src/Codeception/Specify/SpecifyBoostrap.php new file mode 100644 index 0000000..84daa96 --- /dev/null +++ b/src/Codeception/Specify/SpecifyBoostrap.php @@ -0,0 +1,227 @@ +currentSpecifyTest; + } + /** + * @param string $specification + * @param Closure|null $callable + * @param callable|array $params + */ + private function runSpec($specification, Closure $callable = null, $params = []) + { + if (!$callable) { + return; + } + + if (!$this->copier) { + $this->copier = new DeepCopy(); + $this->copier->skipUncloneable(); + } + + $properties = $this->getSpecifyObjectProperties(); + + // prepare for execution + $examples = $this->getSpecifyExamples($params); + $showExamplesIndex = $examples !== [[]]; + + $specifyName = $this->specifyName; + $this->specifyName .= ' ' . $specification; + + foreach ($examples as $idx => $example) { + $test = new SpecifyTest($callable->bindTo($this)); + $this->currentSpecifyTest = $test; + $test->setName($this->getName() . ' |' . $this->specifyName); + $test->setExample($example); + if ($showExamplesIndex) { + $test->setName($this->getName() . ' |' . $this->specifyName . ' # example ' . $idx); + } + + // copy current object properties + $this->specifyCloneProperties($properties); + + if (!empty($this->beforeSpecify) && is_array($this->beforeSpecify)) { + foreach ($this->beforeSpecify as $closure) { + if ($closure instanceof Closure) $closure->__invoke(); + } + } + + $test->run($this->getTestResultObject()); + $this->specifyCheckMockObjects(); + + // restore object properties + $this->specifyRestoreProperties($properties); + + if (!empty($this->afterSpecify) && is_array($this->afterSpecify)) { + foreach ($this->afterSpecify as $closure) { + if ($closure instanceof Closure) $closure->__invoke(); + } + } + } + + // revert specify name + $this->specifyName = $specifyName; + } + + /** + * @param $params + * @return array + */ + private function getSpecifyExamples($params) + { + if (isset($params['examples'])) { + if (!is_array($params['examples'])) { + throw new RuntimeException("Examples should be an array"); + } + return $params['examples']; + } + return [[]]; + } + + /** + * @return ReflectionClass|null + */ + private function specifyGetPhpUnitReflection() + { + if ($this instanceof TestCase) { + return new ReflectionClass(TestCase::class); + } + return null; + } + + private function specifyCheckMockObjects() + { + if (($phpUnitReflection = $this->specifyGetPhpUnitReflection()) !== null) { + try { + $verifyMockObjects = $phpUnitReflection->getMethod('verifyMockObjects'); + $verifyMockObjects->setAccessible(true); + $verifyMockObjects->invoke($this); + } catch (ReflectionException $e) { + } + } + } + + /** + * @param ObjectProperty[] $properties + */ + private function specifyRestoreProperties($properties) + { + foreach ($properties as $property) { + $property->restoreValue(); + } + } + + /** + * @return ObjectProperty[] + */ + private function getSpecifyObjectProperties() + { + $objectReflection = new ReflectionObject($this); + $properties = $objectReflection->getProperties(); + + if (($classProperties = $this->specifyGetClassPrivateProperties()) !== []) { + $properties = array_merge($properties, $classProperties); + } + + $clonedProperties = []; + + foreach ($properties as $property) { + /** @var ReflectionProperty $property **/ + $docBlock = $property->getDocComment(); + if (!$docBlock) { + continue; + } + if (preg_match('~\*(\s+)?@specify\s?~', $docBlock)) { + $property->setAccessible(true); + $clonedProperties[] = new ObjectProperty($this, $property); + } + } + + // isolate mockObjects property from PHPUnit\Framework\TestCase + if ($classReflection = $this->specifyGetPhpUnitReflection()) { + try { + $property = $classReflection->getProperty('mockObjects'); + // remove all mock objects inherited from parent scope(s) + $clonedProperties[] = new ObjectProperty($this, $property); + $property->setValue($this, []); + } catch (ReflectionException $e) { + } + } + + return $clonedProperties; + } + + private function specifyGetClassPrivateProperties() + { + static $properties = []; + + if (!isset($properties[__CLASS__])) { + $reflection = new ReflectionClass(__CLASS__); + + $properties[__CLASS__] = (get_class($this) !== __CLASS__) + ? $reflection->getProperties(ReflectionProperty::IS_PRIVATE) : []; + } + + return $properties[__CLASS__]; + } + + /** + * @param ObjectProperty[] $properties + */ + private function specifyCloneProperties($properties) + { + foreach ($properties as $property) { + $propertyValue = $property->getValue(); + $property->setValue($this->copier->copy($propertyValue)); + } + } + + private function beforeSpecify(Closure $callable = null) + { + $this->beforeSpecify[] = $callable->bindTo($this); + } + + private function afterSpecify(Closure $callable = null) + { + $this->afterSpecify[] = $callable->bindTo($this); + } + + private function cleanSpecify() + { + $this->afterSpecify = []; + $this->beforeSpecify = []; + } +} \ No newline at end of file From 26737e6a1996e04ab75fb5da4dee35618f54d6b4 Mon Sep 17 00:00:00 2001 From: Tavo Nieves J Date: Wed, 19 Aug 2020 09:58:50 -0500 Subject: [PATCH 2/3] Fix markTestIncomplete at empty 'it' or 'should' feature. --- src/Codeception/Specify.php | 7 ++++++- src/Codeception/Specify/ObjectProperty.php | 9 ++++++--- src/Codeception/Specify/SpecifyTest.php | 7 ++++--- tests/ObjectPropertyTest.php | 16 +++++++++------- tests/SpecifyTest.php | 6 ++++-- tests/_support/SpecifyUnitTest.php | 4 +++- 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/Codeception/Specify.php b/src/Codeception/Specify.php index fbab72b..4d548e7 100644 --- a/src/Codeception/Specify.php +++ b/src/Codeception/Specify.php @@ -21,7 +21,6 @@ public function specify(string $thing, Closure $code = null, $examples = []): ?s $this->runSpec($thing, $code, $examples); return null; } - TestCase::markTestIncomplete(); return $this; } @@ -38,7 +37,9 @@ public function it(string $specification, Closure $code = null, $examples = []): { if ($code instanceof Closure) { $this->runSpec($specification, $code, $examples); + return $this; } + TestCase::markTestIncomplete(); return $this; } @@ -51,7 +52,9 @@ public function should(string $behavior, Closure $code = null, $examples = []): { if ($code instanceof Closure) { $this->runSpec('should ' . $behavior, $code, $examples); + return $this; } + TestCase::markTestIncomplete(); return $this; } @@ -59,7 +62,9 @@ public function shouldNot(string $behavior, Closure $code = null, $examples = [] { if ($code instanceof Closure) { $this->runSpec('should not ' . $behavior, $code, $examples); + return $this; } + TestCase::markTestIncomplete(); return $this; } } diff --git a/src/Codeception/Specify/ObjectProperty.php b/src/Codeception/Specify/ObjectProperty.php index 2ed08ae..357c05f 100644 --- a/src/Codeception/Specify/ObjectProperty.php +++ b/src/Codeception/Specify/ObjectProperty.php @@ -1,6 +1,9 @@ owner = $owner; $this->property = $property; - if (!($this->property instanceof \ReflectionProperty)) { - $this->property = new \ReflectionProperty($owner, $this->property); + if (!($this->property instanceof ReflectionProperty)) { + $this->property = new ReflectionProperty($owner, $this->property); } $this->property->setAccessible(true); diff --git a/src/Codeception/Specify/SpecifyTest.php b/src/Codeception/Specify/SpecifyTest.php index b4cbf0b..b5b470b 100644 --- a/src/Codeception/Specify/SpecifyTest.php +++ b/src/Codeception/Specify/SpecifyTest.php @@ -4,10 +4,11 @@ use PHPUnit\Framework\AssertionFailedError; use PHPUnit\Framework\SelfDescribing; +use PHPUnit\Framework\Test; use PHPUnit\Framework\TestResult; use SebastianBergmann\Exporter\Exporter; -class SpecifyTest implements \PHPUnit\Framework\Test, SelfDescribing +class SpecifyTest implements Test, SelfDescribing { protected $name; @@ -59,11 +60,11 @@ public function count() /** * Runs a test and collects its result in a TestResult instance. * - * @param TestResult $result + * @param TestResult|null $result * * @return TestResult */ - public function run(TestResult $result = null) : \PHPUnit\Framework\TestResult + public function run(TestResult $result = null): TestResult { try { call_user_func_array($this->test, $this->example); diff --git a/tests/ObjectPropertyTest.php b/tests/ObjectPropertyTest.php index b560527..7cd3e8a 100644 --- a/tests/ObjectPropertyTest.php +++ b/tests/ObjectPropertyTest.php @@ -1,6 +1,8 @@ prop = 'test'; - $prop = new \Codeception\Specify\ObjectProperty($this, 'prop'); + $prop = new ObjectProperty($this, 'prop'); $this->assertEquals('prop', $prop->getName()); $this->assertEquals('test', $prop->getValue()); - $prop = new \Codeception\Specify\ObjectProperty($this, 'private'); + $prop = new ObjectProperty($this, 'private'); $this->assertEquals('private', $prop->getName()); $this->assertEquals('private', $prop->getValue()); - $prop = new \Codeception\Specify\ObjectProperty( + $prop = new ObjectProperty( $this, new ReflectionProperty($this, 'private') ); @@ -30,7 +32,7 @@ public function testRestore() { $this->prop = 'test'; - $prop = new \Codeception\Specify\ObjectProperty($this, 'prop'); + $prop = new ObjectProperty($this, 'prop'); $prop->setValue('another value'); $this->assertEquals('another value', $this->prop); @@ -39,7 +41,7 @@ public function testRestore() $this->assertEquals('test', $this->prop); - $prop = new \Codeception\Specify\ObjectProperty($this, 'private'); + $prop = new ObjectProperty($this, 'private'); $prop->setValue('another private value'); $this->assertEquals('another private value', $this->private); @@ -48,7 +50,7 @@ public function testRestore() $this->assertEquals('private', $this->private); - $prop = new \Codeception\Specify\ObjectProperty($this, 'prop', 'testing'); + $prop = new ObjectProperty($this, 'prop', 'testing'); $this->assertEquals('test', $prop->getValue()); diff --git a/tests/SpecifyTest.php b/tests/SpecifyTest.php index 7d2051a..ef32291 100644 --- a/tests/SpecifyTest.php +++ b/tests/SpecifyTest.php @@ -1,6 +1,8 @@ specify('i can fail here but test goes on', function() { $this->markTestIncomplete(); }); - } catch (\PHPUnit\Framework\IncompleteTestError $e) { + } catch (IncompleteTestError $e) { $this->fail("should not be thrown"); } $this->assertTrue(true); diff --git a/tests/_support/SpecifyUnitTest.php b/tests/_support/SpecifyUnitTest.php index cf82561..ad7b51a 100644 --- a/tests/_support/SpecifyUnitTest.php +++ b/tests/_support/SpecifyUnitTest.php @@ -1,6 +1,8 @@ Date: Thu, 27 Aug 2020 11:18:17 -0500 Subject: [PATCH 3/3] Rename SpecifyBoostrap.php to SpecifyHooks.php --- src/Codeception/Specify.php | 4 ++-- .../Specify/{SpecifyBoostrap.php => SpecifyHooks.php} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/Codeception/Specify/{SpecifyBoostrap.php => SpecifyHooks.php} (99%) diff --git a/src/Codeception/Specify.php b/src/Codeception/Specify.php index 4d548e7..540a09c 100644 --- a/src/Codeception/Specify.php +++ b/src/Codeception/Specify.php @@ -3,12 +3,12 @@ namespace Codeception; use Closure; -use Codeception\Specify\SpecifyBoostrap; +use Codeception\Specify\SpecifyHooks; use PHPUnit\Framework\TestCase; trait Specify { - use SpecifyBoostrap { + use SpecifyHooks { afterSpecify as public; beforeSpecify as public; cleanSpecify as public; diff --git a/src/Codeception/Specify/SpecifyBoostrap.php b/src/Codeception/Specify/SpecifyHooks.php similarity index 99% rename from src/Codeception/Specify/SpecifyBoostrap.php rename to src/Codeception/Specify/SpecifyHooks.php index 84daa96..5f7ab2d 100644 --- a/src/Codeception/Specify/SpecifyBoostrap.php +++ b/src/Codeception/Specify/SpecifyHooks.php @@ -11,7 +11,7 @@ use ReflectionProperty; use RuntimeException; -trait SpecifyBoostrap +trait SpecifyHooks { private $afterSpecify = [];