From 4024c4516a98243c21c3f399ab6ede2d804369ba Mon Sep 17 00:00:00 2001 From: Alexey Stepanov Date: Fri, 4 May 2018 18:21:56 +0200 Subject: [PATCH 1/4] Due to long running tests: use pytest-xdist * modify tests, which can receive race-conditions * remove never documented internal code * add 1 test for coverage bump --- exec_helpers/_ssh_client_base.py | 23 +++---------- exec_helpers/proc_enums.py | 13 +++---- test/test_ssh_client.py | 59 ++++++++++++++++++++++++++++++++ test/test_ssh_client_init.py | 11 +++--- test/test_subprocess_runner.py | 3 +- tox.ini | 3 +- 6 files changed, 77 insertions(+), 35 deletions(-) diff --git a/exec_helpers/_ssh_client_base.py b/exec_helpers/_ssh_client_base.py index 50254f1..3fd47ca 100644 --- a/exec_helpers/_ssh_client_base.py +++ b/exec_helpers/_ssh_client_base.py @@ -191,24 +191,11 @@ def clear_cache(mcs): # type: () -> None mcs.__cache = {} @classmethod - def close_connections( - mcs, - hostname=None # type: typing.Optional[str] - ): # type: (...) -> None - """Close connections for selected or all cached records. - - :type hostname: str - """ - if hostname is None: - keys = [key for key, ssh in mcs.__cache.items() if ssh.is_alive] - else: - keys = [ - (host, port) - for (host, port), ssh - in mcs.__cache.items() if host == hostname and ssh.is_alive] - # raise ValueError(keys) - for key in keys: - mcs.__cache[key].close() + def close_connections(mcs): # type: (...) -> None + """Close connections for selected or all cached records.""" + for ssh in mcs.__cache.values(): + if ssh.is_alive: + ssh.close() class SSHClientBase(six.with_metaclass(_MemorizedSSH, _api.ExecHelper)): diff --git a/exec_helpers/proc_enums.py b/exec_helpers/proc_enums.py index ce0a533..3437407 100644 --- a/exec_helpers/proc_enums.py +++ b/exec_helpers/proc_enums.py @@ -24,7 +24,7 @@ from __future__ import unicode_literals import enum -import typing +import typing # noqa # pylint: disable=unused-import import six @@ -72,7 +72,7 @@ class SigNum(enum.IntEnum): SIGPWR = 30 # Power failure restart (System V). SIGSYS = 31 # Bad system call. - def __str__(self): + def __str__(self): # pragma: no cover """Representation for logs.""" return "{name}<{value:d}(0x{value:02X})>".format( name=self.name, @@ -158,10 +158,7 @@ def __str__(self): ) -_type_exit_codes = typing.Union[int, ExitCodes] - - -def exit_code_to_enum(code): # type: (_type_exit_codes) -> _type_exit_codes +def exit_code_to_enum(code): # type: (typing.Union[int, ExitCodes]) -> typing.Union[int, ExitCodes] """Convert exit code to enum if possible.""" if isinstance(code, int) and code in ExitCodes.__members__.values(): return ExitCodes(code) @@ -169,8 +166,8 @@ def exit_code_to_enum(code): # type: (_type_exit_codes) -> _type_exit_codes def exit_codes_to_enums( - codes=None # type: typing.Optional[typing.Iterable[_type_exit_codes]] -): # type: (...) -> typing.List[_type_exit_codes] + codes=None # type: typing.Optional[typing.Iterable[typing.Union[int, ExitCodes]]] +): # type: (...) -> typing.List[typing.Union[int, ExitCodes]] """Convert integer exit codes to enums.""" if codes is None: return [ExitCodes.EX_OK] diff --git a/test/test_ssh_client.py b/test/test_ssh_client.py index 9af883f..29e5d36 100644 --- a/test/test_ssh_client.py +++ b/test/test_ssh_client.py @@ -1468,6 +1468,65 @@ def test_02_execute_through_host_auth(self, transp, client, policy, logger): mock.call.close() )) + def test_03_execute_through_host_get_pty( + self, transp, client, policy, logger): + target = '127.0.0.2' + exit_code = 0 + + # noinspection PyTypeChecker + return_value = exec_result.ExecResult( + cmd=command, + stderr=stderr_list, + stdout=stdout_list, + exit_code=exit_code + ) + + ( + open_session, + transport, + channel, + get_transport, + open_channel, + intermediate_channel + ) = self.prepare_execute_through_host( + transp=transp, + client=client, + exit_code=exit_code) + + # noinspection PyTypeChecker + ssh = exec_helpers.SSHClient( + host=host, + port=port, + auth=exec_helpers.SSHAuth( + username=username, + password=password + )) + + # noinspection PyTypeChecker + result = ssh.execute_through_host(target, command, get_pty=True) + self.assertEqual(result, return_value) + get_transport.assert_called_once() + open_channel.assert_called_once() + transp.assert_called_once_with(intermediate_channel) + open_session.assert_called_once() + transport.assert_has_calls(( + mock.call.connect( + username=username, password=password, pkey=None, + ), + mock.call.open_session() + )) + + channel.assert_has_calls(( + mock.call.get_pty(term='vt100', width=80, height=24, width_pixels=0, height_pixels=0), + mock.call.makefile('rb'), + mock.call.makefile_stderr('rb'), + mock.call.exec_command(command), + mock.call.recv_ready(), + mock.call.recv_stderr_ready(), + mock.call.status_event.is_set(), + mock.call.close() + )) + @mock.patch('exec_helpers._ssh_client_base.logger', autospec=True) @mock.patch('paramiko.AutoAddPolicy', autospec=True, return_value='AutoAddPolicy') diff --git a/test/test_ssh_client_init.py b/test/test_ssh_client_init.py index 0c77e48..8117eff 100644 --- a/test/test_ssh_client_init.py +++ b/test/test_ssh_client_init.py @@ -708,13 +708,10 @@ def test_023_init_memorize( no_call.assert_not_called() # Mock returns false-connected state, so we just count close calls - client.assert_has_calls(( - mock.call().get_transport(), - mock.call().get_transport(), - mock.call().get_transport(), - mock.call().close(), - mock.call().close(), - mock.call().close(), + client().close.assert_has_calls(( + mock.call(), + mock.call(), + mock.call(), )) # change creds diff --git a/test/test_subprocess_runner.py b/test/test_subprocess_runner.py index e241e7d..c29a1f5 100644 --- a/test/test_subprocess_runner.py +++ b/test/test_subprocess_runner.py @@ -206,6 +206,7 @@ def test_003_context_manager( with mock.patch('threading.RLock', autospec=True): with exec_helpers.Subprocess() as runner: + runner.lock.acquire.assert_called() self.assertEqual( mock.call.acquire(), runner.lock.mock_calls[0] ) @@ -215,7 +216,7 @@ def test_003_context_manager( ) - self.assertEqual(mock.call.release(), runner.lock.mock_calls[-1]) + runner.lock.release.assert_called() subprocess_runner.SingletonMeta._instances.clear() diff --git a/tox.ini b/tox.ini index dd4f1fb..0c666bd 100644 --- a/tox.ini +++ b/tox.ini @@ -18,6 +18,7 @@ deps = sphinx pytest pytest-cov + pytest-xdist pytest-html pytest-sugar py{27,34,35,36}-nocov: Cython @@ -25,7 +26,7 @@ deps = py{27,py}: mock commands = - py.test -vv --junitxml=unit_result.xml --html=report.html --cov-config .coveragerc --cov-report html --cov=exec_helpers {posargs:test} + py.test -n auto -vv --junitxml=unit_result.xml --html=report.html --cov-config .coveragerc --cov-report html --cov=exec_helpers {posargs:test} coverage report --fail-under 97 [testenv:py34-nocov] From ec01ca08284489f7ae37a8c32119146c25a1ca47 Mon Sep 17 00:00:00 2001 From: Alexey Stepanov Date: Fri, 4 May 2018 18:27:17 +0200 Subject: [PATCH 2/4] Also use xdist for nocov and deployment tests --- tools/build-wheels.sh | 4 ++-- tox.ini | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/build-wheels.sh b/tools/build-wheels.sh index 53a19a5..715ed1a 100755 --- a/tools/build-wheels.sh +++ b/tools/build-wheels.sh @@ -52,8 +52,8 @@ for PYTHON in ${PYTHON_VERSIONS}; do echo -n "Test $PYTHON: $package_name " /opt/python/${PYTHON}/bin/python -c "import platform;print(platform.platform())" /opt/python/${PYTHON}/bin/pip install "$package_name" --no-index -f file:///io/dist - /opt/python/${PYTHON}/bin/pip install pytest - /opt/python/${PYTHON}/bin/py.test -vv /io/test + /opt/python/${PYTHON}/bin/pip install pytest pytest-xdist + /opt/python/${PYTHON}/bin/py.test -vv -n auto /io/test done find /io/dist/ -type f -not -name "*$package_name*" -delete diff --git a/tox.ini b/tox.ini index 0c666bd..5b55856 100644 --- a/tox.ini +++ b/tox.ini @@ -34,21 +34,21 @@ usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv {posargs:test} + py.test -vv -n auto {posargs:test} [testenv:py35-nocov] usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv {posargs:test} + py.test -vv -n auto {posargs:test} [testenv:py36-nocov] usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv {posargs:test} + py.test -vv -n auto {posargs:test} [testenv:venv] commands = {posargs:} From 5fabb317732c0819d9314134c82c6335d75cf9cf Mon Sep 17 00:00:00 2001 From: Alexey Stepanov Date: Mon, 7 May 2018 10:06:48 +0200 Subject: [PATCH 3/4] Increase timeout for tests, do not reset stop event for reading 100 ms timeout & 100 ms cycle can cause false negative If stop set - all data is already read --- exec_helpers/_ssh_client_base.py | 1 - exec_helpers/subprocess_runner.py | 1 - test/test_ssh_client.py | 4 ++-- test/test_subprocess_runner.py | 4 ++-- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/exec_helpers/_ssh_client_base.py b/exec_helpers/_ssh_client_base.py index 3fd47ca..8ac7d88 100644 --- a/exec_helpers/_ssh_client_base.py +++ b/exec_helpers/_ssh_client_base.py @@ -729,7 +729,6 @@ def poll_pipes(stop, ): # type: (threading.Event) -> None # Process closed? if stop_event.is_set(): - stop_event.clear() interface.close() return result diff --git a/exec_helpers/subprocess_runner.py b/exec_helpers/subprocess_runner.py index 38c364f..57625df 100644 --- a/exec_helpers/subprocess_runner.py +++ b/exec_helpers/subprocess_runner.py @@ -249,7 +249,6 @@ def poll_pipes(stop, ): # type: (threading.Event) -> None # Process closed? if stop_event.is_set(): - stop_event.clear() return result # Kill not ended process and wait for close try: diff --git a/test/test_ssh_client.py b/test/test_ssh_client.py index 29e5d36..3a3c4a3 100644 --- a/test/test_ssh_client.py +++ b/test/test_ssh_client.py @@ -994,7 +994,7 @@ def test_024_execute_timeout( logger.reset_mock() # noinspection PyTypeChecker - result = ssh.execute(command=command, verbose=False, timeout=0.1) + result = ssh.execute(command=command, verbose=False, timeout=0.2) self.assertEqual( result, @@ -1030,7 +1030,7 @@ def test_025_execute_timeout_fail( with self.assertRaises(exec_helpers.ExecHelperTimeoutError): # noinspection PyTypeChecker - ssh.execute(command=command, verbose=False, timeout=0.1) + ssh.execute(command=command, verbose=False, timeout=0.2) execute_async.assert_called_once_with(command, verbose=False) chan.assert_has_calls((mock.call.status_event.is_set(), )) diff --git a/test/test_subprocess_runner.py b/test/test_subprocess_runner.py index c29a1f5..ef3449a 100644 --- a/test/test_subprocess_runner.py +++ b/test/test_subprocess_runner.py @@ -236,7 +236,7 @@ def test_004_execute_timeout_fail( with self.assertRaises(exec_helpers.ExecHelperTimeoutError): # noinspection PyTypeChecker - runner.execute(command, timeout=1) + runner.execute(command, timeout=0.2) popen.assert_has_calls(( mock.call( @@ -831,7 +831,7 @@ def test_013_execute_timeout_done( # noinspection PyTypeChecker - res = runner.execute(command, timeout=0.1) + res = runner.execute(command, timeout=0.2) self.assertEqual(res, exp_result) From 8e6677762c2057755ae6d1087be508e69d84d9fa Mon Sep 17 00:00:00 2001 From: Alexey Stepanov Date: Mon, 7 May 2018 10:29:20 +0200 Subject: [PATCH 4/4] no xdist use --- tools/build-wheels.sh | 4 ++-- tox.ini | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/build-wheels.sh b/tools/build-wheels.sh index 715ed1a..53a19a5 100755 --- a/tools/build-wheels.sh +++ b/tools/build-wheels.sh @@ -52,8 +52,8 @@ for PYTHON in ${PYTHON_VERSIONS}; do echo -n "Test $PYTHON: $package_name " /opt/python/${PYTHON}/bin/python -c "import platform;print(platform.platform())" /opt/python/${PYTHON}/bin/pip install "$package_name" --no-index -f file:///io/dist - /opt/python/${PYTHON}/bin/pip install pytest pytest-xdist - /opt/python/${PYTHON}/bin/py.test -vv -n auto /io/test + /opt/python/${PYTHON}/bin/pip install pytest + /opt/python/${PYTHON}/bin/py.test -vv /io/test done find /io/dist/ -type f -not -name "*$package_name*" -delete diff --git a/tox.ini b/tox.ini index 5b55856..dd4f1fb 100644 --- a/tox.ini +++ b/tox.ini @@ -18,7 +18,6 @@ deps = sphinx pytest pytest-cov - pytest-xdist pytest-html pytest-sugar py{27,34,35,36}-nocov: Cython @@ -26,7 +25,7 @@ deps = py{27,py}: mock commands = - py.test -n auto -vv --junitxml=unit_result.xml --html=report.html --cov-config .coveragerc --cov-report html --cov=exec_helpers {posargs:test} + py.test -vv --junitxml=unit_result.xml --html=report.html --cov-config .coveragerc --cov-report html --cov=exec_helpers {posargs:test} coverage report --fail-under 97 [testenv:py34-nocov] @@ -34,21 +33,21 @@ usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv -n auto {posargs:test} + py.test -vv {posargs:test} [testenv:py35-nocov] usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv -n auto {posargs:test} + py.test -vv {posargs:test} [testenv:py36-nocov] usedevelop = False commands = python setup.py bdist_wheel pip install exec_helpers --no-index -f dist - py.test -vv -n auto {posargs:test} + py.test -vv {posargs:test} [testenv:venv] commands = {posargs:}