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 3f2b23cd..95dcfca3 100644 --- a/composer.json +++ b/composer.json @@ -32,10 +32,11 @@ "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", "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/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-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/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/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 @@ +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/Listener/WebDriverListener.php b/src/Listener/WebDriverListener.php index e5c47fc7..17874834 100644 --- a/src/Listener/WebDriverListener.php +++ b/src/Listener/WebDriverListener.php @@ -8,9 +8,9 @@ use Lmc\Steward\ConfigProvider; use Lmc\Steward\Selenium\CapabilitiesResolver; use Lmc\Steward\Test\AbstractTestCase; +use Lmc\Steward\Utils\Annotations\ClassAnnotations; use Lmc\Steward\WebDriver\NullWebDriver; use Lmc\Steward\WebDriver\RemoteWebDriver; -use Nette\Reflection\AnnotationsParser; use PHPUnit\Framework\Test; use PHPUnit\Framework\TestListener; use PHPUnit\Framework\TestListenerDefaultImplementation; @@ -65,8 +65,8 @@ public function startTest(Test $test): void } // Initialize NullWebDriver if self::NO_BROWSER_ANNOTATION is used on testcase class or test method - $testCaseAnnotations = AnnotationsParser::getAll(new \ReflectionClass($test)); - $testAnnotations = AnnotationsParser::getAll(new \ReflectionMethod($test, $test->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 bb52f023..5918daee 100644 --- a/src/Process/ProcessSetCreator.php +++ b/src/Process/ProcessSetCreator.php @@ -7,8 +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; @@ -84,7 +85,7 @@ 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 +217,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']) : ''; @@ -259,12 +237,12 @@ 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 { try { - return AnnotationsParser::getAll(new \ReflectionClass($className)); + return ClassAnnotations::getAnnotationsForClass($className); } catch (\ReflectionException $e) { throw new \RuntimeException( sprintf( @@ -272,7 +250,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; + } +} 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) + ) + ); + } + } +}