Fix oppia#9945: Fixed automatic chrome driver version retrieval for M…

…ac OS. (oppia#9947)

* fixed get_chrome_driver_version() to work for mac users.

* Changed wording back.

* Added backend tests

* Changed the branching.

* Added comments to make the \ and lack of \ more clear.

* Removed command_that_errors variable.

* Added r for \ string to avoid warnings.

* Moved the comment down so that it is easier to find.

scripts/run_e2e_tests.py

@@ -434,18 +434,35 @@ def get_chrome_driver_version():
     This method follows the steps mentioned here:
     https://chromedriver.chromium.org/downloads/version-selection
     """
+    popen_args = ['google-chrome', '--version']
+    if common.is_mac_os():
+        # There are spaces between Google and Chrome in the path. Spaces don't
+        # need to be escaped when we're not using the terminal, ie. shell=False
+        # for Popen by default.
+        popen_args = [
+            '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome',
+            '--version'
+        ]
     try:
-        proc = subprocess.Popen(
-            ['google-chrome', '--version'], stdout=subprocess.PIPE)
+        proc = subprocess.Popen(popen_args, stdout=subprocess.PIPE)
         output = proc.stdout.readline()
     except OSError:
+        # For the error message for the mac command, we need to add the
+        # backslashes in. This is because it is likely that a user will try to
+        # run the command on their terminal and, as mentioned above, the mac
+        # get chrome version command has spaces in the path which need to be
+        # escaped for successful terminal use.
         raise Exception(
-            'Failed to execute "google-chrome --version" command. This is '
-            'used to determine the chromedriver version to use. Please set '
-            'the chromedriver version manually using --chrome_driver_version '
-            'flag. To determine the chromedriver version to be used, please '
-            'follow the instructions mentioned in the following URL:\n'
-            'https://chromedriver.chromium.org/downloads/version-selection')
+            'Failed to execute "%s" command. '
+            'This is used to determine the chromedriver version to use. '
+            'Please set the chromedriver version manually using '
+            '--chrome_driver_version flag. To determine the chromedriver '
+            'version to be used, please follow the instructions mentioned '
+            'in the following URL:\n'
+            'https://chromedriver.chromium.org/downloads/version-selection' % (
+                ' '.join(arg.replace(' ', r'\ ') for arg in popen_args)
+            )
+        )
     chrome_version = ''.join(re.findall(r'([0-9]|\.)', output))
     chrome_version = '.'.join(chrome_version.split('.')[:-1])
     response = python_utils.url_open(

scripts/run_e2e_tests_test.py

@@ -1019,7 +1019,8 @@ def mock_get_chrome_driver_version():
                                 run_e2e_tests.main(
                                     args=['--skip-install', '--skip-build'])
 
-    def test_chrome_not_found_failure(self):
+    def test_linux_chrome_version_command_not_found_failure(self):
+        os_name_swap = self.swap(common, 'OS_NAME', 'Linux')
 
         def mock_popen(unused_commands, stdout):
             self.assertEqual(stdout, -1)
@@ -1036,7 +1037,35 @@ def mock_popen(unused_commands, stdout):
             'follow the instructions mentioned in the following URL:\n'
             'https://chromedriver.chromium.org/downloads/version-selection')
 
-        with popen_swap, self.assertRaisesRegexp(Exception, expected_message):
+        with os_name_swap, popen_swap, self.assertRaisesRegexp(
+            Exception, expected_message):
+            run_e2e_tests.get_chrome_driver_version()
+
+    def test_mac_chrome_version_command_not_found_failure(self):
+        os_name_swap = self.swap(common, 'OS_NAME', 'Darwin')
+
+        def mock_popen(unused_commands, stdout):
+            self.assertEqual(stdout, -1)
+            raise OSError(
+                r'/Applications/Google\ Chrome.app/Contents/MacOS/Google\ '
+                'Chrome not found')
+
+        popen_swap = self.swap_with_checks(
+            subprocess, 'Popen', mock_popen, expected_args=[([
+                '/Applications/Google Chrome.app/Contents/MacOS/Google '
+                'Chrome', '--version'],)])
+        expected_message = (
+            r'Failed to execute "/Applications/Google\\ '
+            r'Chrome.app/Contents/MacOS/Google\\ Chrome --version" command. '
+            'This is used to determine the chromedriver version to use. '
+            'Please set the chromedriver version manually using '
+            '--chrome_driver_version flag. To determine the chromedriver '
+            'version to be used, please follow the instructions mentioned '
+            'in the following URL:\n'
+            'https://chromedriver.chromium.org/downloads/version-selection')
+
+        with os_name_swap, popen_swap, self.assertRaisesRegexp(
+            Exception, expected_message):
             run_e2e_tests.get_chrome_driver_version()
 
     def test_start_tests_in_debug_mode(self):