Auto merge of #2391 - micbou:refactor-tests-current-directory, r=Valloric

[READY] Fix unicode issues when working with current directory on Python 2

This PR replaces each occurrence of `os.getcwd` by [the function helper `GetCurrentDirectory`](https://github.com/Valloric/ycmd/blob/master/ycmd/utils.py#L450) added in PR https://github.com/Valloric/ycmd/pull/622. This fixes three different issues covered by the new tests.

As in PR https://github.com/Valloric/ycmd/pull/622, I am only pushing the tests for now so that you can see the errors yourself. Note that these tests are only failing on Python 2 and the `CreateCompletionRequest_UnicodeWorkingDirectory` test only fails on Windows.

Fixes #2375.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2391)
<!-- Reviewable:end -->

python/ycm/tests/__init__.py

@@ -27,6 +27,7 @@ from ycm.tests.test_utils import MockVimModule
 MockVimModule()
 
 import functools
+import os
 import requests
 import time
 
@@ -48,6 +49,11 @@ DEFAULT_CLIENT_OPTIONS = {
 }
 
 
+def PathToTestFile( *args ):
+  dir_of_current_script = os.path.dirname( os.path.abspath( __file__ ) )
+  return os.path.join( dir_of_current_script, 'testdata', *args )
+
+
 def _MakeUserOptions( custom_options = {} ):
   options = dict( user_options_store.DefaultOptions() )
   options.update( DEFAULT_CLIENT_OPTIONS )

python/ycm/tests/completion_test.py

@@ -0,0 +1,54 @@
+# coding: utf-8
+#
+# Copyright (C) 2016 YouCompleteMe contributors
+#
+# This file is part of YouCompleteMe.
+#
+# YouCompleteMe is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# YouCompleteMe is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import unicode_literals
+from __future__ import print_function
+from __future__ import division
+from __future__ import absolute_import
+from future import standard_library
+standard_library.install_aliases()
+from builtins import *  # noqa
+
+from ycm.tests.test_utils import ( CurrentWorkingDirectory, MockVimModule,
+                                   MockVimBuffers, VimBuffer )
+MockVimModule()
+
+from hamcrest import assert_that, empty, has_entries
+
+from ycm.tests import PathToTestFile, YouCompleteMeInstance
+
+
+@YouCompleteMeInstance()
+def CreateCompletionRequest_UnicodeWorkingDirectory_test( ycm ):
+  unicode_dir = PathToTestFile( 'uni¢𐍈d€' )
+  current_buffer = VimBuffer( PathToTestFile( 'uni¢𐍈d€', 'current_buffer' ) )
+
+  with CurrentWorkingDirectory( unicode_dir ):
+    with MockVimBuffers( [ current_buffer ], current_buffer, ( 5, 2 ) ):
+      ycm.CreateCompletionRequest(),
+
+    results = ycm.GetCompletions()
+
+  assert_that(
+    results,
+    has_entries( {
+      'words': empty(),
+      'refresh': 'always'
+    } )
+  )

python/ycm/tests/event_notification_test.py

@@ -1,3 +1,5 @@
+# coding: utf-8
+#
 # Copyright (C) 2015-2016 YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
@@ -23,17 +25,18 @@ from future import standard_library
 standard_library.install_aliases()
 from builtins import *  # noqa
 
-from ycm.tests.test_utils import ExtendedMock, MockVimModule, VimBuffer
+from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
+                                   MockVimBuffers, MockVimModule, VimBuffer )
 MockVimModule()
 
 import contextlib
 import os
 
-from ycm.tests import YouCompleteMeInstance
+from ycm.tests import PathToTestFile, YouCompleteMeInstance
 from ycmd.responses import ( BuildDiagnosticData, Diagnostic, Location, Range,
                              UnknownExtraConf, ServerError )
 
-from hamcrest import assert_that, contains, has_entries
+from hamcrest import assert_that, contains, has_entries, has_item
 from mock import call, MagicMock, patch
 from nose.tools import eq_, ok_
 
