Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes dcos_cli.exec return the full process object. Marks exec_command as deprecated #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions dcos_test_utils/dcos_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import tempfile
from typing import Optional

import deprecation
import requests

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -92,6 +93,67 @@ def clear_cli_dir():
if os.path.exists(path):
shutil.rmtree(path)

def exec(self, cmd: str, stdin=None, check=True) -> subprocess.CompletedProcess:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have check?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While testing I would like to be able to run commands that I know will fail, and assert on their exit, without having to rescue from the exception. There is a behavior on the subprocess library that is to pass a boolean called check that when True, does not check for the command terminating with status code 0. I want to be able to configure this from the client of the exec function.

"""Execute CLI command and processes result.

This method expects that process won't block.

:param cmd: Program and arguments
:type cmd: str
:param stdin: File to use for stdin
:param check: Does it check for raised errors
:type stdin: typing.optional[File]
:returns: A tuple with stdout and stderr
:rtype: subprocess.CompletedProcess
Fabs marked this conversation as resolved.
Show resolved Hide resolved

:raises subprocess.CalledProcessError: When check=True if the returncode of \
Fabs marked this conversation as resolved.
Show resolved Hide resolved
cmd is not 0
exception description.
"""

log.info('CMD: {!r}'.format(cmd))

# Borrowed from dcos-e2e
# https://github.com/dcos/dcos-e2e/blob/8d4916780ade8caf41dae376fdf47f4253eb52c7/src/dcos_e2e/_common.py#L46-L59
def safe_decode(output_bytes: bytes) -> str:
"""
Decode a bytestring to Unicode with a safe fallback.
"""
try:
return output_bytes.decode(
encoding='utf-8',
errors='strict')
except UnicodeDecodeError:
return output_bytes.decode(
encoding='ascii',
errors='backslashreplace')

try:
process = subprocess.run(
cmd,
stdin=stdin,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
env=self.env,
check=check)
except subprocess.CalledProcessError as e:
stderr = safe_decode(e.stderr)
log.error('STDERR: {}'.format(stderr))

stdout = safe_decode(e.stdout)
log.error('STDOUT: {}'.format(stdout))

raise

stdout = safe_decode(process.stdout)
log.info('STDOUT: {}'.format(stdout))

stderr = safe_decode(process.stdout)
log.info('STDERR: {}'.format(stderr))

return process

@deprecation.deprecated(details="Deprecated in favor of the `exec` function. DCOS-44823")
def exec_command(self, cmd: str, stdin=None) -> tuple:
"""Execute CLI command and processes result.

Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ pytest
requests
retrying
tox
deprecation

sphinx
41 changes: 40 additions & 1 deletion tests/test_dcos_cli.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import pytest
import subprocess

import deprecation
import pytest
from dcos_test_utils import dcos_cli


@deprecation.deprecated(details="Deprecated in favor of the `exec` function. DCOS-44823")
def test_exec_command(caplog):
cli = dcos_cli.DcosCli('')
stdout, stderr = cli.exec_command(
Expand All @@ -16,9 +18,46 @@ def test_exec_command(caplog):
assert any(rec.message.startswith('STDERR:') for rec in caplog.records)


@deprecation.deprecated(details="Deprecated in favor of the `exec` function. DCOS-44823")
def test_exec_command_fail(caplog):
cli = dcos_cli.DcosCli('')
with pytest.raises(subprocess.CalledProcessError):
cli.exec_command(['/bin/sh', '-c', 'does-not-exist'])
assert any(rec.message.startswith('CMD:') for rec in caplog.records)
assert any(rec.message.startswith('STDERR:') for rec in caplog.records)


def test_exec(caplog):
cli = dcos_cli.DcosCli('')
process = cli.exec(
['/bin/sh', '-c', 'echo "hello, world!"']
)
assert process.stdout == b'hello, world!\n'
assert process.stderr == b''
assert process.returncode == 0
assert any(rec.message.startswith('CMD:') for rec in caplog.records)
assert any(rec.message.startswith('STDOUT:') for rec in caplog.records)
assert any(rec.message.startswith('STDERR:') for rec in caplog.records)


def test_exec_fail(caplog):
cli = dcos_cli.DcosCli('')
with pytest.raises(subprocess.CalledProcessError):
cli.exec(['/bin/sh', '-c', 'does-not-exist'])
assert any(rec.message.startswith('CMD:') for rec in caplog.records)
assert any(rec.message.startswith('STDERR:') for rec in caplog.records)
assert any(rec.message.startswith('STDOUT:') for rec in caplog.records)


def test_exec_fail_without_check(caplog):
cli = dcos_cli.DcosCli('')
process = cli.exec(
['/bin/sh', '-c', 'does-not-exist'],
check=False
)
assert process.stdout == b''
assert b'does-not-exist' in process.stderr
assert process.returncode == 127
assert any(rec.message.startswith('CMD:') for rec in caplog.records)
assert any(rec.message.startswith('STDOUT:') for rec in caplog.records)
assert any(rec.message.startswith('STDERR:') for rec in caplog.records)