Fix g:ycm_roslyn_binary_path being ignored (#1721)

Fix g:ycm_roslyn_binary_path being ignored

* Add test

* Clean test

* Fix formatting

* Fix UT

* Fix UT for Python 3.8

* Make popen mock args more readable

* Fix UT on Windows

* Explicit index

* Explicit index name

ycmd/completers/cs/cs_completer.py

@@ -349,7 +349,7 @@ def DebugInfo( self, request_data ):
       omnisharp_server = responses.DebugInfoServer(
         name = 'OmniSharp',
         handle = completer._omnisharp_phandle,
-        executable = PATH_TO_ROSLYN_OMNISHARP,
+        executable = ' '.join( completer._ConstructOmnisharpCommand() ),
         address = 'localhost',
         port = completer._omnisharp_port,
         logfiles = [ completer._filename_stdout, completer._filename_stderr ],
@@ -398,6 +398,7 @@ def __init__( self,
     self._keep_logfiles = keep_logfiles
     self._filename_stderr = None
     self._filename_stdout = None
+    self._omnisharp_command = None
     self._omnisharp_port = None
     self._omnisharp_phandle = None
     self._desired_omnisharp_port = desired_omnisharp_port
@@ -420,6 +421,24 @@ def _StartServer( self ):
       return self._StartServerNoLock()
 
 
+  def _ConstructOmnisharpCommand( self ):
+    if self._omnisharp_command:
+      return self._omnisharp_command
+
+    self._ChooseOmnisharpPort()
+    self._omnisharp_command = [ self._roslyn_path,
+                                '-p',
+                                str( self._omnisharp_port ),
+                                '-s',
+                                str( self._solution_path ) ]
+
+    if ( not utils.OnWindows()
+         and self._roslyn_path.endswith( '.exe' ) ):
+      self._omnisharp_command.insert( 0, self._mono_path )
+
+    return self._omnisharp_command
+
+
   def _StartServerNoLock( self ):
     """ Start the OmniSharp server if not already running. Use a lock to avoid
     starting the server multiple times for the same solution. """
@@ -429,18 +448,7 @@ def _StartServerNoLock( self ):
     LOGGER.info( 'Starting OmniSharp server' )
     LOGGER.info( 'Loading solution file %s', self._solution_path )
 
-    self._ChooseOmnisharpPort()
-
-    command = [ PATH_TO_OMNISHARP_ROSLYN_BINARY,
-                '-p',
-                str( self._omnisharp_port ),
-                '-s',
-                str( self._solution_path ) ]
-
-    if ( not utils.OnWindows()
-         and self._roslyn_path.endswith( '.exe' ) ):
-      command.insert( 0, self._mono_path )
-
+    command = self._ConstructOmnisharpCommand()
     LOGGER.info( 'Starting OmniSharp server with: %s', command )
 
     solutionfile = os.path.basename( self._solution_path )
@@ -513,6 +521,7 @@ def _ForceStopServer( self ):
 
 
   def _CleanUp( self ):
+    self._omnisharp_command = None
     self._omnisharp_port = None
     self._omnisharp_phandle = None
     if not self._keep_logfiles:

ycmd/tests/cs/debug_info_test.py

@@ -17,12 +17,19 @@
 
 from hamcrest import ( assert_that, contains_exactly, empty, equal_to,
                        has_entries, has_entry, instance_of )
+
+from subprocess import Popen as _mockable_popen
+
 from unittest.mock import patch
 from unittest import TestCase
 
+from ycmd.completers.cs.cs_completer import PATH_TO_OMNISHARP_ROSLYN_BINARY
 from ycmd.completers.cs.hook import GetCompleter
 from ycmd.tests.cs import setUpModule, tearDownModule # noqa
-from ycmd.tests.cs import PathToTestFile, SharedYcmd
+from ycmd.tests.cs import ( PathToTestFile,
+                            SharedYcmd,
+                            IsolatedYcmd,
+                            WrapOmniSharpServer )
 from ycmd.tests.test_utils import ( BuildRequest,
                                     WaitUntilCompleterServerReady )
 from ycmd import user_options_store
@@ -197,14 +204,24 @@ def test_GetCompleter_RoslynNotFound( *args ):
     assert_that( not GetCompleter( user_options_store.GetAll() ) )
 
 
-  @patch( 'ycmd.completers.cs.cs_completer.FindExecutableWithFallback',
-          wraps = lambda x, fb: x if x == 'roslyn' else fb )
   @patch( 'os.path.isfile', return_value = True )
-  def test_GetCompleter_RoslynFromUserOption( *args ):
-    user_options = user_options_store.GetAll().copy(
-        roslyn_binary_path = 'roslyn' )
-    assert_that( GetCompleter( user_options )._roslyn_path,
-                 equal_to( 'roslyn' ) )
+  @IsolatedYcmd( { 'roslyn_binary_path': 'my_roslyn.exe' } )
+  def test_GetCompleter_RoslynFromUserOption( self, app, *args ):
+    # `@patch` does not play nice with functions defined at class scope
+    def _popen_mock( cmdline, **kwargs ):
+      exe_index = 1 if cmdline[ 0 ].endswith( 'mono' ) else 0
+      assert_that( cmdline[ exe_index ], equal_to( 'my_roslyn.exe' ) )
+      # Need to redirect to real binary to allow test to pass
+      cmdline[ exe_index ] = PATH_TO_OMNISHARP_ROSLYN_BINARY
+      return _mockable_popen( cmdline, **kwargs )
+
+    filepath = PathToTestFile( 'testy', 'Program.cs' )
+    with patch( 'subprocess.Popen', wraps = _popen_mock ) as popen_mock:
+      with WrapOmniSharpServer( app, filepath ):
+        request = BuildRequest( filepath = filepath, filetype = 'cs' )
+        app.post_json( '/debug_info', request )
+
+    popen_mock.assert_called()
 
 
   @patch( 'os.path.isfile', return_value = False )