@@ -56,22 +59,16 @@ def UnplaceSign_Call( sign_id, buffer_num ):
 
 
 @contextlib.contextmanager
-def MockArbitraryBuffer( filetype, native_available = True ):
+def MockArbitraryBuffer( filetype ):
-  """Used via the with statement, set up mocked versions of the vim module such
+  """Used via the with statement, set up a single buffer with an arbitrary name
-  that a single buffer is open with an arbitrary name and arbirary contents. Its
+  and no contents. Its filetype is set to the supplied filetype."""
-  filetype is set to the supplied filetype"""
 
   # Arbitrary, but valid, single buffer open.
   current_buffer = VimBuffer( os.path.realpath( 'TEST_BUFFER' ),
                               window = 1,
                               filetype = filetype )
 
-  # The rest just mock up the Vim module so that our single arbitrary buffer
+  with MockVimBuffers( [ current_buffer ], current_buffer ):
-  # makes sense to vimsupport module.
-  with patch( 'vim.buffers', [ current_buffer ] ):
-    with patch( 'vim.current.buffer', current_buffer ):
-      # Arbitrary but valid cursor position.
-      with patch( 'vim.current.window.cursor', ( 1, 2 ) ):
     yield
 
 
@@ -352,6 +349,44 @@ def _Check_FileReadyToParse_Diagnostic_Clean( ycm, vim_command ):
       eq_( ycm.GetWarningCount(), 0 )
 
 
