Merge pull request #15 from OndraM/feature/fail-on-unknown-browser

Throw an exception if given browser is not supported
OndraM · May 25, 2015 · fafab93 · fafab93
2 parents 364aa1e + b364e7f
commit fafab93
Show file tree

Hide file tree

Showing 6 changed files with 139 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,9 +3,12 @@
 <!-- There is always Unreleased section on the top. Subsections (Added, Changed, Fixed, Removed) should be added as needed. -->
 
 ## Unreleased
+### Added
+- Check if browser given to `run-tests` command is supported (this helps avoiding typos, interchange of browser and environment etc.).
+
 ### Changed
 - The `logs/results.xml` could now be also accessed locally (the XSLT is now embedded right into the file, so we don't encounter same-origin policy problems).
-- Use tagged version 0.6.0 of [php-webdriver](https://github.com/facebook/php-webdriver)
+- Use tagged version 0.6.0 of [php-webdriver](https://github.com/facebook/php-webdriver).
 
 ### Fixed
 - Properly trigger PHPUnit colored (ANSI) mode when Steward itself is in ANSI mode.

diff --git a/src-tests/Console/Command/RunTestsCommandTest.php b/src-tests/Console/Command/RunTestsCommandTest.php
@@ -9,6 +9,7 @@
 use Lmc\Steward\Process\ProcessSetCreator;
 use Lmc\Steward\Selenium\SeleniumServerAdapter;
 use Symfony\Component\Console\Application;
+use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\Finder\Finder;
@@ -69,13 +70,20 @@ public function testShouldFailWithoutEnvironmentSpecified()
     /**
      * @dataProvider directoryOptionsProvider
      * @param string $directoryOption Passed path type option
-     * @param string $errorBeginning Beginning of error message
+     * @param string $errorBeginning Beginning of exception message
      */
-    public function testShouldStopIfAnyRequiredDirectoryIsNotAccessible($directoryOption, $errorBeginning)
+    public function testShouldThrowExceptionIfAnyRequiredDirectoryIsNotAccessible($directoryOption, $errorBeginning)
     {
         $seleniumAdapterMock = $this->getSeleniumAdapterMock();
         $this->command->setSeleniumAdapter($seleniumAdapterMock);
 
+        $expectedError = sprintf(
+            '%s, make sure it is accessible or define your own path using --%s option',
+            $errorBeginning,
+            $directoryOption
+        );
+        $this->setExpectedException('\RuntimeException', $expectedError);
+
         $this->tester->execute(
             [
                 'command' => $this->command->getName(),
@@ -84,15 +92,6 @@ public function testShouldStopIfAnyRequiredDirectoryIsNotAccessible($directoryOp
                 '--' . $directoryOption => '/not/accessible'
             ]
         );
-
-        $expectedError = sprintf(
-            '%s, make sure it is accessible or define your own path using --%s option',
-            $errorBeginning,
-            $directoryOption
-        );
-
-        $this->assertContains($expectedError, $this->tester->getDisplay());
-        $this->assertSame(1, $this->tester->getStatusCode());
     }
 
     /**
@@ -107,6 +106,70 @@ public function directoryOptionsProvider()
         ];
     }
 
+    public function testShouldOutputAssembledPathsToDirectoriesInDebugMode()
+    {
+        $seleniumAdapterMock = $this->getSeleniumAdapterMock();
+        $this->command->setSeleniumAdapter($seleniumAdapterMock);
+
+        $this->tester->execute(
+            [
+                'command' => $this->command->getName(),
+                'environment' => 'staging',
+                'browser' => 'firefox',
+                '--tests-dir' => __DIR__ . '/Fixtures/DummyTests',
+                '--pattern' => 'NotExisting.foo' // so the test stops execution
+            ],
+            ['verbosity' => OutputInterface::VERBOSITY_DEBUG]
+        );
+
+        $output = $this->tester->getDisplay();
+        $this->assertContains('Base path to fixtures results: ' . realpath(__DIR__) . '/Fixtures/tests', $output);
+        $this->assertContains('Path to logs: ' . realpath(__DIR__) . '/Fixtures/logs', $output);
+        $this->assertContains(' - in directory "' . realpath(__DIR__) . '/Fixtures/DummyTests"', $output);
+        $this->assertSame(1, $this->tester->getStatusCode());
+    }
+
+    /**
+     * @dataProvider browserNameProvider
+     * @param string $browserName
+     * @param bool $shouldThrowException
+     */
+    public function testShouldThrowExceptionIfUnsupportedBrowserSelected($browserName, $shouldThrowException)
+    {
+        if ($shouldThrowException) {
+            $this->setExpectedException('\RuntimeException', 'Browser "' . $browserName . '" is not supported');
+        }
+
+        $seleniumAdapterMock = $this->getSeleniumAdapterMock();
+        $this->command->setSeleniumAdapter($seleniumAdapterMock);
+
+        $this->tester->execute(
+            ['command' => $this->command->getName(), 'environment' => 'prod', 'browser' => $browserName]
+        );
+
+        if (!$shouldThrowException) {
+            $output = $this->tester->getDisplay();
+            $this->assertContains('Browser: ' . strtolower($browserName), $output);
+            $this->assertContains('No testcases found, exiting.', $output);
+        }
+    }
+
+    /**
+     * @return array
+     */
+    public function browserNameProvider()
+    {
+        return [
+            // $browserName, $shouldThrowException
+            'firefox is supported' => ['firefox', false],
+            'chrome is supported' => ['chrome', false],
+            'phantomjs is supported' => ['phantomjs', false],
+            'browser name is case insensitive' => ['FIREFOX', false],
+            'not supported browser' => ['mosaic', true],
+            'unprintable character in browser name' => ['firefox', true],
+        ];
+    }
+
     public function testShouldStopIfServerIsNotResponding()
     {
         $seleniumAdapterMock = $this->getMockBuilder(SeleniumServerAdapter::class)

diff --git a/src/Console/Command/RunTestsCommand.php b/src/Console/Command/RunTestsCommand.php
@@ -28,6 +28,14 @@ class RunTestsCommand extends Command
     protected $seleniumAdapter;
     /** @var ProcessSetCreator */
     protected $processSetCreator;
+    /** @var array */
+    protected $supportedBrowsers = [
+        \WebDriverBrowserType::FIREFOX,
+        \WebDriverBrowserType::CHROME,
+        \WebDriverBrowserType::IE,
+        \WebDriverBrowserType::SAFARI,
+        \WebDriverBrowserType::PHANTOMJS,
+    ];
 
     const ARGUMENT_ENVIRONMENT = 'environment';
     const ARGUMENT_BROWSER = 'browser';
@@ -130,14 +138,15 @@ protected function configure()
     }
 
     /**
-     * Execute command
+     * Initialize, check arguments and options values etc.
      *
      * @param InputInterface $input
      * @param OutputInterface $output
-     * @return int|null|void
      */
-    protected function execute(InputInterface $input, OutputInterface $output)
+    protected function initialize(InputInterface $input, OutputInterface $output)
     {
+        parent::initialize($input, $output);
+
         $output->writeln(
             sprintf(
                 '<info>Steward</info> <comment>%s</comment> is running the tests...%s',
@@ -146,11 +155,33 @@ protected function execute(InputInterface $input, OutputInterface $output)
             )
         );
 
-        $output->writeln(sprintf('Browser: %s', $input->getArgument(self::ARGUMENT_BROWSER)));
+        // If browser name or env is empty, ends initialization and let the Console/Command fail on input validation
+        if (empty($input->getArgument(self::ARGUMENT_BROWSER))
+            || empty($input->getArgument(self::ARGUMENT_ENVIRONMENT))
+        ) {
+            return;
+        }
+
+        // Browser name is case insensitive, normalize it to lower case
+        $input->setArgument(self::ARGUMENT_BROWSER, strtolower($input->getArgument(self::ARGUMENT_BROWSER)));
+        $browser = $input->getArgument(self::ARGUMENT_BROWSER);
+
+        // Check if browser is supported
+        if (!in_array($browser, $this->supportedBrowsers)) {
+            throw new \RuntimeException(
+                sprintf(
+                    'Browser "%s" is not supported (use one of: %s)',
+                    $browser,
+                    implode(', ', $this->supportedBrowsers)
+                )
+            );
+        }
+
+        $output->writeln(sprintf('Browser: %s', $browser));
         $output->writeln(sprintf('Environment: %s', $input->getArgument(self::ARGUMENT_ENVIRONMENT)));
 
-        // Tests directories exists
-        $testDirectoriesResult = $this->testDirectories(
+        // Check if directories exists
+        $this->testDirectories(
             $input,
             $output,
             [
@@ -159,9 +190,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
                 $this->getDefinition()->getOption(self::OPTION_FIXTURES_DIR),
             ]
         );
-        if (!$testDirectoriesResult) {
-            return 1;
-        }
 
         if ($output->isDebug()) {
             $output->writeln(
@@ -174,7 +202,17 @@ protected function execute(InputInterface $input, OutputInterface $output)
                 sprintf('Publish results: %s', ($input->getOption(self::OPTION_PUBLISH_RESULTS)) ? 'yes' : 'no')
             );
         }
+    }
 
+    /**
+     * Execute command
+     *
+     * @param InputInterface $input
+     * @param OutputInterface $output
+     * @return int|null|void
+     */
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
         $this->getDispatcher()->dispatch(
             CommandEvents::RUN_TESTS_INIT,
             new ExtendedConsoleEvent($this, $input, $output)
@@ -412,7 +450,7 @@ protected function getProcessOutput(Process $process)
      * @param InputInterface $input
      * @param OutputInterface $output
      * @param InputOption[] $dirs Option defining directories
-     * @return bool
+     * @throws \RuntimeException Thrown when directory is not accessible
      */
     protected function testDirectories(InputInterface $input, OutputInterface $output, array $dirs)
     {
@@ -421,18 +459,15 @@ protected function testDirectories(InputInterface $input, OutputInterface $outpu
             $currentValue = $input->getOption($dir->getName());
 
             if ($currentValue === false || realpath($currentValue) === false) {
-                $output->writeln(sprintf(
-                    '<error>%s does not exist, make sure it is accessible or define your own path using %s'
-                    . ' option</error>',
-                    $dir->getDescription(),
-                    '--' . $dir->getName()
-                ));
-
-                return false;
+                throw new \RuntimeException(
+                    sprintf(
+                        '%s does not exist, make sure it is accessible or define your own path using %s option',
+                        $dir->getDescription(),
+                        '--' . $dir->getName()
+                    )
+                );
             }
         }
-
-        return true;
     }
 
     /**

diff --git a/src/Listener/WebdriverListener.php → src/Listener/WebDriverListener.php b/src/Listener/WebdriverListener.php → src/Listener/WebDriverListener.php
@@ -16,7 +16,7 @@
  * If taking screenshot using addFailure(), tearDown() would have already been called and the
  * browser would be closed.
  */
-class WebdriverListener extends \PHPUnit_Framework_BaseTestListener
+class WebDriverListener extends \PHPUnit_Framework_BaseTestListener
 {
     const NO_BROWSER_ANNOTATION = 'noBrowser';
 
@@ -32,7 +32,7 @@ public function startTest(\PHPUnit_Framework_Test $test)
 
         $config = ConfigProvider::getInstance();
 
-        // Initialize NullWebdriver if self::NO_BROWSER_ANNOTATION is used on testcase class or test method
+        // 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)));
 
@@ -41,7 +41,7 @@ public function startTest(\PHPUnit_Framework_Test $test)
         ) {
             $test->wd = new NullWebDriver();
             $test->log(
-                'Initializing Null webdriver for "%s::%s" (@%s annotation used %s)',
+                'Initializing Null WebDriver for "%s::%s" (@%s annotation used %s)',
                 get_class($test),
                 $test->getName(),
                 self::NO_BROWSER_ANNOTATION,
@@ -88,7 +88,7 @@ public function endTest(\PHPUnit_Framework_Test $test, $time)
 
         if ($test->wd instanceof \RemoteWebDriver) {
             $test->log(
-                'Destroying "%s" webdriver for "%s::%s" (session %s)',
+                'Destroying "%s" WebDriver for "%s::%s" (session %s)',
                 ConfigProvider::getInstance()->browserName,
                 get_class($test),
                 $test->getName(),

diff --git a/src/Process/ProcessSet.php b/src/Process/ProcessSet.php
@@ -39,7 +39,7 @@ class ProcessSet implements \Countable
     const PROCESS_RESULT_PASSED = 'passed';
     /** Process failed - some tests have failed or are broken */
     const PROCESS_RESULT_FAILED = 'failed';
-    /** Process fatally failed (PHP fatal error occurred - eg. no webdriver available) */
+    /** Process fatally failed (PHP fatal error occurred - eg. no WebDriver available) */
     const PROCESS_RESULT_FATAL = 'fatal';
 
     /** @var array List of possible process statuses */

diff --git a/src/phpunit.xml b/src/phpunit.xml
@@ -5,7 +5,7 @@
 >
     <listeners>
         <listener class="Lmc\Steward\Listener\SnapshotListener"/>
-        <listener class="Lmc\Steward\Listener\WebdriverListener"/>
+        <listener class="Lmc\Steward\Listener\WebDriverListener"/>
         <listener class="Lmc\Steward\Listener\TestStatusListener">
             <arguments>
                 <array/>