From 5d78e4c2c0791cc459b6a44c94bbc0342fe7910b Mon Sep 17 00:00:00 2001 From: micbou Date: Sat, 16 Sep 2017 23:22:56 +0200 Subject: [PATCH] Improve path to python interpreter error handling --- python/ycm/paths.py | 11 ++--- python/ycm/tests/__init__.py | 4 +- python/ycm/tests/test_utils.py | 7 ++- python/ycm/tests/youcompleteme_test.py | 60 +++++++++++++++++++++++++- python/ycm/youcompleteme.py | 32 +++++++++----- 5 files changed, 94 insertions(+), 20 deletions(-) diff --git a/python/ycm/paths.py b/python/ycm/paths.py index 524a23f4..1b9009f6 100644 --- a/python/ycm/paths.py +++ b/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(): diff --git a/python/ycm/tests/__init__.py b/python/ycm/tests/__init__.py index 8c2943c2..bc5090bb 100644 --- a/python/ycm/tests/__init__.py +++ b/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: diff --git a/python/ycm/tests/test_utils.py b/python/ycm/tests/test_utils.py index 468b48ca..8aa65ba5 100644 --- a/python/ycm/tests/test_utils.py +++ b/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_' diff --git a/python/ycm/tests/youcompleteme_test.py b/python/ycm/tests/youcompleteme_test.py index 45bfed22..566d71ba 100644 --- a/python/ycm/tests/youcompleteme_test.py +++ b/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( diff --git a/python/ycm/youcompleteme.py b/python/ycm/youcompleteme.py index 5173f6fe..7fa834ec 100644 --- a/python/ycm/youcompleteme.py +++ b/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'