+@patch( 'ycm.youcompleteme.YouCompleteMe._AddUltiSnipsDataIfNeeded' )
+@YouCompleteMeInstance( { 'collect_identifiers_from_tags_files': 1 } )
+def EventNotification_FileReadyToParse_TagFiles_UnicodeWorkingDirectory_test(
+    ycm, *args ):
+  unicode_dir = PathToTestFile( 'uni¢𐍈d€' )
+  current_buffer_file = PathToTestFile( 'uni¢𐍈d€', 'current_buffer' )
+  current_buffer = VimBuffer( name = current_buffer_file,
+                              contents = [ 'current_buffer_contents' ],
+                              filetype = 'some_filetype' )
+
+  with patch( 'ycm.client.base_request.BaseRequest.'
+              'PostDataToHandlerAsync' ) as post_data_to_handler_async:
+    with CurrentWorkingDirectory( unicode_dir ):
+      with MockVimBuffers( [ current_buffer ], current_buffer, ( 6, 5 ) ):
+        ycm.OnFileReadyToParse()
+
+    assert_that(
+      # Positional arguments passed to PostDataToHandlerAsync.
+      post_data_to_handler_async.call_args[ 0 ],
+      contains(
+        has_entries( {
+          'filepath': current_buffer_file,
+          'line_num': 6,
+          'column_num': 6,
+          'file_data': has_entries( {
+            current_buffer_file: has_entries( {
+              'contents': 'current_buffer_contents\n',
+              'filetypes': [ 'some_filetype' ]
+            } )
+          } ),
+          'event_name': 'FileReadyToParse',
+          'tag_files': has_item( PathToTestFile( 'uni¢𐍈d€', 'tags' ) )
+        } ),
+        'event_notification'
+      )
+    )
+
+
 @patch( 'ycm.youcompleteme.YouCompleteMe._AddUltiSnipsDataIfNeeded' )
 @YouCompleteMeInstance()
 def EventNotification_BufferVisit_BuildRequestForCurrentAndUnsavedBuffers_test(
@@ -380,11 +415,9 @@ def EventNotification_BufferVisit_BuildRequestForCurrentAndUnsavedBuffers_test(
 
   with patch( 'ycm.client.base_request.BaseRequest.'
               'PostDataToHandlerAsync' ) as post_data_to_handler_async:
-    with patch( 'vim.buffers', [ current_buffer,
+    with MockVimBuffers( [ current_buffer, modified_buffer, unmodified_buffer ],
-                                 modified_buffer,
+                         current_buffer,
-                                 unmodified_buffer ] ):
+                         ( 3, 5 ) ):
-      with patch( 'vim.current.buffer', current_buffer ):
-        with patch( 'vim.current.window.cursor', ( 3, 5 ) ):
       ycm.OnBufferVisit()
 
     assert_that(
@@ -431,8 +464,7 @@ def EventNotification_BufferUnload_BuildRequestForDeletedAndUnsavedBuffers_test(
 
   with patch( 'ycm.client.base_request.BaseRequest.'
               'PostDataToHandlerAsync' ) as post_data_to_handler_async:
-    with patch( 'vim.buffers', [ current_buffer, deleted_buffer ] ):
+    with MockVimBuffers( [ current_buffer, deleted_buffer ], current_buffer ):
-      with patch( 'vim.current.buffer', current_buffer ):
       ycm.OnBufferUnload( deleted_buffer_file )
 
   assert_that(

python/ycm/tests/test_utils.py

@@ -24,14 +24,16 @@ from future import standard_library
 standard_library.install_aliases()
 from builtins import *  # noqa
 
-from mock import MagicMock
+from mock import MagicMock, patch
 from hamcrest import assert_that, equal_to
+import contextlib
+import functools
+import nose
+import os
 import re
 import sys
-import nose
-import functools
 
-from ycmd.utils import ToUnicode
+from ycmd.utils import GetCurrentDirectory, ToUnicode
 
 
 BUFNR_REGEX = re.compile( '^bufnr\(\'(?P<buffer_filename>.+)\', ([01])\)$' )
@@ -52,21 +54,31 @@ GETBUFVAR_REGEX = re.compile(
 VIM_MOCK = MagicMock()
 
 
-def MockGetBufferNumber( buffer_filename ):
+@contextlib.contextmanager
+def CurrentWorkingDirectory( path ):
+  old_cwd = GetCurrentDirectory()
+  os.chdir( path )
+  try:
+    yield
+  finally:
+    os.chdir( old_cwd )
+
+
+def _MockGetBufferNumber( buffer_filename ):
   for vim_buffer in VIM_MOCK.buffers:
     if vim_buffer.name == buffer_filename:
       return vim_buffer.number
   return -1
 
 
-def MockGetBufferWindowNumber( buffer_number ):
+def _MockGetBufferWindowNumber( buffer_number ):
   for vim_buffer in VIM_MOCK.buffers:
     if vim_buffer.number == buffer_number and vim_buffer.window:
       return vim_buffer.window
   return -1
 
 
-def MockGetBufferVariable( buffer_number, option ):
+def _MockGetBufferVariable( buffer_number, option ):
   for vim_buffer in VIM_MOCK.buffers:
     if vim_buffer.number == buffer_number:
       if option == 'mod':
@@ -77,20 +89,7 @@ def MockGetBufferVariable( buffer_number, option ):
   return ''
 
 
-def MockVimEval( value ):
+def _MockVimBufferEval( value ):
-  if value == 'g:ycm_min_num_of_chars_for_completion':
-    return 0
-
-  if value == 'g:ycm_server_python_interpreter':
-    return ''
-
-  if value == 'tempname()':
-    return '_TEMP_FILE_'
-
-  if value == '&previewheight':
-    # Default value from Vim
-    return 12
-
   if value == '&omnifunc':
     return VIM_MOCK.current.buffer.omnifunc
 
@@ -100,23 +99,66 @@ def MockVimEval( value ):
   match = BUFNR_REGEX.search( value )
   if match:
     buffer_filename = match.group( 'buffer_filename' )
-    return MockGetBufferNumber( buffer_filename )
+    return _MockGetBufferNumber( buffer_filename )
 
   match = BUFWINNR_REGEX.search( value )
   if match:
     buffer_number = int( match.group( 'buffer_number' ) )
-    return MockGetBufferWindowNumber( buffer_number )
+    return _MockGetBufferWindowNumber( buffer_number )
 
   match = GETBUFVAR_REGEX.search( value )
   if match:
     buffer_number = int( match.group( 'buffer_number' ) )
     option = match.group( 'option' )
-    return MockGetBufferVariable( buffer_number, option )
+    return _MockGetBufferVariable( buffer_number, option )
+
+  return None
+
+
+def _MockVimOptionsEval( value ):
+  if value == '&previewheight':
+    return 12
+
+  if value == '&columns':
+    return 80
+
+  if value == '&ruler':
+    return 0
+
+  if value == '&showcmd':
+    return 1
+
+  return None
+
+
+def _MockVimEval( value ):
+  if value == 'g:ycm_min_num_of_chars_for_completion':
+    return 0
+
+  if value == 'g:ycm_server_python_interpreter':
+    return ''
+
+  if value == 'tempname()':
+    return '_TEMP_FILE_'
+
+  if value == 'complete_check()':
+    return 0
+
+  if value == 'tagfiles()':
+    return [ 'tags' ]
+
+  result = _MockVimOptionsEval( value )
+  if result is not None:
+    return result
+
+  result = _MockVimBufferEval( value )
+  if result is not None:
+    return result
 
   raise ValueError( 'Unexpected evaluation: {0}'.format( value ) )
 
 
-def MockWipeoutBuffer( buffer_number ):
+def _MockWipeoutBuffer( buffer_number ):
   buffers = VIM_MOCK.buffers
 
   for index, buffer in enumerate( buffers ):
@@ -127,7 +169,7 @@ def MockWipeoutBuffer( buffer_number ):
 def MockVimCommand( command ):
   match = BWIPEOUT_REGEX.search( command )
   if match:
-    return MockWipeoutBuffer( int( match.group( 1 ) ) )
+    return _MockWipeoutBuffer( int( match.group( 1 ) ) )
 
   raise RuntimeError( 'Unexpected command: ' + command )
 
@@ -159,7 +201,7 @@ class VimBuffer( object ):
 
 
   def __getitem__( self, index ):
-    """Return the bytes for a given line at index |index|."""
+    """Returns the bytes for a given line at index |index|."""
     return self.contents[ index ]
 
 
@@ -172,10 +214,24 @@ class VimBuffer( object ):
 
 
   def GetLines( self ):
-    """Return the contents of the buffer as a list of unicode strings."""
+    """Returns the contents of the buffer as a list of unicode strings."""
     return [ ToUnicode( x ) for x in self.contents ]
 
 
+@contextlib.contextmanager
+def MockVimBuffers( buffers, current_buffer, cursor_position = ( 1, 1 ) ):
+  """Simulates the Vim buffers list |buffers| where |current_buffer| is the
+  buffer displayed in the current window and |cursor_position| is the current
+  cursor position. All buffers are represented by a VimBuffer object."""
+  if current_buffer not in buffers:
+    raise RuntimeError( 'Current buffer must be part of the buffers list.' )
+
+  with patch( 'vim.buffers', buffers ):
+    with patch( 'vim.current.buffer', current_buffer ):
+      with patch( 'vim.current.window.cursor', cursor_position ):
+        yield
+
+
 def MockVimModule():
   """The 'vim' module is something that is only present when running inside the
   Vim Python interpreter, so we replace it with a MagicMock for tests. If you
@@ -199,7 +255,7 @@ def MockVimModule():
   tests."""
 
   VIM_MOCK.buffers = {}
-  VIM_MOCK.eval = MagicMock( side_effect = MockVimEval )
+  VIM_MOCK.eval = MagicMock( side_effect = _MockVimEval )
   sys.modules[ 'vim' ] = VIM_MOCK
 
   return VIM_MOCK

python/ycm/tests/vimsupport_test.py

@@ -25,13 +25,14 @@ from future import standard_library
 standard_library.install_aliases()
 from builtins import *  # noqa
 
-from ycm.tests.test_utils import ( ExtendedMock, MockVimCommand, VimBuffer,
+from ycm.tests import PathToTestFile
-                                   MockVimModule )
+from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
+                                   MockVimCommand, MockVimModule, VimBuffer )
 MockVimModule()
 
 from ycm import vimsupport
 from nose.tools import eq_
-from hamcrest import assert_that, calling, raises, none, has_entry
+from hamcrest import assert_that, calling, equal_to, has_entry, none, raises
 from mock import MagicMock, call, patch
 from ycmd.utils import ToBytes
 import os
@@ -1418,6 +1419,14 @@ def GetUnsavedAndSpecifiedBufferData_EncodedUnicodeCharsInBuffers_test():
                             has_entry( u'contents', u'abc\nfДa\n' ) ) )
 
 
+def GetBufferFilepath_NoBufferName_UnicodeWorkingDirectory_test():
+  vim_buffer = VimBuffer( '', number = 42 )
+  unicode_dir = PathToTestFile( u'uni¢𐍈d€' )
+  with CurrentWorkingDirectory( unicode_dir ):
+    assert_that( vimsupport.GetBufferFilepath( vim_buffer ),
+                 equal_to( os.path.join( unicode_dir, '42' ) ) )
+
+
 # NOTE: Vim returns byte offsets for columns, not actual character columns. This
 # makes 'ДД' have 4 columns: column 0, column 2 and column 4.
 @patch( 'vim.current.line', ToBytes( 'ДДaa' ) )

python/ycm/vimsupport.py

@@ -1,4 +1,5 @@
-# Copyright (C) 2011, 2012  Google Inc.
+# Copyright (C) 2011-2012 Google Inc.
+#               2016      YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -26,11 +27,11 @@ from builtins import *  # noqa
 from future.utils import iterkeys
 import vim
 import os
-import tempfile
 import json
 import re
 from collections import defaultdict
-from ycmd.utils import ToUnicode, ToBytes, JoinLinesAsUnicode
+from ycmd.utils import ( GetCurrentDirectory, JoinLinesAsUnicode, ToBytes,
+                         ToUnicode )
 from ycmd import user_options_store
 
 BUFFER_COMMAND_MAP = { 'same-buffer'      : 'edit',
@@ -156,13 +157,8 @@ def GetBufferFilepath( buffer_object ):
   if buffer_object.name:
     return buffer_object.name
   # Buffers that have just been created by a command like :enew don't have any
-  # buffer name so we use the buffer number for that. Also, os.getcwd() throws
+  # buffer name so we use the buffer number for that.
-  # an exception when the CWD has been deleted so we handle that.
+  return os.path.join( GetCurrentDirectory(), str( buffer_object.number ) )
-  try:
-    folder_path = os.getcwd()
-  except OSError:
-    folder_path = tempfile.gettempdir()
-  return os.path.join( folder_path, str( buffer_object.number ) )
 
 
 def UnplaceSignInBuffer( buffer_number, sign_id ):

python/ycm/youcompleteme.py

@@ -1,4 +1,5 @@
-# Copyright (C) 2011, 2012  Google Inc.
+# Copyright (C) 2011-2012 Google Inc.
+#               2016      YouCompleteMe contributors
 #
 # This file is part of YouCompleteMe.
 #
@@ -225,7 +226,7 @@ class YouCompleteMe( object ):
             self._omnicomp, wrapped_request_data )
         return self._latest_completion_request
 
-    request_data[ 'working_dir' ] = os.getcwd()
+    request_data[ 'working_dir' ] = utils.GetCurrentDirectory()
 
     self._AddExtraConfDataIfNeeded( request_data )
     if force_semantic:
@@ -677,12 +678,8 @@ class YouCompleteMe( object ):
   def _AddTagsFilesIfNeeded( self, extra_data ):
     def GetTagFiles():
       tag_files = vim.eval( 'tagfiles()' )
-      # getcwd() throws an exception when the CWD has been deleted.
+      return [ os.path.join( utils.GetCurrentDirectory(), tag_file )
-      try:
+               for tag_file in tag_files ]
-        current_working_directory = os.getcwd()
-      except OSError:
-        return []
-      return [ os.path.join( current_working_directory, x ) for x in tag_files ]
 
     if not self._user_options[ 'collect_identifiers_from_tags_files' ]:
       return

third_party/ycmd

@@ -1 +1 @@
-Subproject commit f3232ce5180753822d839c80e9b9ea4e33e89d4c
+Subproject commit 63c3d992a2db8d189cd78a25a70c87348726fc52