ycm-core / ycmd

A code-completion & code-comprehension server
https://ycm-core.github.io/ycmd/
GNU General Public License v3.0
1.69k stars 764 forks source link

Fix g:ycm_roslyn_binary_path being ignored #1721

Closed dkaszews closed 8 months ago

dkaszews commented 9 months ago

Fixes the g:ycm_roslyn_binary_path only being used in prechecks, and the actual server always being started from the ycmd/third_party directory.

Also fixes the :YcmDebugInfo printing the directory containing default OmniSharp executable (and not the executable file itself); now prints full command with arguments used to start the server.


This change is Reviewable

bstaletic commented 9 months ago

Also, we have merged the unicode/regex update, so you should rebase.

dkaszews commented 9 months ago

Apparently @patch does not work with @staticmethod before Python 3.10 and CI uses 3.8. Wouldn't be an issue if stupid Python had multiline lambdas. Will have to look at alternative solutions.

dkaszews commented 9 months ago

That did not work either, was getting errors that instance method was called without self. Found alternative of patching inside test with local function.

dkaszews commented 9 months ago

Any clue why this fails only on older Windows?

puremourning commented 8 months ago

I have re-run them. Let's see if it was CI flake.

puremourning commented 8 months ago

Nope, looks like the test really does just fail on windows:

======================================================================
ERROR: test_GetCompleter_RoslynFromUserOption (ycmd.tests.cs.debug_info_test.DebugInfoTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\unittest\mock.py", line 1325, in patched
    return func(*newargs, **newkeywargs)
  File "D:\a\ycmd\ycmd\ycmd\tests\cs\__init__.py", line 70, in Wrapper
    test( test_case_instance, app, *args, **kwargs )
  File "D:\a\ycmd\ycmd\ycmd\tests\cs\debug_info_test.py", line 219, in test_GetCompleter_RoslynFromUserOption
    with WrapOmniSharpServer( app, filepath ):
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "D:\a\ycmd\ycmd\ycmd\tests\cs\__init__.py", line 108, in WrapOmniSharpServer
    GetDiagnostics( app, filepath )
  File "D:\a\ycmd\ycmd\ycmd\tests\cs\__init__.py", line 99, in GetDiagnostics
    return app.post_json( '/event_notification', event_data ).json
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\webtest\utils.py", line 34, in wrapper
    return self._gen_request(method, url, **kw)
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\webtest\app.py", line 749, in _gen_request
    return self.do_request(req, status=status,
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\webtest\app.py", line 646, in do_request
    self._check_status(status, res)
  File "C:\hostedtoolcache\windows\Python\3.8.10\x64\lib\site-packages\webtest\app.py", line 675, in _check_status
    raise AppError(
webtest.app.AppError: Bad response: 500 Internal Server Error (not 200 OK or 3xx redirect for http://localhost/event_notification)
b'{"exception":{"TYPE":"AssertionError"},"message":"\\nExpected: \'my_roslyn.exe\'\\n     but: was \'-p\'\\n","traceback":"Traceback (most recent call last):\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\third_party\\\\bottle\\\\bottle.py\\", line 876, in _handle\\n    return route.call(**args)\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\third_party\\\\bottle\\\\bottle.py\\", line 1756, in wrapper\\n    rv = callback(*a, **ka)\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\handlers.py\\", line 65, in EventNotification\\n    response_data = getattr( _server_state.GetFiletypeCompleter( filetypes ),\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\completers\\\\cs\\\\cs_completer.py\\", line 262, in OnFileReadyToParse\\n    solutioncompleter._StartServer()\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\completers\\\\cs\\\\cs_completer.py\\", line 421, in _StartServer\\n    return self._StartServerNoLock()\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\completers\\\\cs\\\\cs_completer.py\\", line 466, in _StartServerNoLock\\n    self._omnisharp_phandle = utils.SafePopen(\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\utils.py\\", line 370, in SafePopen\\n    return subprocess.Popen( args, **kwargs )\\n  File \\"C:\\\\hostedtoolcache\\\\windows\\\\Python\\\\3.8.10\\\\x64\\\\lib\\\\unittest\\\\mock.py\\", line 1081, in __call__\\n    return self._mock_call(*args, **kwargs)\\n  File \\"C:\\\\hostedtoolcache\\\\windows\\\\Python\\\\3.8.10\\\\x64\\\\lib\\\\unittest\\\\mock.py\\", line 1085, in _mock_call\\n    return self._execute_mock_call(*args, **kwargs)\\n  File \\"C:\\\\hostedtoolcache\\\\windows\\\\Python\\\\3.8.10\\\\x64\\\\lib\\\\unittest\\\\mock.py\\", line 1155, in _execute_mock_call\\n    return self._mock_wraps(*args, **kwargs)\\n  File \\"D:\\\\a\\\\ycmd\\\\ycmd\\\\ycmd\\\\tests\\\\cs\\\\debug_info_test.py\\", line 212, in _popen_mock\\n    assert_that( cmdline[ 1 ], equal_to( \'my_roslyn.exe\' ) )\\n  File \\"C:\\\\hostedtoolcache\\\\windows\\\\Python\\\\3.8.10\\\\x64\\\\lib\\\\site-packages\\\\hamcrest\\\\core\\\\assert_that.py\\", line 58, in assert_that\\n    _assert_match(actual=actual_or_assertion, matcher=matcher, reason=reason)\\n  File \\"C:\\\\hostedtoolcache\\\\windows\\\\Python\\\\3.8.10\\\\x64\\\\lib\\\\site-packages\\\\hamcrest\\\\core\\\\assert_that.py\\", line 73, in _assert_match\\n    raise AssertionError(description)\\nAssertionError: \\nExpected: \'my_roslyn.exe\'\\n     but: was \'-p\'\\n\\n"}'

nExpected: \'my_roslyn.exe\'\n but: was \'-p\'\n\n"}'

dkaszews commented 8 months ago

Could not get through the callstack myself, but now it just looks like it fails because on Windows it runs with my_roslyn.exe ... instead of mono my_roslyn.exe .... Easy enough fix, that's why I asked on gitter whether this test will run on Windows 🤷

puremourning commented 8 months ago

yeah, it's a bit cryptic the way the failure is reported!

codecov[bot] commented 8 months ago

Codecov Report

Merging #1721 (e318b5e) into master (f4cfdb8) will increase coverage by 0.00%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1721 +/- ## ======================================= Coverage 95.45% 95.45% ======================================= Files 83 83 Lines 8136 8143 +7 Branches 163 163 ======================================= + Hits 7766 7773 +7 Misses 320 320 Partials 50 50 ```
dkaszews commented 8 months ago

@puremourning Do you need me to squash manually? I'm AFK so might be a bit hard.

The codecov fails seem to be because I rewrote bits of code to be shorter so the line percentage went down 🙄

puremourning commented 8 months ago

@mergifyio squash

mergify[bot] commented 8 months ago

squash

✅ Pull request squashed successfully

mergify[bot] commented 8 months ago

Thanks for sending a PR!

mergify[bot] commented 8 months ago

Thanks for sending a PR!