Merge pull request #261 from lmc-eu/feature/deprecations

Remove deprecations from uderlying libraries
lmc-eu · Nov 29, 2020 · 1aabbce · 1aabbce
2 parents f4f7869 + 275dc65
commit 1aabbce
Show file tree

Hide file tree

Showing 11 changed files with 58 additions and 142 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,10 +32,12 @@
     - Instead of `TestUtils::setSelect2Value()` use directly the new `Select2` component and `selectByVisiblePartialText()` method.
     - Instead of `TestUtils::getFixturePath()` use `Facebook\WebDriver\Remote\FileDetector` instead.
     - Instead of `TestUtils::sleep()` use `AbstractTestCase::sleep()` method instead.
-- `getConfig()` method of ConfigProvider. Instead call the property directly on instance of the ConfigProvider.
+- `getConfig()` method of ConfigProvider. Call the property instead directly on an instance of the ConfigProvider.
 - `--fixtures-dir` option of the `run` command, `fixtures_dir` option of configuration file and `ConfigProvider->fixturesDir` variable. They were deprecated in version 2.1.0 and no longer used since.
 - `UniqueValue` component. (You may use Faker or some other library for similar use-case.)
 - `AbstractTestCaseBase` class. It should probably not affect anything, as it was only used internally.
+- Workarounds for legacy Firefox (version 47 and older) which are no longer needed.
+- PhantomJS support.
 
 ## 2.3.2 - 2017-12-02
 ### Changed

diff --git a/composer.json b/composer.json
@@ -29,29 +29,29 @@
         "ext-zip": "*",
         "beberlei/assert": "^3.0",
         "clue/graph": "~0.9.0",
-        "doctrine/inflector": "~1.0",
+        "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",
-        "phpunit/phpunit": "^7.5",
+        "phpunit/phpunit": "^7.5.20",
         "symfony/console": "^4.0",
         "symfony/event-dispatcher": "^4.0",
         "symfony/filesystem": "^4.0",
         "symfony/finder": "^4.0",
         "symfony/options-resolver": "^4.0",
         "symfony/polyfill-mbstring": "^1.12",
