oracle / opengrok

OpenGrok is a fast and usable source code search and cross reference engine, written in Java
http://oracle.github.io/opengrok/
Other
4.35k stars 744 forks source link

test_dosync_check_config_empty sometimes fails on Windows #4430

Closed vladak closed 11 months ago

vladak commented 12 months ago

The test_dosync_check_config_empty test sometimes fails on Windows due to PermissionError when the pool processes are being terminated:

___________________ test_dosync_check_config_empty[False-1] ___________________

check_config = False, expected_times = 1

    @pytest.mark.parametrize(['check_config', 'expected_times'], [(True, 0), (False, 1)])
    def test_dosync_check_config_empty(check_config, expected_times):
        commands = []
        with patch(pool.Pool.map, lambda x, y, z: []):
>           assert do_sync(logging.INFO, commands, None, ["foo", "bar"], [],
                           "http://localhost:8080/source", 42, check_config=check_config) == SUCCESS_EXITVAL

src\test\python\test_sync.py:42: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
D:\a\opengrok\opengrok\tools\target\env\lib\site-packages\opengrok_tools-1.12.18-py3.9.egg\opengrok_tools\sync.py:129: in do_sync
    cmds.print_outputs(logger, lines=True)
C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\pool.py:736: in __exit__
    self.terminate()
C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\pool.py:654: in terminate
    self._terminate()
C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\util.py:224: in __call__
    res = self._callback(*self._args, **self._kwargs)
C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\pool.py:713: in _terminate_pool
    p.terminate()
C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\process.py:133: in terminate
    self._popen.terminate()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <multiprocessing.popen_spawn_win32.Popen object at 0x000001B9A5C7F250>

    def terminate(self):
        if self.returncode is None:
            try:
>               _winapi.TerminateProcess(int(self._handle), TERMINATE)
E               PermissionError: [WinError 5] Access is denied

C:\hostedtoolcache\windows\Python\3.9.13\x64\lib\multiprocessing\popen_spawn_win32.py:123: PermissionError
vladak commented 12 months ago

The test is kind of special: it patches the Pool.map() function to be a no-op and then calls do_sync() with worker count of 42 and empty command set. The Pool.map() is called with a context manager, so that when it is being closed, the terminate() will be called for the workers and this is where it fails on Windows.

Instead of patching map(), it would probably make better sense just to spy on it and give the pool some legitimate commands to process.

vladak commented 12 months ago

Also, if the commands parameter of do_sync() is empty, the function should just return. Maybe this is what is leading to the failure.

vladak commented 12 months ago

Looking at the do_sync() code, the check_config parameter should be better explained in the do_sync() code - the configuration check is being done in CommandSequence initialization. If there is any problem with the configuration, CommandConfigurationException will be raised so if the init function returns to do_sync(), then the configuration check must have passed.