From a1975528694b6de51b8a0b523e3b4c88966f14b3 Mon Sep 17 00:00:00 2001 From: Ondrej Machulda Date: Sun, 29 Nov 2020 04:28:47 +0100 Subject: [PATCH 1/3] Replace deprecated AnnotationsParser from Nette with BetterReflection --- composer.json | 1 + easy-coding-standard.yaml | 1 + .../Utils/Annotations/ClassParserTest.php | 45 +++++++++++++++++++ .../Annotations/Fixtures/ClassNoDocBlock.php | 7 +++ .../Fixtures/MultipleClassesInFile.php | 11 +++++ .../Annotations/Fixtures/NoClassInFile.php | 9 ++++ src-tests/Utils/StringsTest.php | 2 +- .../EventListener/ListenerInstantiator.php | 6 +-- src/Process/ProcessSetCreator.php | 27 ++--------- src/Utils/Annotations/ClassParser.php | 41 +++++++++++++++++ 10 files changed, 121 insertions(+), 29 deletions(-) create mode 100644 src-tests/Utils/Annotations/ClassParserTest.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassNoDocBlock.php create mode 100644 src-tests/Utils/Annotations/Fixtures/MultipleClassesInFile.php create mode 100644 src-tests/Utils/Annotations/Fixtures/NoClassInFile.php create mode 100644 src/Utils/Annotations/ClassParser.php diff --git a/composer.json b/composer.json index 3f2b23cd..a8bd3060 100644 --- a/composer.json +++ b/composer.json @@ -36,6 +36,7 @@ "ondram/ci-detector": "^3.1", "php-webdriver/webdriver": "^1.8.1", "phpunit/phpunit": "^7.5.20", + "roave/better-reflection": "^4.3", "symfony/console": "^4.0", "symfony/event-dispatcher": "^4.0", "symfony/filesystem": "^4.0", diff --git a/easy-coding-standard.yaml b/easy-coding-standard.yaml index 075cbc6c..fef76b94 100644 --- a/easy-coding-standard.yaml +++ b/easy-coding-standard.yaml @@ -18,3 +18,4 @@ parameters: exclude_files: - 'src-tests/coverage/*' - 'src-tests/FunctionalTests/logs/coverage/*' + - 'src-tests/Utils/Annotations/Fixtures/*' diff --git a/src-tests/Utils/Annotations/ClassParserTest.php b/src-tests/Utils/Annotations/ClassParserTest.php new file mode 100644 index 00000000..21195be4 --- /dev/null +++ b/src-tests/Utils/Annotations/ClassParserTest.php @@ -0,0 +1,45 @@ +createFileInfo('ClassNoDocBlock.php'); + + $className = ClassParser::readClassNameFromFile($file); + + $this->assertSame('Lmc\Steward\Utils\Annotations\Fixtures\ClassNoDocBlock', $className); + } + + public function testShouldThrowExceptionForMultipleClassesInOneFile(): void + { + $file = $this->createFileInfo('MultipleClassesInFile.php'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageRegExp('/^File ".+MultipleClassesInFile.php" contains definition of 2 classes\./'); + ClassParser::readClassNameFromFile($file); + } + + public function testShouldThrowExceptionForNoClassInOneFile(): void + { + $file = $this->createFileInfo('NoClassInFile.php'); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageRegExp('/^No class found in file ".+NoClassInFile.php"/'); + ClassParser::readClassNameFromFile($file); + } + + private function createFileInfo(string $fileName): SplFileInfo + { + return new SplFileInfo(__DIR__ . '/Fixtures/' . $fileName, 'Fixtures/', 'Fixtures/' . $fileName); + } +} diff --git a/src-tests/Utils/Annotations/Fixtures/ClassNoDocBlock.php b/src-tests/Utils/Annotations/Fixtures/ClassNoDocBlock.php new file mode 100644 index 00000000..f3ecb4b0 --- /dev/null +++ b/src-tests/Utils/Annotations/Fixtures/ClassNoDocBlock.php @@ -0,0 +1,7 @@ +assertSame($expectedFilename, Strings::toFilename($string)); } diff --git a/src/Console/EventListener/ListenerInstantiator.php b/src/Console/EventListener/ListenerInstantiator.php index dfa3dcd2..4dd80885 100644 --- a/src/Console/EventListener/ListenerInstantiator.php +++ b/src/Console/EventListener/ListenerInstantiator.php @@ -2,11 +2,10 @@ namespace Lmc\Steward\Console\EventListener; -use Nette\Reflection\AnnotationsParser; +use Lmc\Steward\Utils\Annotations\ClassParser; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Finder\Finder; -use Symfony\Component\Finder\SplFileInfo; /** * Search and instantiate event-listeners for the commands @@ -48,9 +47,8 @@ protected function searchListeners(string $dir): array ->name('*Listener.php'); $listeners = []; - /** @var SplFileInfo $file */ foreach ($files as $file) { - $listeners[] = key(AnnotationsParser::parsePhp(\file_get_contents($file->getRealPath()))); + $listeners[] = ClassParser::readClassNameFromFile($file); } return $listeners; diff --git a/src/Process/ProcessSetCreator.php b/src/Process/ProcessSetCreator.php index bb52f023..ed295572 100644 --- a/src/Process/ProcessSetCreator.php +++ b/src/Process/ProcessSetCreator.php @@ -7,6 +7,7 @@ use Lmc\Steward\Console\Configuration\ConfigOptions; use Lmc\Steward\Console\Event\RunTestsProcessEvent; use Lmc\Steward\Publisher\AbstractPublisher; +use Lmc\Steward\Utils\Annotations\ClassParser; use Lmc\Steward\Utils\Strings; use Nette\Reflection\AnnotationsParser; use Symfony\Component\Console\Input\InputInterface; @@ -84,7 +85,8 @@ public function createFromFiles( /** @var SplFileInfo $file */ foreach ($files as $file) { $fileName = $file->getRealPath(); - $className = $this->getClassNameFromFile($fileName); + $className = ClassParser::readClassNameFromFile($file); + $annotations = $this->getClassAnnotations($className, $fileName); if ($excludingGroups = $this->getExcludingGroups($excludeGroups, $annotations)) { @@ -216,29 +218,6 @@ private function getExcludingGroups(array $excludeGroups, array $annotations): a return $excludingGroups; } - private function getClassNameFromFile(string $fileName): string - { - // Parse classes from the testcase file - $classes = AnnotationsParser::parsePhp(\file_get_contents($fileName)); - - if (empty($classes)) { - throw new \RuntimeException(sprintf('No class found in file "%s"', $fileName)); - } - - if (count($classes) > 1) { - throw new \RuntimeException( - sprintf( - 'File "%s" contains definition of %d classes. However, each class must be defined in its own' - . ' separate file.', - $fileName, - count($classes) - ) - ); - } - - return key($classes); - } - private function setupProcessDelays(ProcessWrapper $processWrapper, array $annotations): void { $delayAfter = !empty($annotations['delayAfter']) ? current($annotations['delayAfter']) : ''; diff --git a/src/Utils/Annotations/ClassParser.php b/src/Utils/Annotations/ClassParser.php new file mode 100644 index 00000000..372ee1b4 --- /dev/null +++ b/src/Utils/Annotations/ClassParser.php @@ -0,0 +1,41 @@ +getRealPath(), (new BetterReflection())->astLocator()) + ); + + $classesInFile = $reflection->getAllClasses(); + self::assertOneClassInFile($classesInFile, $file); + + return $classesInFile[0]->getName(); + } + + private static function assertOneClassInFile(array $classesInFile, SplFileInfo $file): void + { + if (count($classesInFile) === 0) { + throw new \RuntimeException(sprintf('No class found in file "%s"', $file->getRelativePathname())); + } + + if (count($classesInFile) > 1) { + throw new \RuntimeException( + sprintf( + 'File "%s" contains definition of %d classes. However, each class must be defined in its own' + . ' separate file.', + $file->getRelativePathname(), + count($classesInFile) + ) + ); + } + } +} From d7b999ede87726d6e438920886ec7ed634347421 Mon Sep 17 00:00:00 2001 From: Ondrej Machulda Date: Sun, 29 Nov 2020 14:31:19 +0100 Subject: [PATCH 2/3] Read class annotations using phpdocumentor, remove deprecated nette/reflection --- CHANGELOG.md | 1 + composer.json | 2 +- .../Annotations/ClassAnnotationsTest.php | 136 ++++++++++++++++++ .../Fixtures/ClassEmptyDockBlock.php | 10 ++ .../Fixtures/ClassKeyAnnotations.php | 15 ++ .../Fixtures/ClassKeyValueAnnotations.php | 13 ++ .../Fixtures/ClassMixedAnnotations.php | 14 ++ .../Annotations/Fixtures/ClassWithMethods.php | 56 ++++++++ src/Listener/WebDriverListener.php | 6 +- src/Process/ProcessSetCreator.php | 8 +- src/Utils/Annotations/ClassAnnotations.php | 60 ++++++++ 11 files changed, 314 insertions(+), 7 deletions(-) create mode 100644 src-tests/Utils/Annotations/ClassAnnotationsTest.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassEmptyDockBlock.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassKeyAnnotations.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassKeyValueAnnotations.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassMixedAnnotations.php create mode 100644 src-tests/Utils/Annotations/Fixtures/ClassWithMethods.php create mode 100644 src/Utils/Annotations/ClassAnnotations.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e6bb52d..616d07ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Don't hardcode timezone to `Europe/Prague`. Timezone is now used based on your PHP settings ([date.timezone](https://php.net/manual/en/datetime.configuration.php#ini.date.timezone)). - Server URL now must be provided including URL prefix (if it has one, like `/wd/hub`) - eg. `http://foo.bar:4444/wd/hub`. This means the `/wd/hub` part is now never auto-amended. - Package `symfony/polyfill-mbstring` now provides mbstring functions even if PHP mbstring extension is not installed. +- Read annotations (like `@group`, `@noBrowser` etc.) using different and more-robust underlying library. ### Fixed - Remote server running in W3C-protocol mode (eg. Selenium v3.5.3+) was erroneously detected as BrowserStack cloud service. diff --git a/composer.json b/composer.json index a8bd3060..95dcfca3 100644 --- a/composer.json +++ b/composer.json @@ -32,9 +32,9 @@ "doctrine/inflector": "^2.0.3", "florianwolters/component-util-singleton": "0.3.2", "graphp/algorithms": "^0.8.1", - "nette/reflection": "^2.4.2", "ondram/ci-detector": "^3.1", "php-webdriver/webdriver": "^1.8.1", + "phpdocumentor/reflection-docblock": "^5.2", "phpunit/phpunit": "^7.5.20", "roave/better-reflection": "^4.3", "symfony/console": "^4.0", diff --git a/src-tests/Utils/Annotations/ClassAnnotationsTest.php b/src-tests/Utils/Annotations/ClassAnnotationsTest.php new file mode 100644 index 00000000..50b0f0d4 --- /dev/null +++ b/src-tests/Utils/Annotations/ClassAnnotationsTest.php @@ -0,0 +1,136 @@ +assertSame($expectedAnnotations, $annotations); + } + + /** + * @return array[] + */ + public function provideClass(): array + { + return [ + 'Class with empty doc block' => [ + ClassEmptyDockBlock::class, + [], + ], + 'Class having no doc block at all' => [ + ClassNoDocBlock::class, + [], + ], + 'Class with key-only annotations, without values' => [ + ClassKeyAnnotations::class, + [ + 'first' => ['', ''], + 'second' => [''], + 'third' => [''], + ], + ], + 'Class with key-value annotations' => [ + ClassKeyValueAnnotations::class, + [ + 'first' => ['First value', 'Second value of first'], + 'second' => ['Second value'], + 'third' => ['Third "special" value'], + ], + ], + 'Class with mixed key-only and key-value annotations' => [ + ClassMixedAnnotations::class, + [ + 'first' => ['', 'First with some value'], + 'second' => [''], + 'third' => [''], + 'fourth' => ['Fourth value'], + ], + ], + ]; + } + + public function testShouldGetAnnotationsForInstances(): void + { + $annotations = ClassAnnotations::getAnnotationsForInstance(new ClassKeyAnnotations()); + + $this->assertSame( + [ + 'first' => ['', ''], + 'second' => [''], + 'third' => [''], + ], + $annotations + ); + } + + /** + * @dataProvider provideMethod + */ + public function testShouldGetAnnotationsForMethodInClass(string $methodName, array $expectedAnnotations): void + { + $classInstance = new ClassWithMethods(); + + $annotations = ClassAnnotations::getAnnotationsForMethodOfInstance($classInstance, $methodName); + + $this->assertSame($expectedAnnotations, $annotations); + } + + /** + * @return array[] + */ + public function provideMethod(): array + { + return [ + 'Method with empty doc block' => [ + 'methodWithEmptyAnnotations', + [], + ], + 'Method having no doc block at all' => [ + 'methodWithout', + [], + ], + 'Method with key-only annotations, without values' => [ + 'methodWithKeys', + [ + 'first' => ['', ''], + 'second' => [''], + 'third' => [''], + ], + ], + 'Method with key-value annotations' => [ + 'methodWithKeyValues', + [ + 'first' => ['First value', 'Second value of first'], + 'second' => ['Second value'], + 'third' => ['Third "special" @value!'], + ], + ], + 'Method with mixed key-only and key-value annotations' => [ + 'methodWithMixedKeyValues', + [ + 'first' => ['', 'First with some value'], + 'second' => ['Second with value', ''], + 'third' => [''], + ], + ], + ]; + } +} diff --git a/src-tests/Utils/Annotations/Fixtures/ClassEmptyDockBlock.php b/src-tests/Utils/Annotations/Fixtures/ClassEmptyDockBlock.php new file mode 100644 index 00000000..32935136 --- /dev/null +++ b/src-tests/Utils/Annotations/Fixtures/ClassEmptyDockBlock.php @@ -0,0 +1,10 @@ +getName(false))); + $testCaseAnnotations = ClassAnnotations::getAnnotationsForInstance($test); + $testAnnotations = ClassAnnotations::getAnnotationsForMethodOfInstance($test, $test->getName(false)); if (isset($testCaseAnnotations[self::NO_BROWSER_ANNOTATION]) || isset($testAnnotations[self::NO_BROWSER_ANNOTATION]) diff --git a/src/Process/ProcessSetCreator.php b/src/Process/ProcessSetCreator.php index ed295572..ef9b700c 100644 --- a/src/Process/ProcessSetCreator.php +++ b/src/Process/ProcessSetCreator.php @@ -7,9 +7,9 @@ use Lmc\Steward\Console\Configuration\ConfigOptions; use Lmc\Steward\Console\Event\RunTestsProcessEvent; use Lmc\Steward\Publisher\AbstractPublisher; +use Lmc\Steward\Utils\Annotations\ClassAnnotations; use Lmc\Steward\Utils\Annotations\ClassParser; use Lmc\Steward\Utils\Strings; -use Nette\Reflection\AnnotationsParser; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Finder\Finder; @@ -243,7 +243,7 @@ private function setupProcessDelays(ProcessWrapper $processWrapper, array $annot private function getClassAnnotations(string $className, string $fileName): array { try { - return AnnotationsParser::getAll(new \ReflectionClass($className)); + return ClassAnnotations::getAnnotationsForClass($className); } catch (\ReflectionException $e) { throw new \RuntimeException( sprintf( @@ -251,7 +251,9 @@ private function getClassAnnotations(string $className, string $fileName): array . 'the file path.', $className, $fileName - ) + ), + 0, + $e ); } } diff --git a/src/Utils/Annotations/ClassAnnotations.php b/src/Utils/Annotations/ClassAnnotations.php new file mode 100644 index 00000000..cd3e60fc --- /dev/null +++ b/src/Utils/Annotations/ClassAnnotations.php @@ -0,0 +1,60 @@ +getDocComment())) { + return []; + } + + $docBlockFactory = DocBlockFactory::createInstance(); + $docBlock = $docBlockFactory->create($reflectionClass); + + $annotations = []; + + /** @var BaseTag $tag */ + foreach ($docBlock->getTags() as $tag) { + $annotationToAdd = ''; + if ($tag->getDescription() !== null) { + $annotationToAdd = $tag->getDescription()->getBodyTemplate(); + } + + $annotations[$tag->getName()][] = $annotationToAdd; + } + + return $annotations; + } +} From ea97682443efcdea113efabaaa506e8108119608 Mon Sep 17 00:00:00 2001 From: Ondrej Machulda Date: Sun, 29 Nov 2020 15:26:46 +0100 Subject: [PATCH 3/3] Remove redundant tests --- .../MultipleClassesInFileTest.php | 16 -------------- .../Fixtures/InvalidTests/NoClassTest.php | 2 -- src-tests/Process/ProcessSetCreatorTest.php | 21 ------------------- src/Process/ProcessSetCreator.php | 3 +-- 4 files changed, 1 insertion(+), 41 deletions(-) delete mode 100644 src-tests/Process/Fixtures/InvalidTests/MultipleClassesInFileTest.php delete mode 100644 src-tests/Process/Fixtures/InvalidTests/NoClassTest.php diff --git a/src-tests/Process/Fixtures/InvalidTests/MultipleClassesInFileTest.php b/src-tests/Process/Fixtures/InvalidTests/MultipleClassesInFileTest.php deleted file mode 100644 index 0b7fc08c..00000000 --- a/src-tests/Process/Fixtures/InvalidTests/MultipleClassesInFileTest.php +++ /dev/null @@ -1,16 +0,0 @@ -assertSame('/foo/bar/logs', $testEnv['LOGS_DIR']); } - public function testShouldThrowExceptionIfAddingFileWithNoClass(): void - { - $files = $this->findDummyTests('NoClassTest.php', 'InvalidTests'); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessageRegExp('/No class found in file ".*NoClassTest.php"/'); - - $this->creator->createFromFiles($files, [], []); - } - public function testShouldThrowExceptionIfAddingClassWithNameMismatchingTheFileName(): void { $files = $this->findDummyTests('WrongClassTest.php', 'InvalidTests'); @@ -143,17 +133,6 @@ public function testShouldThrowExceptionIfAddingClassWithNameMismatchingTheFileN $this->creator->createFromFiles($files, [], []); } - public function testShouldThrowExceptionIfMultipleClassesAreDefinedInFile(): void - { - $files = $this->findDummyTests('MultipleClassesInFileTest.php', 'InvalidTests'); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessageRegExp( - '/File ".*MultipleClassesInFileTest.php" contains definition of 2 classes/' - ); - $this->creator->createFromFiles($files, [], []); - } - public function testShouldOnlyAddTestsOfGivenGroups(): void { $processSet = $this->creator->createFromFiles($this->findDummyTests(), ['bar', 'foo'], []); diff --git a/src/Process/ProcessSetCreator.php b/src/Process/ProcessSetCreator.php index ef9b700c..5918daee 100644 --- a/src/Process/ProcessSetCreator.php +++ b/src/Process/ProcessSetCreator.php @@ -86,7 +86,6 @@ public function createFromFiles( foreach ($files as $file) { $fileName = $file->getRealPath(); $className = ClassParser::readClassNameFromFile($file); - $annotations = $this->getClassAnnotations($className, $fileName); if ($excludingGroups = $this->getExcludingGroups($excludeGroups, $annotations)) { @@ -238,7 +237,7 @@ private function setupProcessDelays(ProcessWrapper $processWrapper, array $annot } /** - * Get annotations for the first class in testcase (one file = one class) + * Get annotations for the class */ private function getClassAnnotations(string $className, string $fileName): array {