From 1b312e2fb10f0fe41090cc46fb0745709ce3362e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Wed, 4 Jul 2018 23:55:55 +0200 Subject: [PATCH] Analyse with PHPStan. Fix bugs. --- .travis.yml | 4 +- phpstan.neon | 6 + src/Client.php | 2 +- src/DomCrawler/Crawler.php | 3 + src/DomCrawler/Field/ChoiceFormField.php | 8 +- src/DomCrawler/Form.php | 10 +- src/PanthereTestCase.php | 110 +--------------- src/PanthereTestCaseTrait.php | 117 ++++++++++++++++++ src/ProcessManager/ChromeManager.php | 2 - tests/DomCrawler/FormTest.php | 31 ++++- tests/ProcessManager/ChromeManagerTest.php | 8 +- tests/ProcessManager/WebServerManagerTest.php | 8 +- tests/TestCase.php | 3 - 13 files changed, 180 insertions(+), 132 deletions(-) create mode 100644 phpstan.neon create mode 100644 src/PanthereTestCaseTrait.php diff --git a/.travis.yml b/.travis.yml index 0d2a9b1b..f925ed12 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,8 @@ cache: - $HOME/.composer/cache before_install: - - wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.11.1/php-cs-fixer.phar + - wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.12.1/php-cs-fixer.phar + - wget https://github.com/phpstan/phpstan/releases/download/0.10.1/phpstan.phar before_script: - phpenv config-rm xdebug.ini @@ -19,3 +20,4 @@ before_script: script: - phpunit - php php-cs-fixer.phar fix --dry-run --diff --no-ansi + - php phpstan.phar analyse src tests --level=6 diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 00000000..5a7fde1e --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,6 @@ +parameters: + ignoreErrors: + # https://github.com/symfony/symfony/pull/27849 + - '/Parameter #1 \$value of method Symfony\\Component\\DomCrawler\\Field\\ChoiceFormField::select\(\) expects string, array given\./' + # Require a redesign of the underlying Symfony components + - '#Panthere\\[a-zA-Z\\]+::__construct\(\) does not call parent constructor from Symfony\\Component\\(BrowserKit|DomCrawler)\\[a-zA-Z]+\.#' diff --git a/src/Client.php b/src/Client.php index a72063e9..a1fb9619 100644 --- a/src/Client.php +++ b/src/Client.php @@ -38,7 +38,7 @@ final class Client extends BaseClient implements WebDriver use ExceptionThrower; /** - * @var WebDriver + * @var WebDriver|null */ private $webDriver; private $browserManager; diff --git a/src/DomCrawler/Crawler.php b/src/DomCrawler/Crawler.php index f53f5841..f0558c4f 100644 --- a/src/DomCrawler/Crawler.php +++ b/src/DomCrawler/Crawler.php @@ -17,6 +17,7 @@ use Facebook\WebDriver\WebDriver; use Facebook\WebDriver\WebDriverBy; use Facebook\WebDriver\WebDriverElement; +use Panthere\ExceptionThrower; use Symfony\Component\DomCrawler\Crawler as BaseCrawler; /** @@ -24,6 +25,8 @@ */ final class Crawler extends BaseCrawler implements WebDriverElement { + use ExceptionThrower; + private $elements; private $webDriver; diff --git a/src/DomCrawler/Field/ChoiceFormField.php b/src/DomCrawler/Field/ChoiceFormField.php index 452dcb9b..1a8bf587 100644 --- a/src/DomCrawler/Field/ChoiceFormField.php +++ b/src/DomCrawler/Field/ChoiceFormField.php @@ -78,13 +78,17 @@ public function untick() /** * Sets the value of the field. * - * @param string $value The value of the field + * @param string|array|bool $value The value of the field * * @throws \InvalidArgumentException When value type provided is not correct */ public function setValue($value) { - if ('checkbox' === $this->type && \is_bool($value)) { + if (\is_bool($value)) { + if ('checkbox' !== $this->type) { + throw new \InvalidArgumentException(\sprintf('Invalid argument of type "%s"', \gettype($value))); + } + if ($value) { if (!$this->element->isSelected()) { $this->element->click(); diff --git a/src/DomCrawler/Form.php b/src/DomCrawler/Form.php index f9eb0be8..5ad824c6 100644 --- a/src/DomCrawler/Form.php +++ b/src/DomCrawler/Form.php @@ -126,7 +126,7 @@ public function getValues() } // Flatten non array-checkboxes - if (!$isArrayElement && 1 === \count($value)) { + if (\is_array($value) && !$isArrayElement && 1 === \count($value)) { $value = $value[0]; } } @@ -300,10 +300,12 @@ private function setValue(string $name, $value): void try { $element = $this->element->findElement(WebDriverBy::name($name)); } catch (NoSuchElementException $e) { - // Compatibility with the DomCrawler API - if (\is_array($value)) { - $element = $this->element->findElement(WebDriverBy::name($name.'[]')); + if (!\is_array($value)) { + throw $e; } + + // Compatibility with the DomCrawler API + $element = $this->element->findElement(WebDriverBy::name($name.'[]')); } if (null === $webDriverSelect = $this->getWebDriverSelect($element)) { diff --git a/src/PanthereTestCase.php b/src/PanthereTestCase.php index 9dfa3441..86e6e364 100644 --- a/src/PanthereTestCase.php +++ b/src/PanthereTestCase.php @@ -13,119 +13,17 @@ namespace Panthere; -use Goutte\Client as GoutteClient; -use GuzzleHttp\Client as GuzzleClient; -use Panthere\Client as PanthereClient; -use Panthere\ProcessManager\WebServerManager; use PHPUnit\Framework\TestCase; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; if (\class_exists(WebTestCase::class)) { - /** - * @internal - */ - abstract class InternalTestCase extends WebTestCase + abstract class PanthereTestCase extends WebTestCase { + use PanthereTestCaseTrait; } } else { - /** - * @internal - */ - abstract class InternalTestCase extends TestCase + abstract class PanthereTestCase extends TestCase { - } -} - -/** - * @author Kévin Dunglas - */ -abstract class PanthereTestCase extends InternalTestCase -{ - /** - * @var string|null - */ - protected static $webServerDir; - - /** - * @var WebServerManager|null - */ - protected static $webServerManager; - - /** - * @var string|null - */ - protected static $baseUri; - - /** - * @var GoutteClient|null - */ - protected static $goutteClient; - - /** - * @var PanthereClient|null - */ - protected static $panthereClient; - - public static function tearDownAfterClass() - { - if (null !== self::$webServerManager) { - self::$webServerManager->quit(); - self::$webServerManager = null; - } - - if (null !== self::$panthereClient) { - self::$panthereClient->quit(); - self::$panthereClient = null; - } - - if (null !== self::$goutteClient) { - self::$goutteClient = null; - } - - self::$baseUri = null; - } - - protected static function startWebServer(?string $webServerDir = null, string $hostname = '127.0.0.1', int $port = 9000): void - { - if (null !== static::$webServerManager) { - return; - } - - if (null === $webServerDir) { - // Try the local $webServerDir property, or the PANTHERE_WEB_SERVER_DIR env var or default to the Flex directory structure - $webServerDir = static::$webServerDir ?? $_ENV['PANTHERE_WEB_SERVER_DIR'] ?? __DIR__.'/../../../../public'; - } - - self::$webServerManager = new WebServerManager($webServerDir, $hostname, $port); - self::$webServerManager->start(); - - self::$baseUri = "http://$hostname:$port"; - } - - protected static function createPanthereClient(string $hostname = '127.0.0.1', int $port = 9000): PanthereClient - { - self::startWebServer(null, $hostname, $port); - if (null === self::$panthereClient) { - self::$panthereClient = Client::createChromeClient(null, null, [], self::$baseUri); - } - - return self::$panthereClient; - } - - protected static function createGoutteClient(): GoutteClient - { - if (!\class_exists(GoutteClient::class)) { - throw new \RuntimeException('Goutte is not installed. Run "composer req fabpot/goutte".'); - } - - self::startWebServer(); - if (null === self::$goutteClient) { - $goutteClient = new GoutteClient(); - $goutteClient->setClient(new GuzzleClient(['base_uri' => self::$baseUri])); - - self::$goutteClient = $goutteClient; - } - - return self::$goutteClient; + use PanthereTestCaseTrait; } } diff --git a/src/PanthereTestCaseTrait.php b/src/PanthereTestCaseTrait.php new file mode 100644 index 00000000..73abafad --- /dev/null +++ b/src/PanthereTestCaseTrait.php @@ -0,0 +1,117 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Panthere; + +use Goutte\Client as GoutteClient; +use GuzzleHttp\Client as GuzzleClient; +use Panthere\Client as PanthereClient; +use Panthere\ProcessManager\WebServerManager; + +/** + * Eases conditional class definition. + * + * @internal + * + * @author Kévin Dunglas + */ +trait PanthereTestCaseTrait +{ + /** + * @var string|null + */ + protected static $webServerDir; + + /** + * @var WebServerManager|null + */ + protected static $webServerManager; + + /** + * @var string|null + */ + protected static $baseUri; + + /** + * @var GoutteClient|null + */ + protected static $goutteClient; + + /** + * @var PanthereClient|null + */ + protected static $panthereClient; + + public static function tearDownAfterClass() + { + if (null !== self::$webServerManager) { + self::$webServerManager->quit(); + self::$webServerManager = null; + } + + if (null !== self::$panthereClient) { + self::$panthereClient->quit(); + self::$panthereClient = null; + } + + if (null !== self::$goutteClient) { + self::$goutteClient = null; + } + + self::$baseUri = null; + } + + protected static function startWebServer(?string $webServerDir = null, string $hostname = '127.0.0.1', int $port = 9000): void + { + if (null !== static::$webServerManager) { + return; + } + + if (null === $webServerDir) { + // Try the local $webServerDir property, or the PANTHERE_WEB_SERVER_DIR env var or default to the Flex directory structure + $webServerDir = static::$webServerDir ?? $_ENV['PANTHERE_WEB_SERVER_DIR'] ?? __DIR__.'/../../../../public'; + } + + self::$webServerManager = new WebServerManager($webServerDir, $hostname, $port); + self::$webServerManager->start(); + + self::$baseUri = "http://$hostname:$port"; + } + + protected static function createPanthereClient(string $hostname = '127.0.0.1', int $port = 9000): PanthereClient + { + self::startWebServer(null, $hostname, $port); + if (null === self::$panthereClient) { + self::$panthereClient = Client::createChromeClient(null, null, [], self::$baseUri); + } + + return self::$panthereClient; + } + + protected static function createGoutteClient(): GoutteClient + { + if (!\class_exists(GoutteClient::class)) { + throw new \RuntimeException('Goutte is not installed. Run "composer req fabpot/goutte".'); + } + + self::startWebServer(); + if (null === self::$goutteClient) { + $goutteClient = new GoutteClient(); + $goutteClient->setClient(new GuzzleClient(['base_uri' => self::$baseUri])); + + self::$goutteClient = $goutteClient; + } + + return self::$goutteClient; + } +} diff --git a/src/ProcessManager/ChromeManager.php b/src/ProcessManager/ChromeManager.php index 80171a36..9a4e5bc2 100644 --- a/src/ProcessManager/ChromeManager.php +++ b/src/ProcessManager/ChromeManager.php @@ -38,8 +38,6 @@ public function __construct(?string $chromeDriverBinary = null, ?array $argument } /** - * @param string[]|null $arguments - * * @throws \RuntimeException */ public function start(): WebDriver diff --git a/tests/DomCrawler/FormTest.php b/tests/DomCrawler/FormTest.php index 11fc9986..ad4be76d 100644 --- a/tests/DomCrawler/FormTest.php +++ b/tests/DomCrawler/FormTest.php @@ -14,6 +14,7 @@ namespace Panthere\Tests\DomCrawler; use Panthere\Tests\TestCase; +use Symfony\Component\DomCrawler\Field\ChoiceFormField; /** * @author Kévin Dunglas @@ -75,17 +76,27 @@ public function testFormById(callable $clientFactory): void public function testFormFields(callable $clientFactory) { $crawler = $this->request($clientFactory, '/form.html'); + $form = $crawler->filter('#another-form')->form(); - $form['j3']->select('j3a'); + + /** + * @var ChoiceFormField + */ + $j3 = $form['j3']; + $j3->select('j3a'); $originalValues = $form->getValues(); unset($originalValues['single-cb']); - $form['single-cb']->tick(); + /** + * @var ChoiceFormField + */ + $singleCb = $form['single-cb']; + $singleCb->tick(); $this->assertSame($originalValues + ['single-cb' => 'hello'], $form->getValues()); $this->assertSame('hello', $form['single-cb']->getValue()); - $form['single-cb']->untick(); + $singleCb->untick(); $this->assertSame($originalValues, $form->getValues()); } @@ -99,8 +110,18 @@ public function testSelect(callable $clientFactory) $form['i1']->setValue('Durruti'); $form['i2']->setValue('Бакунин'); $form['i3']->setValue('Ferrer'); - $form['i4']->select('i4b'); - $form['i5']->select(['i5b', 'i5c']); + + /** + * @var ChoiceFormField + */ + $i4 = $form['i4']; + $i4->select('i4b'); + + /** + * @var ChoiceFormField + */ + $i5 = $form['i5']; + $i5->select(['i5b', 'i5c']); $this->assertSame([ 'i1' => 'Durruti', diff --git a/tests/ProcessManager/ChromeManagerTest.php b/tests/ProcessManager/ChromeManagerTest.php index e13b55ed..0a681fae 100644 --- a/tests/ProcessManager/ChromeManagerTest.php +++ b/tests/ProcessManager/ChromeManagerTest.php @@ -35,11 +35,11 @@ public function testRun() */ public function testAlreadyRunning() { - try { - $driver1 = new ChromeManager(); - $driver1->start(); + $driver1 = new ChromeManager(); + $driver1->start(); - $driver2 = new ChromeManager(); + $driver2 = new ChromeManager(); + try { $driver2->start(); } finally { $driver1->quit(); diff --git a/tests/ProcessManager/WebServerManagerTest.php b/tests/ProcessManager/WebServerManagerTest.php index 1acef4b7..2e62e4e7 100644 --- a/tests/ProcessManager/WebServerManagerTest.php +++ b/tests/ProcessManager/WebServerManagerTest.php @@ -36,11 +36,11 @@ public function testRun() */ public function testAlreadyRunning() { - try { - $server1 = new WebServerManager(__DIR__.'/../fixtures/', '127.0.0.1', 1234); - $server1->start(); + $server1 = new WebServerManager(__DIR__.'/../fixtures/', '127.0.0.1', 1234); + $server1->start(); - $server2 = new WebServerManager(__DIR__.'/../fixtures/', '127.0.0.1', 1234); + $server2 = new WebServerManager(__DIR__.'/../fixtures/', '127.0.0.1', 1234); + try { $server2->start(); } finally { $server1->quit(); diff --git a/tests/TestCase.php b/tests/TestCase.php index 6ea7a0b1..82f78e8b 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -25,9 +25,6 @@ abstract class TestCase extends PanthereTestCase { protected static $webServerDir = __DIR__.'/fixtures'; - /** - * @return callable[] - */ public function clientFactoryProvider(): array { // Tests must pass with both Panthere and Goutte