Auto merge of #295 - vheon:basic-any-python, r=puremourning

[READY] Make python binary specifiable by the user

I'm starting this PR as WIP because it lacks tests and I wanted to ask for suggestion about that. What do you think would be the best approach? I was thinking about configuring the `user_option` and check how the `utils.SafePopen` was called but in general I'm not a fan of this kind of tests, so I'm open to suggestion 😃

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/295)
<!-- Reviewable:end -->

ycmd/completers/python/jedi_completer.py

@@ -34,7 +34,8 @@
 
 
 HMAC_SECRET_LENGTH = 16
-PYTHON_EXECUTABLE_PATH = sys.executable
+BINARY_NOT_FOUND_MESSAGE = ( 'The specified python interpreter {0} ' +
+                             'was not found. Did you specify it correctly?' )
 LOG_FILENAME_FORMAT = os.path.join( utils.PathToTempDir(),
                                     u'jedihttp_{port}_{std}.log' )
 PATH_TO_JEDIHTTP = os.path.join( os.path.abspath( os.path.dirname( __file__ ) ),
@@ -71,9 +72,27 @@ def __init__( self, user_options ):
     self._logfile_stderr = None
     self._keep_logfiles = user_options[ 'server_keep_logfiles' ]
     self._hmac_secret = ''
+    self._python_binary_path = sys.executable
+
+    self._UpdatePythonBinary( user_options.get( 'python_binary_path' ) )
+
     self._StartServer()
 
 
+  def _UpdatePythonBinary( self, binary ):
+    if binary:
+      if not self._CheckBinaryExists( binary ):
+        msg = BINARY_NOT_FOUND_MESSAGE.format( binary )
+        self._logger.error( msg )
+        raise RuntimeError( msg )
+      self._python_binary_path = binary
+
+
+  def _CheckBinaryExists( self, binary ):
+    """This method is here to help testing"""
+    return os.path.isfile( binary )
+
+
   def SupportedFiletypes( self ):
     """ Just python """
     return [ 'python' ]
@@ -104,9 +123,11 @@ def ServerIsRunning( self ):
         return False
 
 
-  def RestartServer( self ):
+  def RestartServer( self, binary = None ):
     """ Restart the JediHTTP Server. """
     with self._server_lock:
+      if binary:
+        self._UpdatePythonBinary( binary )
       self._StopServer()
       self._StartServer()
 
@@ -131,7 +152,7 @@ def _StartServer( self ):
       self._GenerateHmacSecret()
 
       with hmaclib.TemporaryHmacSecretFile( self._hmac_secret ) as hmac_file:
-        command = [ PYTHON_EXECUTABLE_PATH,
+        command = [ self._python_binary_path,
                     PATH_TO_JEDIHTTP,
                     '--port', str( self._jedihttp_port ),
                     '--hmac-file-secret', hmac_file.name ]
@@ -244,7 +265,7 @@ def GetSubcommandsMap( self ):
       'StopServer'     : ( lambda self, request_data, args:
                            self.Shutdown() ),
       'RestartServer'  : ( lambda self, request_data, args:
-                           self.RestartServer() )
+                           self.RestartServer( *args ) )
     }
 
 
@@ -334,8 +355,10 @@ def DebugInfo( self, request_data ):
      with self._server_lock:
        if self.ServerIsRunning():
          return ( 'JediHTTP running at 127.0.0.1:{0}\n'
-                  '  stdout log: {1}\n'
-                  '  stderr log: {2}' ).format( self._jedihttp_port,
+                  '  python binary: {1}\n'
+                  '  stdout log: {2}\n'
+                  '  stderr log: {3}' ).format( self._jedihttp_port,
+                                                self._python_binary_path,
                                                 self._logfile_stdout,
                                                 self._logfile_stderr )
 

ycmd/default_settings.json

@@ -40,5 +40,6 @@
   "server_keep_logfiles": 0,
   "gocode_binary_path": "",
   "rust_src_path": "",
-  "racerd_binary_path": ""
+  "racerd_binary_path": "",
+  "python_binary_path": ""
 }

ycmd/tests/clang/get_completions_test.py

@@ -294,12 +294,12 @@ def WorksWithExplicitFlags_test( self ):
 
 
   def NoCompletionsWhenAutoTriggerOff_test( self ):
