Improve path to python interpreter error handling

python/ycm/paths.py

@@ -46,14 +46,15 @@ def PathToPythonInterpreter():
 
   python_interpreter = vim.eval( 'g:ycm_server_python_interpreter' )
   if python_interpreter:
-    if _EndsWithPython( python_interpreter ):
+    python_interpreter = utils.FindExecutable( python_interpreter )
+    if python_interpreter:
       return python_interpreter
 
     raise RuntimeError( "Path in 'g:ycm_server_python_interpreter' option "
                         "does not point to a valid Python 2.6+ or 3.3+." )
 
   python_interpreter = _PathToPythonUsedDuringBuild()
-  if _EndsWithPython( python_interpreter ):
+  if python_interpreter and utils.GetExecutable( python_interpreter ):
     return python_interpreter
 
   # On UNIX platforms, we use sys.executable as the Python interpreter path.
@@ -76,9 +77,9 @@ def PathToPythonInterpreter():
   if python_interpreter:
     return python_interpreter
 
-  raise RuntimeError( "Cannot find Python 2.6+ or 3.3+. You can set its path "
-                      "using the 'g:ycm_server_python_interpreter' "
-                      "option." )
+  raise RuntimeError( "Cannot find Python 2.6+ or 3.3+. "
+                      "Set the 'g:ycm_server_python_interpreter' option "
+                      "to a Python interpreter path." )
 
 
 def _PathToPythonUsedDuringBuild():

python/ycm/tests/__init__.py

@@ -55,7 +55,7 @@ def PathToTestFile( *args ):
   return os.path.join( dir_of_current_script, 'testdata', *args )
 
 
-def _MakeUserOptions( custom_options = {} ):
+def MakeUserOptions( custom_options = {} ):
   options = dict( user_options_store.DefaultOptions() )
   options.update( DEFAULT_CLIENT_OPTIONS )
   options.update( custom_options )
@@ -122,7 +122,7 @@ def YouCompleteMeInstance( custom_options = {} ):
   def Decorator( test ):
     @functools.wraps( test )
     def Wrapper( *args, **kwargs ):
-      ycm = YouCompleteMe( _MakeUserOptions( custom_options ) )
+      ycm = YouCompleteMe( MakeUserOptions( custom_options ) )
       WaitUntilReady()
       ycm.CheckIfServerIsReady()
       try:

python/ycm/tests/test_utils.py

@@ -181,12 +181,17 @@ def _MockVimMatchEval( value ):
   return None
 
 
+# This variable exists to easily mock the 'g:ycm_server_python_interpreter'
+# option in tests.
+server_python_interpreter = ''
+
+
 def _MockVimEval( value ):
   if value == 'g:ycm_min_num_of_chars_for_completion':
     return 0
 
   if value == 'g:ycm_server_python_interpreter':
-    return ''
+    return server_python_interpreter
 
   if value == 'tempname()':
     return '_TEMP_FILE_'

python/ycm/tests/youcompleteme_test.py

@@ -32,7 +32,10 @@ from hamcrest import ( assert_that, contains, empty, equal_to, is_in, is_not,
                        matches_regexp )
 from mock import call, MagicMock, patch
 
-from ycm.tests import StopServer, test_utils, YouCompleteMeInstance
+from ycm.paths import _PathToPythonUsedDuringBuild
+from ycm.youcompleteme import YouCompleteMe
+from ycm.tests import ( MakeUserOptions, StopServer, test_utils,
+                        WaitUntilReady, YouCompleteMeInstance )
 from ycm.client.base_request import _LoadExtraConfFile
 from ycmd.responses import ServerError
 
@@ -42,6 +45,60 @@ def YouCompleteMe_YcmCoreNotImported_test( ycm ):
   assert_that( 'ycm_core', is_not( is_in( sys.modules ) ) )
 
 
