Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions python/ycm/client/messages_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ycm.client.base_request import BaseRequest, BuildRequestData
from ycm.vimsupport import PostVimMessage

import json
import logging

_logger = logging.getLogger( __name__ )
Expand Down Expand Up @@ -56,8 +57,21 @@ def Poll( self, diagnostics_handler ):
# Nothing yet...
return True

response = self.HandleFuture( self._response_future,
display_message = False )
# Non-blocking extraction since done() is True. We avoid HandleFuture()
# because it calls response.read() which blocks on large responses.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code also calls response.read() so please can you explain in the comment and in detail in the PR description why this fixes the issue?

try:
response = self._response_future.result( timeout = 0 )
response_text = response.read()
response.close()
if response_text:
response = json.loads( response_text )
else:
response = None
except Exception:
_logger.exception( 'Error while handling server response in Poll' )
# Server returned an exception.
return False

if response is None:
# Server returned an exception.
return False
Expand Down
116 changes: 113 additions & 3 deletions python/ycm/tests/client/messages_request_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
# You should have received a copy of the GNU General Public License
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.

import json
from ycm.tests.test_utils import MockVimModule
MockVimModule()

from hamcrest import assert_that, equal_to
from unittest import TestCase
from unittest.mock import patch, call
from unittest.mock import patch, call, MagicMock

from ycm.client.messages_request import _HandlePollResponse
from ycm.tests.test_utils import ExtendedMock
from ycm.client.messages_request import _HandlePollResponse, MessagesPoll
from ycm.tests.test_utils import ExtendedMock, MockVimBuffers, VimBuffer


class MessagesRequestTest( TestCase ):
Expand Down Expand Up @@ -138,3 +139,112 @@ def test_HandlePollResponse_MultipleMessagesAndDiagnostics(
warning=False,
truncate=True ),
] )


def test_Poll_FirstCall_StartsRequest( self ):
test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] )

with MockVimBuffers( [ test_buffer ], [ test_buffer ] ):
poller = MessagesPoll( test_buffer )

# Mock the async request method to avoid actual HTTP call
mock_future = MagicMock()
with patch.object( poller, 'PostDataToHandlerAsync',
return_value = mock_future ) as mock_post:
# First poll should start request
result = poller.Poll( None )

assert_that( result, equal_to( True ) )
mock_post.assert_called_once()


def test_Poll_FutureNotDone_ReturnsTrue( self ):
test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] )

with MockVimBuffers( [ test_buffer ], [ test_buffer ] ):
poller = MessagesPoll( test_buffer )

# Mock future that is not done
mock_future = MagicMock()
mock_future.done.return_value = False
poller._response_future = mock_future

# Should return True without extracting result
result = poller.Poll( None )

assert_that( result, equal_to( True ) )
mock_future.result.assert_not_called()


def test_Poll_FutureReady_ExtractsResponseNonBlocking( self ):
test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] )

with MockVimBuffers( [ test_buffer ], [ test_buffer ] ):
poller = MessagesPoll( test_buffer )

# Mock completed future with response
mock_response = MagicMock()
mock_response.read.return_value = json.dumps(
[ { 'message': 'test' } ] ).encode()
mock_response.close = MagicMock()

mock_future = MagicMock()
mock_future.done.return_value = True
mock_future.result.return_value = mock_response
poller._response_future = mock_future

# Mock diagnostics handler
mock_handler = MagicMock()

# Should extract result with timeout=0 (non-blocking)
with patch( 'ycm.client.messages_request.PostVimMessage' ):
result = poller.Poll( mock_handler )

# Verify non-blocking extraction
mock_future.result.assert_called_once_with( timeout = 0 )
mock_response.read.assert_called_once()
mock_response.close.assert_called_once()
assert_that( result, equal_to( True ) )


def test_Poll_FutureException_ReturnsFalse( self ):
test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] )

with MockVimBuffers( [ test_buffer ], [ test_buffer ] ):
poller = MessagesPoll( test_buffer )

# Mock future that raises exception
mock_future = MagicMock()
mock_future.done.return_value = True
mock_future.result.side_effect = Exception( 'Connection error' )
poller._response_future = mock_future

# Should catch exception and return False
result = poller.Poll( None )

assert_that( result, equal_to( False ) )


def test_Poll_DoesNotCallHandleFuture( self ):
"""Verify that Poll() does NOT call HandleFuture() to avoid blocking."""
test_buffer = VimBuffer( 'test_buffer', number = 1, contents = [ '' ] )

with MockVimBuffers( [ test_buffer ], [ test_buffer ] ):
poller = MessagesPoll( test_buffer )

# Mock completed future
mock_response = MagicMock()
mock_response.read.return_value = json.dumps( True ).encode()
mock_response.close = MagicMock()

mock_future = MagicMock()
mock_future.done.return_value = True
mock_future.result.return_value = mock_response
poller._response_future = mock_future

# Spy on HandleFuture to ensure it's NOT called
with patch.object( poller, 'HandleFuture' ) as mock_handle_future:
# Poll should not call HandleFuture
poller.Poll( None )

mock_handle_future.assert_not_called()
2 changes: 1 addition & 1 deletion python/ycm/tests/mock_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def done( self ):
return self._done


def result( self ):
def result( self, timeout = None ):
return self._result


Expand Down