-    self._ChangeSpecificOptions( { 'auto_trigger': False } )
-    self._app = TestApp( handlers.app )
-    self._app.post_json(
-      '/ignore_extra_conf_file',
-      { 'filepath': self._PathToTestFile( '.ycm_extra_conf.py' ) } )
-    contents = """
+    with self.UserOption( 'auto_trigger', False ):
+      self._app = TestApp( handlers.app )
+      self._app.post_json(
+        '/ignore_extra_conf_file',
+        { 'filepath': self._PathToTestFile( '.ycm_extra_conf.py' ) } )
+      contents = """
 struct Foo {
   int x;
   int y;
@@ -313,16 +313,16 @@ def NoCompletionsWhenAutoTriggerOff_test( self ):
 }
 """
 
-    completion_data = self._BuildRequest( filepath = '/foo.cpp',
-                                          filetype = 'cpp',
-                                          contents = contents,
-                                          line_num = 11,
-                                          column_num = 7,
-                                          compilation_flags = ['-x', 'c++'] )
+      completion_data = self._BuildRequest( filepath = '/foo.cpp',
+                                            filetype = 'cpp',
+                                            contents = contents,
+                                            line_num = 11,
+                                            column_num = 7,
+                                            compilation_flags = ['-x', 'c++'] )
 
-    results = self._app.post_json( '/completions',
-                                   completion_data ).json[ 'completions' ]
-    assert_that( results, empty() )
+      results = self._app.post_json( '/completions',
+                                    completion_data ).json[ 'completions' ]
+      assert_that( results, empty() )
 
 
   def UnknownExtraConfException_test( self ):

ycmd/tests/cs/subcommands_test.py

@@ -628,51 +628,50 @@ def StopServer_KeepLogFiles_test( self ):
 
 
   def _StopServer_KeepLogFiles( self, keeping_log_files ):
-    self._ChangeSpecificOptions(
-      { 'server_keep_logfiles': keeping_log_files } )
-    self._app = TestApp( handlers.app )
-    self._app.post_json(
-      '/ignore_extra_conf_file',
-      { 'filepath': self._PathToTestFile( '.ycm_extra_conf.py' ) } )
-    filepath = self._PathToTestFile( 'testy', 'GotoTestCase.cs' )
-    contents = open( filepath ).read()
-    event_data = self._BuildRequest( filepath = filepath,
-                                     filetype = 'cs',
-                                     contents = contents,
-                                     event_name = 'FileReadyToParse' )
-
-    self._app.post_json( '/event_notification', event_data )
-    self._WaitUntilOmniSharpServerReady( filepath )
-
-    event_data = self._BuildRequest( filetype = 'cs', filepath = filepath )
-
-    debuginfo = self._app.post_json( '/debug_info', event_data ).json
-
-    log_files_match = re.search( "^OmniSharp logfiles:\n(.*)\n(.*)",
-                                 debuginfo,
-                                 re.MULTILINE )
-    stdout_logfiles_location = log_files_match.group( 1 )
-    stderr_logfiles_location = log_files_match.group( 2 )
-
-    try:
-      ok_( os.path.exists(stdout_logfiles_location ),
-           "Logfile should exist at {0}".format( stdout_logfiles_location ) )
-      ok_( os.path.exists( stderr_logfiles_location ),
-           "Logfile should exist at {0}".format( stderr_logfiles_location ) )
-    finally:
-      self._StopOmniSharpServer( filepath )
-
-    if keeping_log_files:
-      ok_( os.path.exists( stdout_logfiles_location ),
-           "Logfile should still exist at "
-           "{0}".format( stdout_logfiles_location ) )
-      ok_( os.path.exists( stderr_logfiles_location ),
-           "Logfile should still exist at "
-           "{0}".format( stderr_logfiles_location ) )
-    else:
-      ok_( not os.path.exists( stdout_logfiles_location ),
-           "Logfile should no longer exist at "
-           "{0}".format( stdout_logfiles_location ) )
-      ok_( not os.path.exists( stderr_logfiles_location ),
-           "Logfile should no longer exist at "
-           "{0}".format( stderr_logfiles_location ) )
+    with self.UserOption( 'server_keep_logfiles', keeping_log_files ):
+      self._app = TestApp( handlers.app )
+      self._app.post_json(
+        '/ignore_extra_conf_file',
+        { 'filepath': self._PathToTestFile( '.ycm_extra_conf.py' ) } )
+      filepath = self._PathToTestFile( 'testy', 'GotoTestCase.cs' )
+      contents = open( filepath ).read()
+      event_data = self._BuildRequest( filepath = filepath,
+                                      filetype = 'cs',
+                                      contents = contents,
+                                      event_name = 'FileReadyToParse' )
+
+      self._app.post_json( '/event_notification', event_data )
+      self._WaitUntilOmniSharpServerReady( filepath )
+
+      event_data = self._BuildRequest( filetype = 'cs', filepath = filepath )
+
+      debuginfo = self._app.post_json( '/debug_info', event_data ).json
+
+      log_files_match = re.search( "^OmniSharp logfiles:\n(.*)\n(.*)",
+                                  debuginfo,
+                                  re.MULTILINE )
+      stdout_logfiles_location = log_files_match.group( 1 )
+      stderr_logfiles_location = log_files_match.group( 2 )
+
+      try:
+        ok_( os.path.exists(stdout_logfiles_location ),
+            "Logfile should exist at {0}".format( stdout_logfiles_location ) )
+        ok_( os.path.exists( stderr_logfiles_location ),
+            "Logfile should exist at {0}".format( stderr_logfiles_location ) )
+      finally:
+        self._StopOmniSharpServer( filepath )
+
+      if keeping_log_files:
+        ok_( os.path.exists( stdout_logfiles_location ),
+            "Logfile should still exist at "
+            "{0}".format( stdout_logfiles_location ) )
+        ok_( os.path.exists( stderr_logfiles_location ),
+            "Logfile should still exist at "
+            "{0}".format( stderr_logfiles_location ) )
+      else:
+        ok_( not os.path.exists( stdout_logfiles_location ),
+            "Logfile should no longer exist at "
+            "{0}".format( stdout_logfiles_location ) )
+        ok_( not os.path.exists( stderr_logfiles_location ),
+            "Logfile should no longer exist at "
+            "{0}".format( stderr_logfiles_location ) )

ycmd/tests/get_completions_test.py

@@ -150,21 +150,21 @@ def UltiSnipsCompleter_Works_test( self ):
 
 
   def UltiSnipsCompleter_UnusedWhenOffWithOption_test( self ):
-    self._ChangeSpecificOptions( { 'use_ultisnips_completer': False } )
-    self._app = TestApp( handlers.app )
+    with self.UserOption( 'use_ultisnips_completer', False ):
+      self._app = TestApp( handlers.app )
 
-    event_data = self._BuildRequest(
-      event_name = 'BufferVisit',
-      ultisnips_snippets = [
-          {'trigger': 'foo', 'description': 'bar'},
-          {'trigger': 'zoo', 'description': 'goo'},
-      ] )
+      event_data = self._BuildRequest(
+        event_name = 'BufferVisit',
+        ultisnips_snippets = [
+            {'trigger': 'foo', 'description': 'bar'},
+            {'trigger': 'zoo', 'description': 'goo'},
+        ] )
 
-    self._app.post_json( '/event_notification', event_data )
+      self._app.post_json( '/event_notification', event_data )
 
-    completion_data = self._BuildRequest( contents = 'oo ',
-                                          column_num = 3 )
+      completion_data = self._BuildRequest( contents = 'oo ',
+                                            column_num = 3 )
 
-    eq_( [],
-         self._app.post_json( '/completions',
-                              completion_data ).json[ 'completions' ] )
+      eq_( [],
+          self._app.post_json( '/completions',
+                                completion_data ).json[ 'completions' ] )

ycmd/tests/handlers_test.py

@@ -50,6 +50,18 @@ def PatchCompleter( self, completer, filetype ):
       yield
 
 
+  @contextlib.contextmanager
+  def UserOption( self, key, value ):
+    try:
+      current_options = dict( user_options_store.GetAll() )
+      user_options = current_options.copy()
+      user_options.update( { key: value } )
+      handlers.UpdateUserOptions( user_options )
+      yield
+    finally:
+      handlers.UpdateUserOptions( current_options )
+
+
   @staticmethod
   def _BuildRequest( **kwargs ):
     return BuildRequest( **kwargs )
@@ -77,13 +89,6 @@ def _CompletionLocationMatcher( location_type, value ):
                                  has_entry( location_type, value ) ) )
 
 
-  @staticmethod
-  def _ChangeSpecificOptions( options ):
-    current_options = dict( user_options_store.GetAll() )
-    current_options.update( options )
-    handlers.UpdateUserOptions( current_options )
-
-
   @staticmethod
   def _ErrorMatcher( cls, msg = None ):
     """ Returns a hamcrest matcher for a server exception response """

ycmd/tests/python/get_completions_test.py

@@ -27,6 +27,11 @@
 
 class Python_GetCompletions_test( Python_Handlers_test ):
 
+  def setUp( self ):
+    super( Python_GetCompletions_test, self ).setUp()
+    self.WaitUntilJediHTTPServerReady()
+
+
   def _RunTest( self, test ):
     """
     Method to run a simple completion test and verify the result

ycmd/tests/python/python_handlers_test.py

@@ -27,11 +27,6 @@ def __init__( self ):
     self._file = __file__
 
 
-  def setUp( self ):
-    super( Python_Handlers_test, self ).setUp()
-    self.WaitUntilJediHTTPServerReady()
-
-
   def tearDown( self ):
     self.StopJediHTTPServer()
 
@@ -54,4 +49,8 @@ def StopJediHTTPServer( self ):
     request = self._BuildRequest( completer_target = 'filetype_default',
                                   command_arguments = [ 'StopServer' ],
                                   filetype = 'python' )
-    self._app.post_json( '/run_completer_command', request )
+    # We don't actually start a JediHTTP server on every test, so we just
+    # ignore errors when stopping the server
+    self._app.post_json( '/run_completer_command',
+                         request,
+                         expect_errors = True )

ycmd/tests/python/subcommands_test.py

@@ -25,6 +25,11 @@
 
 class Python_Subcommands_test( Python_Handlers_test ):
 
+  def setUp( self ):
+    super( Python_Subcommands_test, self ).setUp()
+    self.WaitUntilJediHTTPServerReady()
+
+
   def GoTo_Variation_ZeroBasedLineAndColumn_test( self ):
     tests = [
           {