+@patch( 'ycm.vimsupport.PostVimMessage' )
+def YouCompleteMe_InvalidPythonInterpreterPath_test( post_vim_message ):
+  try:
+    with patch( 'ycm.tests.test_utils.server_python_interpreter',
+                '/invalid/path/to/python' ):
+      ycm = YouCompleteMe( MakeUserOptions() )
+      assert_that( ycm.IsServerAlive(), equal_to( False ) )
+      post_vim_message.assert_called_once_with(
+        "Unable to start the ycmd server. "
+        "Path in 'g:ycm_server_python_interpreter' option does not point "
+        "to a valid Python 2.6+ or 3.3+. "
+        "Correct the error then restart the server with ':YcmRestartServer'." )
+
+    post_vim_message.reset_mock()
+
+    with patch( 'ycm.tests.test_utils.server_python_interpreter',
+                _PathToPythonUsedDuringBuild() ):
+      ycm.RestartServer()
+
+    assert_that( ycm.IsServerAlive(), equal_to( True ) )
+    post_vim_message.assert_called_once_with( 'Restarting ycmd server...' )
+  finally:
+    WaitUntilReady()
+    StopServer( ycm )
+
+
+@patch( 'ycmd.utils.PathToFirstExistingExecutable', return_value = None )
+@patch( 'ycm.paths._EndsWithPython', return_value = False )
+@patch( 'ycm.vimsupport.PostVimMessage' )
+def YouCompleteMe_NoPythonInterpreterFound_test( post_vim_message, *args ):
+  try:
+    with patch( 'ycmd.utils.ReadFile', side_effect = IOError ):
+
+      ycm = YouCompleteMe( MakeUserOptions() )
+      assert_that( ycm.IsServerAlive(), equal_to( False ) )
+      post_vim_message.assert_called_once_with(
+        "Unable to start the ycmd server. Cannot find Python 2.6+ or 3.3+. "
+        "Set the 'g:ycm_server_python_interpreter' option to a Python "
+        "interpreter path. "
+        "Correct the error then restart the server with ':YcmRestartServer'." )
+
+    post_vim_message.reset_mock()
+
+    with patch( 'ycm.tests.test_utils.server_python_interpreter',
+                _PathToPythonUsedDuringBuild() ):
+      ycm.RestartServer()
+
+    assert_that( ycm.IsServerAlive(), equal_to( True ) )
+    post_vim_message.assert_called_once_with( 'Restarting ycmd server...' )
+  finally:
+    WaitUntilReady()
+    StopServer( ycm )
+
+
 @YouCompleteMeInstance()
 @patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
 def RunNotifyUserIfServerCrashed( ycm, test, post_vim_message ):
@@ -271,7 +328,6 @@ def YouCompleteMe_GetDefinedSubcommands_ErrorFromServer_test( ycm,
   assert_that( result, empty() )
 
 
-
 @YouCompleteMeInstance()
 @patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
 def YouCompleteMe_ShowDetailedDiagnostic_MessageFromServer_test(

python/ycm/youcompleteme.py

@@ -150,7 +150,21 @@ class YouCompleteMe( object ):
       json.dump( options_dict, options_file )
       options_file.flush()
 
-      args = [ paths.PathToPythonInterpreter(),
+      BaseRequest.server_location = 'http://127.0.0.1:' + str( server_port )
+      BaseRequest.hmac_secret = hmac_secret
+
+      try:
+        python_interpreter = paths.PathToPythonInterpreter()
+      except RuntimeError as error:
+        error_message = (
+          "Unable to start the ycmd server. {0}. "
+          "Correct the error then restart the server "
+          "with ':YcmRestartServer'.".format( str( error ).rstrip( '.' ) ) )
+        self._logger.exception( error_message )
+        vimsupport.PostVimMessage( error_message )
+        return
+
+      args = [ python_interpreter,
                paths.PathToServerScript(),
                '--port={0}'.format( server_port ),
                '--options_file={0}'.format( options_file.name ),
@@ -170,8 +184,6 @@ class YouCompleteMe( object ):
 
       self._server_popen = utils.SafePopen( args, stdin_windows = PIPE,
                                             stdout = PIPE, stderr = PIPE )
-      BaseRequest.server_location = 'http://127.0.0.1:' + str( server_port )
-      BaseRequest.hmac_secret = hmac_secret
 
     self._NotifyUserIfServerCrashed()
 
@@ -215,9 +227,8 @@ class YouCompleteMe( object ):
 
 
   def IsServerAlive( self ):
-    return_code = self._server_popen.poll()
     # When the process hasn't finished yet, poll() returns None.
-    return return_code is None
+    return bool( self._server_popen ) and self._server_popen.poll() is None
 
 
   def CheckIfServerIsReady( self ):
@@ -233,7 +244,8 @@ class YouCompleteMe( object ):
 
 
   def _NotifyUserIfServerCrashed( self ):
-    if self._user_notified_about_crash or self.IsServerAlive():
+    if ( not self._server_popen or self._user_notified_about_crash or
+         self.IsServerAlive() ):
       return
     self._user_notified_about_crash = True
 
@@ -589,10 +601,10 @@ class YouCompleteMe( object ):
     extra_data = {}
     self._AddExtraConfDataIfNeeded( extra_data )
     debug_info += FormatDebugInfoResponse( SendDebugInfoRequest( extra_data ) )
-    debug_info += (
-      'Server running at: {0}\n'
-      'Server process ID: {1}\n'.format( BaseRequest.server_location,
-                                         self._server_popen.pid ) )
+    debug_info += 'Server running at: {0}\n'.format(
+      BaseRequest.server_location )
+    if self._server_popen:
+      debug_info += 'Server process ID: {0}\n'.format( self._server_popen.pid )
     if self._server_stdout and self._server_stderr:
       debug_info += ( 'Server logfiles:\n'
                       '  {0}\n'