-        "symfony/process": "^4.0 !=4.0.2",
+        "symfony/process": "^4.0.3",
         "symfony/stopwatch": "^4.0",
         "symfony/yaml": "^4.0"
     },
     "require-dev": {
-        "ergebnis/composer-normalize": "^2.2",
+        "ergebnis/composer-normalize": "^2.11",
         "lmc/coding-standard": "^2.1",
-        "php-coveralls/php-coveralls": "^2.0",
-        "php-mock/php-mock-phpunit": "^2.1.2",
-        "php-parallel-lint/php-parallel-lint": "^1.0.0",
+        "php-coveralls/php-coveralls": "^2.4.2",
+        "php-mock/php-mock-phpunit": "^2.6.0",
+        "php-parallel-lint/php-parallel-lint": "^1.2.0",
         "phpstan/extension-installer": "^1.0",
         "phpstan/phpstan": "^0.12.11",
         "phpstan/phpstan-phpunit": "^0.12.6",

diff --git a/phpstan.neon b/phpstan.neon
@@ -6,8 +6,6 @@ parameters:
         - "#Property Lmc\\\\Steward\\\\Test\\\\AbstractTestCase::\\$wd \\(Facebook\\\\WebDriver\\\\Remote\\\\RemoteWebDriver\\) does not accept Lmc\\\\Steward\\\\WebDriver\\\\NullWebDriver#"
         - message: '#should return .+ but return statement is missing#'
           path: 'src/WebDriver/NullWebDriver.php'
-        - message: '#Variable \$e might not be defined.#'
-          path: 'src/Listener/WebDriverListener.php'
     excludes_analyse:
         - '%rootDir%/../../../src-tests/Process/Fixtures/InvalidTests/WrongClassTest.php'
         - '%rootDir%/../../../src-tests/Test/SyntaxSugarTraitTest.php'

diff --git a/src-tests/Console/Command/RunCommandTest.php b/src-tests/Console/Command/RunCommandTest.php
@@ -174,7 +174,6 @@ public function provideBrowserName(): array
         return [
             'firefox is supported' => ['firefox', false, 'firefox'],
             'chrome is supported' => ['chrome', false, 'chrome'],
-            'phantomjs is supported' => ['phantomjs', false, 'phantomjs'],
             'MicrosoftEdge is supported' => ['MicrosoftEdge', false, 'MicrosoftEdge'],
             'MicrosoftEdge is supported in lowercase' => ['microsoftedge', false, 'MicrosoftEdge'],
             'browser name is case insensitive' => ['FIREFOX', false, 'firefox'],
@@ -249,7 +248,7 @@ public function testShouldStopIfServerIsRespondingButIsNotSelenium(): void
 
         $this->assertContains('Unexpected response from Selenium server (This is teapot)', $this->tester->getDisplay());
         $this->assertContains(
-            'Looks like url "http://foo.bar:1337" is occupied by something else than Selenium server.',
+            'URL "http://foo.bar:1337" is occupied by something else than Selenium server.',
             $this->tester->getDisplay()
         );
         $this->assertSame(1, $this->tester->getStatusCode());

diff --git a/src-tests/Process/ProcessSetCreatorTest.php b/src-tests/Process/ProcessSetCreatorTest.php
@@ -101,7 +101,6 @@ public function testShouldCreateProcessSetFromGivenFiles(): void
         $processes = $processSet->get(ProcessWrapper::PROCESS_STATUS_QUEUED);
         $dummyTestProcess = $processes[self::NAME_DUMMY_TEST]->getProcess();
         $testCommand = $dummyTestProcess->getCommandLine();
-        $testEnv = $dummyTestProcess->getEnv();
 
         $this->assertContains('phpunit', $testCommand);
         $this->assertContains(
@@ -112,15 +111,13 @@ public function testShouldCreateProcessSetFromGivenFiles(): void
         $this->assertNotContains('--filter', $testCommand);
         $this->assertStringMatchesFormat('%A--configuration=%A%esrc%ephpunit.xml%A', $testCommand);
 
-        // Check defaults were passed to the Processes
+        // Check defaults were passed to the Process environment
+        $testEnv = $dummyTestProcess->getEnv();
         $definition = $this->command->getDefinition();
-        $expectedEnv = [
-            'BROWSER_NAME' => 'firefox',
-            'ENV' => 'staging',
-            'SERVER_URL' => $definition->getOption(RunCommand::OPTION_SERVER_URL)->getDefault(),
-            'LOGS_DIR' => '/foo/bar/logs',
-        ];
-        $this->assertArraySubset($expectedEnv, $testEnv);
+        $this->assertSame('firefox', $testEnv['BROWSER_NAME']);
+        $this->assertSame('staging', $testEnv['ENV']);
+        $this->assertSame($definition->getOption(RunCommand::OPTION_SERVER_URL)->getDefault(), $testEnv['SERVER_URL']);
+        $this->assertSame('/foo/bar/logs', $testEnv['LOGS_DIR']);
     }
 
     public function testShouldThrowExceptionIfAddingFileWithNoClass(): void
@@ -295,18 +292,15 @@ public function testShouldPropagateCustomOptionsIntoProcess(): void
         $process = $processSet->get(ProcessWrapper::PROCESS_STATUS_QUEUED)[self::NAME_DUMMY_TEST]->getProcess();
         $processEnv = $process->getEnv();
 
-        $this->assertArraySubset(
-            [
-                'BROWSER_NAME' => 'chrome',
-                'ENV' => 'staging',
-                'SERVER_URL' => 'http://foo.bar:1337',
-                'LOGS_DIR' => realpath(__DIR__ . '/Fixtures/custom-logs-dir/'),
-                'CAPABILITY' => '{"webdriver.log.file":"\/foo\/bar.log","whitespaced":"OS X 10.8",'
-                    . '"webdriver.foo":false,"version":"14.14393"}',
-                'CAPABILITIES_RESOLVER' => '',
-            ],
-            $processEnv
+        $this->assertSame('chrome', $processEnv['BROWSER_NAME']);
+        $this->assertSame('staging', $processEnv['ENV']);
+        $this->assertSame('http://foo.bar:1337', $processEnv['SERVER_URL']);
+        $this->assertSame(realpath(__DIR__ . '/Fixtures/custom-logs-dir/'), $processEnv['LOGS_DIR']);
+        $this->assertSame(
+            '{"webdriver.log.file":"\/foo\/bar.log","whitespaced":"OS X 10.8","webdriver.foo":false,"version":"14.14393"}',
+            $processEnv['CAPABILITY']
         );
+        $this->assertSame('', $processEnv['CAPABILITIES_RESOLVER']);
     }
 
     public function testShouldSetPHPUnitColoredOptionOnlyIfTheOutputIsDecorated(): void

diff --git a/src-tests/Selenium/CapabilitiesResolverTest.php b/src-tests/Selenium/CapabilitiesResolverTest.php
@@ -33,7 +33,6 @@ public function testShouldResolveBasicDesiredCapabilities(string $browser, calla
         $resolver->setCiDetector($this->createConfiguredMock(CiDetector::class, ['isCiDetected' => false]));
 
         $desiredCapabilities = $resolver->resolveDesiredCapabilities($test);
-        $this->assertInstanceOf(DesiredCapabilities::class, $desiredCapabilities);
 
         $desiredCapabilitiesArray = $desiredCapabilities->toArray();
         $this->assertSame($browser, $desiredCapabilitiesArray['browserName']);
@@ -51,12 +50,7 @@ public function testShouldResolveBasicDesiredCapabilities(string $browser, calla
     public function provideBrowsers(): array
     {
         return [
-            [
-                WebDriverBrowserType::FIREFOX,
-                function ($capabilitiesArray): void {
-                    $this->assertNotEmpty($capabilitiesArray['firefox_profile']);
-                },
-            ],
+            [WebDriverBrowserType::FIREFOX],
             [WebDriverBrowserType::CHROME],
             [WebDriverBrowserType::MICROSOFT_EDGE],
             [
@@ -66,7 +60,6 @@ function ($capabilitiesArray): void {
                 },
             ],
             [WebDriverBrowserType::SAFARI],
-            [WebDriverBrowserType::PHANTOMJS],
         ];
     }
 

diff --git a/src-tests/Test/AbstractTestCaseTest.php b/src-tests/Test/AbstractTestCaseTest.php
@@ -47,13 +47,7 @@ public function testShouldSetDefaultWindowSizeInitUtils(): void
 
         $wdWindowMock->expects($this->once())
             ->method('setSize')
-            ->with(
-                $this->logicalAnd(
-                    $this->isInstanceOf(WebDriverDimension::class),
-                    $this->attributeEqualTo('width', 1280),
-                    $this->attributeEqualTo('height', 1024)
-                )
-            );
+            ->with($this->equalTo(new WebDriverDimension(1280, 1024)));
 
         $this->testCase->wd = $wdMock;
 

diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php
@@ -2,7 +2,8 @@
 
 namespace Lmc\Steward;
 
-use Doctrine\Common\Inflector\Inflector;
+use Doctrine\Inflector\InflectorFactory;
+use Doctrine\Inflector\Language;
 use FlorianWolters\Component\Util\Singleton\SingletonTrait;
 
 /**
@@ -117,6 +118,7 @@ private function assembleConfigurationOptions(): array
     private function retrieveConfigurationValues(array $options): array
     {
         $outputValues = [];
+        $inflector = InflectorFactory::createForLanguage(Language::ENGLISH)->build();
 
         foreach ($options as $option) {
             $value = getenv($option);
@@ -126,7 +128,7 @@ private function retrieveConfigurationValues(array $options): array
                 throw new \RuntimeException(sprintf('%s environment variable must be defined', $option));
             }
 
-            $outputValues[Inflector::camelize(mb_strtolower($option))] = $value;
+            $outputValues[$inflector->camelize(mb_strtolower($option))] = $value;
         }
 
         return $outputValues;

diff --git a/src/Console/Command/RunCommand.php b/src/Console/Command/RunCommand.php
@@ -36,7 +36,6 @@ class RunCommand extends Command
         'microsoftedge' => WebDriverBrowserType::MICROSOFT_EDGE,
         'internet explorer' => WebDriverBrowserType::IE,
         'safari' => WebDriverBrowserType::SAFARI,
-        'phantomjs' => WebDriverBrowserType::PHANTOMJS,
     ];
 
     public const ARGUMENT_ENVIRONMENT = 'environment';
@@ -159,7 +158,9 @@ protected function configure(): void
             );
 
         $this->addUsage('staging firefox');
-        $this->addUsage('--group=foo --group=bar --exclude-group=baz -vvv development phantomjs');
+        $this->addUsage(
+            '--group=foo --group=bar --exclude-group=baz --server-url=http://localhost:4444/wd/hub -vv staging chrome'
+        );
 
         $this->getDispatcher()->dispatch(CommandEvents::CONFIGURE, new BasicConsoleEvent($this));
     }
@@ -364,9 +365,9 @@ protected function testSeleniumConnection(): bool
             );
             $this->io->error(
                 sprintf(
-                    'Looks like url "%s" is occupied by something else than Selenium server. '
-                    . 'Make sure Selenium server is really accessible on this url '
-                    . 'or change it using --server-url option',
+                    'URL "%s" is occupied by something else than Selenium server. Make sure Selenium server is really'
+                    . ' accessible on this URL or change it using --server-url option.'
+                    . ' If using Selenium 3.x, also make sure you are not be missing "/wd/hub" part in the server URL.',
                     $seleniumAdapter->getServerUrl()
                 )
             );

diff --git a/src/Listener/WebDriverListener.php b/src/Listener/WebDriverListener.php
@@ -2,10 +2,9 @@
 
 namespace Lmc\Steward\Listener;
 
-use Facebook\WebDriver\Exception\UnknownServerException;
+use Facebook\WebDriver\Exception\SessionNotCreatedException;
 use Facebook\WebDriver\Exception\WebDriverException;
 use Facebook\WebDriver\Remote\DesiredCapabilities;
-use Facebook\WebDriver\Remote\WebDriverBrowserType;
 use Lmc\Steward\ConfigProvider;
 use Lmc\Steward\Selenium\CapabilitiesResolver;
 use Lmc\Steward\Test\AbstractTestCase;
@@ -122,12 +121,6 @@ public function endTest(Test $test, float $time): void
             ob_start(); // Capture any output from commands bellow to make them appended to output of the test.
 
             try {
-                // Workaround for PhantomJS 1.x - see https://github.com/detro/ghostdriver/issues/343
-                // Should be removed with PhantomJS 2
-                if (ConfigProvider::getInstance()->browserName === WebDriverBrowserType::PHANTOMJS) {
-                    $test->wd->execute('deleteAllCookies');
-                }
-
                 $test->wd->close();
                 $test->wd->quit();
             } catch (WebDriverException $e) {
@@ -140,10 +133,8 @@ public function endTest(Test $test, float $time): void
     }
 
     /**
-     * Subroutine to encapsulate creation of real WebDriver. Handles some exceptions that may occur etc.
+     * Subroutine to encapsulate creation of real WebDriver.
      * The WebDriver instance is stored to $test->wd when created.
-     *
-     * @throws UnknownServerException
      */
     protected function createWebDriver(
         AbstractTestCase $test,
@@ -153,66 +144,21 @@ protected function createWebDriver(
         int $connectTimeoutMs,
         int $requestTimeoutMs
     ): void {
-        $browserName = ConfigProvider::getInstance()->browserName;
+        try {
+            $test->wd = RemoteWebDriver::create(
+                $remoteServerUrl,
+                $desiredCapabilities,
+                $connectTimeoutMs,
+                $requestTimeoutMs,
+                null,
+                null,
+                $requiredCapabilities
+            );
 
-        for ($startAttempts = 0; $startAttempts < 4; $startAttempts++) {
-            try {
-                $test->wd = RemoteWebDriver::create(
-                    $remoteServerUrl,
-                    $desiredCapabilities,
-                    $connectTimeoutMs,
-                    $requestTimeoutMs,
-                    null,
-                    null,
-                    $requiredCapabilities
-                );
-
-                return;
-            } catch (UnknownServerException $e) {
-                if ($this->cannotBindToFirefoxLockingPort($browserName, $e->getMessage())) {
-                    // As a consequence of Selenium issue #5172 (cannot change locking port), Firefox may on CI server
-                    // collide with other FF instance. As a workaround, we try to start it again after a short delay.
-                    $test->warn(
-                        'Firefox locking port is occupied; beginning attempt #%d to start it ("%s")',
-                        $startAttempts + 2,
-                        $e->getMessage()
-                    );
-                    sleep(1);
-                    continue;
-                }
-
-                if (mb_strpos($e->getMessage(), 'Error forwarding the new session') !== false) {
-                    $test->warn('Cannot execute test on the node. Maybe you started just the hub and not the node?');
-                }
-
-                throw $e;
-            } catch (WebDriverException $e) {
-                if ($this->mayBeMissingFullServerUrl($remoteServerUrl, $e->getMessage())) {
-                    $test->warn('Unable to initialize new WebDriver session.');
-                    $test->warn(
-                        'Server URL ("%s") appears to be missing "/wd/hub" part, so make sure you are ussing full URL'
-                        . ' of the server (not just a hostname and port).',
-                        $remoteServerUrl
-                    );
-                }
-
-                throw $e;
-            }
+            return;
+        } catch (SessionNotCreatedException $e) {
+            $test->warn('Unable to initialize new WebDriver session.');
+            throw $e;
         }
-
-        $test->warn('All %d attempts to instantiate Firefox WebDriver failed', $startAttempts + 1);
-        throw $e;
-    }
-
-    private function cannotBindToFirefoxLockingPort(string $browserName, string $exceptionMessage): bool
-    {
-        return $browserName === 'firefox'
-            && mb_strpos($exceptionMessage, 'Unable to bind to locking port') !== false;
-    }
-
-    private function mayBeMissingFullServerUrl(string $remoteServerUrl, string $exceptionMessage): bool
-    {
-        return mb_strpos($exceptionMessage, 'JSON decoding of remote response failed.') !== false
-            && mb_strpos($remoteServerUrl, 'wd/hub') === false;
     }
 }