Auto merge of #1927 - micbou:event-notification-test, r=Valloric
[READY] Fix issue in EventNotification tests While I was reviewing PR #1905, I found an issue with the `EventNotification` tests. It's easy to reproduce. You just need to comment [this line in `python/ycm/youcompleteme.py`](https://github.com/Valloric/YouCompleteMe/blob/master/python/ycm/youcompleteme.py#L508) and run the tests. With this change, the `EventNotification` tests should fail since the second call of `ValidateParseRequest` re-raises the warning. However, tests are still passing because `assert_has_calls` does not check if a call was not made. For example, if `functionA` is called twice, both `assert_has_calls( [ functionA ] )` and `assert_has_calls( [ functionA, functionA ] )` are successful. To fix this, we just need to check the number of calls using `call_count`. This is done by creating a subclass of `MagicMock` implementing the `assert_has_exact_calls` method and using it in tests. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/1927) <!-- Reviewable:end -->
This commit is contained in:
commit
1201115141
@ -18,6 +18,7 @@
|
|||||||
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
|
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
from mock import MagicMock
|
from mock import MagicMock
|
||||||
|
from hamcrest import assert_that, equal_to
|
||||||
import re
|
import re
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
@ -116,3 +117,10 @@ def MockVimModule():
|
|||||||
sys.modules[ 'vim' ] = VIM_MOCK
|
sys.modules[ 'vim' ] = VIM_MOCK
|
||||||
|
|
||||||
return VIM_MOCK
|
return VIM_MOCK
|
||||||
|
|
||||||
|
|
||||||
|
class ExtendedMock( MagicMock ):
|
||||||
|
|
||||||
|
def assert_has_exact_calls( self, calls, any_order = False ):
|
||||||
|
self.assert_has_calls( calls, any_order )
|
||||||
|
assert_that( self.call_count, equal_to( len( calls ) ) )
|
||||||
|
@ -16,10 +16,11 @@
|
|||||||
# You should have received a copy of the GNU General Public License
|
# You should have received a copy of the GNU General Public License
|
||||||
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
|
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
from ycm.test_utils import MockVimModule
|
from ycm.test_utils import MockVimModule, ExtendedMock
|
||||||
MockVimModule()
|
MockVimModule()
|
||||||
|
|
||||||
import contextlib, os
|
import contextlib
|
||||||
|
import os
|
||||||
|
|
||||||
from ycm.youcompleteme import YouCompleteMe
|
from ycm.youcompleteme import YouCompleteMe
|
||||||
from ycmd import user_options_store
|
from ycmd import user_options_store
|
||||||
@ -36,6 +37,7 @@ DEFAULT_CLIENT_OPTIONS = {
|
|||||||
'extra_conf_vim_data': [],
|
'extra_conf_vim_data': [],
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def PostVimMessage_Call( message ):
|
def PostVimMessage_Call( message ):
|
||||||
"""Return a mock.call object for a call to vimsupport.PostVimMesasge with the
|
"""Return a mock.call object for a call to vimsupport.PostVimMesasge with the
|
||||||
supplied message"""
|
supplied message"""
|
||||||
@ -74,7 +76,7 @@ def MockArbitraryBuffer( filetype, native_available = True ):
|
|||||||
raise ValueError( 'Unexpected evaluation' )
|
raise ValueError( 'Unexpected evaluation' )
|
||||||
|
|
||||||
# Arbitrary, but valid, cursor position
|
# Arbitrary, but valid, cursor position
|
||||||
vim_current.window.cursor = (1,2)
|
vim_current.window.cursor = ( 1, 2 )
|
||||||
|
|
||||||
# Arbitrary, but valid, single buffer open
|
# Arbitrary, but valid, single buffer open
|
||||||
current_buffer = MagicMock()
|
current_buffer = MagicMock()
|
||||||
@ -129,7 +131,7 @@ class EventNotification_test( object ):
|
|||||||
|
|
||||||
def setUp( self ):
|
def setUp( self ):
|
||||||
options = dict( user_options_store.DefaultOptions() )
|
options = dict( user_options_store.DefaultOptions() )
|
||||||
options.update ( DEFAULT_CLIENT_OPTIONS )
|
options.update( DEFAULT_CLIENT_OPTIONS )
|
||||||
user_options_store.SetAll( options )
|
user_options_store.SetAll( options )
|
||||||
|
|
||||||
self.server_state = YouCompleteMe( user_options_store.GetAll() )
|
self.server_state = YouCompleteMe( user_options_store.GetAll() )
|
||||||
@ -141,7 +143,7 @@ class EventNotification_test( object ):
|
|||||||
self.server_state.OnVimLeave()
|
self.server_state.OnVimLeave()
|
||||||
|
|
||||||
|
|
||||||
@patch( 'vim.command' )
|
@patch( 'vim.command', new_callable = ExtendedMock )
|
||||||
def FileReadyToParse_NonDiagnostic_Error_test( self, vim_command ):
|
def FileReadyToParse_NonDiagnostic_Error_test( self, vim_command ):
|
||||||
# This test validates the behaviour of YouCompleteMe.ValidateParseRequest in
|
# This test validates the behaviour of YouCompleteMe.ValidateParseRequest in
|
||||||
# combination with YouCompleteMe.OnFileReadyToParse when the completer
|
# combination with YouCompleteMe.OnFileReadyToParse when the completer
|
||||||
@ -158,13 +160,13 @@ class EventNotification_test( object ):
|
|||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
# The first call raises a warning
|
# The first call raises a warning
|
||||||
vim_command.assert_has_calls( [
|
vim_command.assert_has_exact_calls( [
|
||||||
PostVimMessage_Call( ERROR_TEXT ),
|
PostVimMessage_Call( ERROR_TEXT ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
# Subsequent calls don't re-raise the warning
|
# Subsequent calls don't re-raise the warning
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
vim_command.assert_has_calls( [
|
vim_command.assert_has_exact_calls( [
|
||||||
PostVimMessage_Call( ERROR_TEXT ),
|
PostVimMessage_Call( ERROR_TEXT ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
@ -172,7 +174,7 @@ class EventNotification_test( object ):
|
|||||||
self.server_state.OnFileReadyToParse()
|
self.server_state.OnFileReadyToParse()
|
||||||
assert self.server_state.DiagnosticsForCurrentFileReady()
|
assert self.server_state.DiagnosticsForCurrentFileReady()
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
vim_command.assert_has_calls( [
|
vim_command.assert_has_exact_calls( [
|
||||||
PostVimMessage_Call( ERROR_TEXT ),
|
PostVimMessage_Call( ERROR_TEXT ),
|
||||||
PostVimMessage_Call( ERROR_TEXT ),
|
PostVimMessage_Call( ERROR_TEXT ),
|
||||||
] )
|
] )
|
||||||
@ -187,8 +189,10 @@ class EventNotification_test( object ):
|
|||||||
vim_command.assert_not_called()
|
vim_command.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
@patch( 'ycm.client.event_notification._LoadExtraConfFile' )
|
@patch( 'ycm.client.event_notification._LoadExtraConfFile',
|
||||||
@patch( 'ycm.client.event_notification._IgnoreExtraConfFile' )
|
new_callable = ExtendedMock )
|
||||||
|
@patch( 'ycm.client.event_notification._IgnoreExtraConfFile',
|
||||||
|
new_callable = ExtendedMock )
|
||||||
def FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
|
def FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
|
||||||
self,
|
self,
|
||||||
ignore_extra_conf,
|
ignore_extra_conf,
|
||||||
@ -211,25 +215,26 @@ class EventNotification_test( object ):
|
|||||||
|
|
||||||
# When the user accepts the extra conf, we load it
|
# When the user accepts the extra conf, we load it
|
||||||
with patch( 'ycm.vimsupport.PresentDialog',
|
with patch( 'ycm.vimsupport.PresentDialog',
|
||||||
return_value = 0 ) as present_dialog:
|
return_value = 0,
|
||||||
|
new_callable = ExtendedMock ) as present_dialog:
|
||||||
self.server_state.OnFileReadyToParse()
|
self.server_state.OnFileReadyToParse()
|
||||||
assert self.server_state.DiagnosticsForCurrentFileReady()
|
assert self.server_state.DiagnosticsForCurrentFileReady()
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
] )
|
] )
|
||||||
load_extra_conf.assert_has_calls( [
|
load_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
# Subsequent calls don't re-raise the warning
|
# Subsequent calls don't re-raise the warning
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE )
|
PresentDialog_Confirm_Call( MESSAGE )
|
||||||
] )
|
] )
|
||||||
load_extra_conf.assert_has_calls( [
|
load_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
@ -238,36 +243,37 @@ class EventNotification_test( object ):
|
|||||||
assert self.server_state.DiagnosticsForCurrentFileReady()
|
assert self.server_state.DiagnosticsForCurrentFileReady()
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
] )
|
] )
|
||||||
load_extra_conf.assert_has_calls( [
|
load_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
# When the user rejects the extra conf, we reject it
|
# When the user rejects the extra conf, we reject it
|
||||||
with patch( 'ycm.vimsupport.PresentDialog',
|
with patch( 'ycm.vimsupport.PresentDialog',
|
||||||
return_value = 1 ) as present_dialog:
|
return_value = 1,
|
||||||
|
new_callable = ExtendedMock ) as present_dialog:
|
||||||
self.server_state.OnFileReadyToParse()
|
self.server_state.OnFileReadyToParse()
|
||||||
assert self.server_state.DiagnosticsForCurrentFileReady()
|
assert self.server_state.DiagnosticsForCurrentFileReady()
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
] )
|
] )
|
||||||
ignore_extra_conf.assert_has_calls( [
|
ignore_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
# Subsequent calls don't re-raise the warning
|
# Subsequent calls don't re-raise the warning
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE )
|
PresentDialog_Confirm_Call( MESSAGE )
|
||||||
] )
|
] )
|
||||||
ignore_extra_conf.assert_has_calls( [
|
ignore_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
|
||||||
@ -276,11 +282,11 @@ class EventNotification_test( object ):
|
|||||||
assert self.server_state.DiagnosticsForCurrentFileReady()
|
assert self.server_state.DiagnosticsForCurrentFileReady()
|
||||||
self.server_state.ValidateParseRequest()
|
self.server_state.ValidateParseRequest()
|
||||||
|
|
||||||
present_dialog.assert_has_calls( [
|
present_dialog.assert_has_exact_calls( [
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
PresentDialog_Confirm_Call( MESSAGE ),
|
PresentDialog_Confirm_Call( MESSAGE ),
|
||||||
] )
|
] )
|
||||||
ignore_extra_conf.assert_has_calls( [
|
ignore_extra_conf.assert_has_exact_calls( [
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
call( FILE_NAME ),
|
call( FILE_NAME ),
|
||||||
] )
|
] )
|
||||||
|
Loading…
Reference in New Issue
Block a user