From cff90e587ac68d1a4094c4e41bc71e8eb0eec48b Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Wed, 27 Jan 2021 16:21:24 -0500 Subject: [PATCH] builder/utils/execute_cmd: remove useless return value and add a test Since we never pass in subprocess.PIPE for stdin/stdout/stderr, subprocess.communicate() does nothing, and the return out/err tuple was always empty. --- module_build_service/builder/utils.py | 3 +-- tests/test_builder/test_builder_utils.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/module_build_service/builder/utils.py b/module_build_service/builder/utils.py index cdffac60..35afac14 100644 --- a/module_build_service/builder/utils.py +++ b/module_build_service/builder/utils.py @@ -44,12 +44,11 @@ def execute_cmd(args, output=None, cwd=None): log.info("Executing the command \"%s\"%s" % (" ".join(args), out_log_msg)) proc = subprocess.Popen(args, stdout=output, stderr=output, cwd=cwd) - out, err = proc.communicate() + proc.wait() if proc.returncode != 0: err_msg = "Command '%s' returned non-zero value %d%s" % (args, proc.returncode, out_log_msg) raise RuntimeError(err_msg) - return out, err def get_koji_config(mbs_config): diff --git a/tests/test_builder/test_builder_utils.py b/tests/test_builder/test_builder_utils.py index 2af324d3..5ea337ea 100644 --- a/tests/test_builder/test_builder_utils.py +++ b/tests/test_builder/test_builder_utils.py @@ -17,6 +17,22 @@ from module_build_service.scheduler.db_session import db_session from tests import read_staged_data, scheduler_init_data +def test_execute_cmd(tmpdir, caplog): + logfile = str(tmpdir / "out.log") + with open(logfile, "w") as f: + utils.execute_cmd(["echo", "hello"], output=f) + + with open(logfile) as f: + assert f.read() == "hello\n" + + assert 'Executing the command "echo hello", output log: %s' % logfile in caplog.text + + +def test_execute_cmd_fail(): + with pytest.raises(RuntimeError): + utils.execute_cmd(["false"]) + + @pytest.mark.parametrize("variation", ("none", "empty", "already_downloaded")) @patch("requests.get") @patch("koji.ClientSession")