Auto merge of #282 - micbou:shutdown-tests, r=puremourning

[READY] Properly shut down on Windows

### Problem

When killing a process on Windows, it is terminated like it received a `SIGKILL` signal on Linux. This means that @atexit functions will not be called. In our case, `ServerCleanup` is not executed:
completers `Shutdown` methods are not called, neither the `Shutdown` function in the global extra conf. In particular, subservers like `OmniSharpServer` and `Tern.js` are left running.
This issue happens in two situations:
 - [a client like YCM terminates the ycmd server](https://github.com/Valloric/YouCompleteMe/blob/master/python/ycm/youcompleteme.py#L175);
 - [watchdog plugin pulls the trigger](https://github.com/Valloric/ycmd/blob/master/ycmd/watchdog_plugin.py#L92).

How to solve it?

### Solution

First situation should be solved by adding a new handler `/shutdown` to properly shut down the ycmd server from a client. Unfortunately, `waitress` and `bottle` don't provide an easy way to shut it down. Using `sys.exit()` in the `/shutdown` handler will not work because it is run in a thread. The solution is to use the same mechanism as in the watchdog plugin. After cleaning up, we start a thread to immediately kill the server.
Second situation is fixed by using the function associated to the `/shutdown` handler in the watchdog plugin.

### Tests

To test that the shutdown handler and the watchdog plugin are properly working, we need to start the ycmd server for real. We cannot use [webtest](https://webtest.readthedocs.org/en/latest/) here. I added 4 tests, which are a combination of shutting down ycmd without and with a subserver, from a request and from the watchdog plugin.
[psutil](http://pythonhosted.org/psutil/) module was added to the test requirements to work with processes in a portable way.
In order to test the watchdog plugin, I added a command-line argument to set the `check_interval_seconds` variable.
Since we are starting subprocesses in those tests, some changes were needed to enable coverage in Python subprocesses. Another change to improve coverage was to update Waitress to version 0.8.10. See the commit message for some explanation.
A benefit of those tests is a significant increase in coverage (~6%).

###  Caveats

- Tests may introduce flakyness. Robustness should be improved;
- Coverage seems to cause issue with the Gocode daemon. Not sure if it is related to this PR. Need to investigate.

### Next

- Update YCM to use the `shutdown` handler (fix issue ycm-core/YouCompleteMe#876);
- Fix logfiles clean up on Windows.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/282)
<!-- Reviewable:end -->

.coveragerc

@@ -1,4 +1,14 @@
 [report]
 omit =
+    */third_party/requests/*
     */tests/*
     */__init__.py
+
+[run]
+parallel = True
+source =
+    ycmd
+
+[paths]
+source =
+    ycmd/

.gitignore

@@ -31,6 +31,7 @@ pip-log.txt
 
 # Unit test / coverage reports
 .coverage
+.coverage.*
 cover/
 .tox
 nosetests.xml

ci/travis/travis_install.sh

@@ -57,6 +57,11 @@ pip install -U pip wheel setuptools
 pip install -r test_requirements.txt
 npm install -g typescript
 
+# Enable coverage for Python subprocesses. See:
+# http://coverage.readthedocs.org/en/coverage-4.0.3/subprocess.html
+echo -e "import coverage\ncoverage.process_startup()" > \
+  ${PYENV_ROOT}/versions/${PYENV_VERSION}/lib/python${YCMD_PYTHON_VERSION}/site-packages/sitecustomize.py
+
 ############
 # rust setup
 ############

examples/example_client.py

@@ -113,10 +113,10 @@ def IsReady( self, include_subservers = False ):
 
   def Shutdown( self ):
     if self.IsAlive():
-      self._popen_handle.terminate()
+      self.PostToHandlerAndLog( 'shutdown' )
 
 
-  def PostToHandlerAndLog( self, handler, data ):
+  def PostToHandlerAndLog( self, handler, data = None ):
     self._CallHttpie( 'post', handler, data )
 
 

run_tests.py

@@ -192,7 +192,9 @@ def NoseTests( parsed_args, extra_nosetests_args ):
       nosetests_args.extend( COMPLETERS[ key ][ 'test' ] )
 
   if parsed_args.coverage:
-    nosetests_args += [ '--with-coverage', '--cover-package=ycmd',
+    nosetests_args += [ '--with-coverage',
+                        '--cover-erase',
+                        '--cover-package=ycmd',
                         '--cover-html' ]
 
   if extra_nosetests_args:

test_requirements.txt

@@ -6,6 +6,7 @@ WebTest>=2.0.20
 ordereddict>=1.1
 nose-exclude>=0.4.1
 unittest2>=1.1.0
+psutil>=3.3.0
 # This needs to be kept in sync with submodule checkout in third_party
 future==0.15.2
 codecov>=2.0.5

ycmd/__main__.py

@@ -36,16 +36,14 @@
 import logging
 import json
 import argparse
-import waitress
 import signal
 import os
 import base64
-from ycmd import user_options_store
-from ycmd import extra_conf_store
-from ycmd import utils
-from ycmd.watchdog_plugin import WatchdogPlugin
+
+from ycmd import extra_conf_store, user_options_store, utils
 from ycmd.hmac_plugin import HmacPlugin
 from ycmd.utils import ToBytes, ReadFile, OpenForStdHandle
+from ycmd.wsgi_server import StoppableWSGIServer
 
 
 def YcmCoreSanityCheck():
@@ -104,6 +102,9 @@ def ParseArguments():
                               '[debug|info|warning|error|critical]' )
   parser.add_argument( '--idle_suicide_seconds', type = int, default = 0,
                        help = 'num idle seconds before server shuts down')
+  parser.add_argument( '--check_interval_seconds', type = int, default = 600,
+                       help = 'interval in seconds to check server '
+                              'inactivity' )
   parser.add_argument( '--options_file', type = str, required = True,
                        help = 'file with user options, in JSON format' )
   parser.add_argument( '--stdout', type = str, default = None,
@@ -163,20 +164,23 @@ def Main():
 
   PossiblyDetachFromTerminal()
 
-  # This can't be a top-level import because it transitively imports
+  # These can't be top-level imports because they transitively import
   # ycm_core which we want to be imported ONLY after extra conf
   # preload has executed.
   from ycmd import handlers
+  from ycmd.watchdog_plugin import WatchdogPlugin
   handlers.UpdateUserOptions( options )
   handlers.SetHmacSecret( hmac_secret )
   SetUpSignalHandler( args.stdout, args.stderr, args.keep_logfiles )
-  handlers.app.install( WatchdogPlugin( args.idle_suicide_seconds ) )
+  handlers.app.install( WatchdogPlugin( args.idle_suicide_seconds,
+                                        args.check_interval_seconds ) )
   handlers.app.install( HmacPlugin( hmac_secret ) )
   CloseStdin()
-  waitress.serve( handlers.app,
-                  host = args.host,
-                  port = args.port,
-                  threads = 30 )
+  handlers.wsgi_server = StoppableWSGIServer( handlers.app,
+                                              host = args.host,
+                                              port = args.port,
+                                              threads = 30 )
+  handlers.wsgi_server.Run()
 
 
 if __name__ == "__main__":

ycmd/handlers.py

@@ -29,6 +29,7 @@
 import logging
 import traceback
 from bottle import request
+from threading import Thread
 
 import ycm_core
 from ycmd import extra_conf_store, hmac_plugin, server_state, user_options_store
@@ -46,6 +47,7 @@
 _hmac_secret = bytes()
 _logger = logging.getLogger( __name__ )
 app = bottle.Bottle()
+wsgi_server = None
 
 
 @app.post( '/event_notification' )
@@ -223,6 +225,14 @@ def DebugInfo():
   return _JsonResponse( '\n'.join( output ) )
 
 
+@app.post( '/shutdown' )
+def Shutdown():
+  _logger.info( 'Received shutdown request' )
+  ServerShutdown()
+
+  return _JsonResponse( True )
+
+
 # The type of the param is Bottle.HTTPError
 def ErrorHandler( httperror ):
   body = _JsonResponse( BuildExceptionResponse( httperror.exception,
@@ -259,9 +269,20 @@ def _GetCompleterForRequestData( request_data ):
     return _server_state.GetFiletypeCompleter( [ completer_target ] )
 
 
-@atexit.register
 def ServerShutdown():
-  _logger.info( 'Server shutting down' )
+  def Terminator():
+    if wsgi_server:
+      wsgi_server.Shutdown()
+
+  # Use a separate thread to let the server send the response before shutting
+  # down.
+  terminator = Thread( target = Terminator )
+  terminator.daemon = True
+  terminator.start()
+
+
+@atexit.register
+def ServerCleanup():
   if _server_state:
     _server_state.Shutdown()
     extra_conf_store.Shutdown()