diff --git a/pungi/ostree/tree.py b/pungi/ostree/tree.py index e0e9f5ff..1ba138b3 100644 --- a/pungi/ostree/tree.py +++ b/pungi/ostree/tree.py @@ -16,7 +16,6 @@ import os import json -import subprocess from kobo import shortcuts from pungi.util import makedirs @@ -60,19 +59,13 @@ class Tree(OSTree): # permissions. See https://pagure.io/releng/issue/8811#comment-629051 oldumask = os.umask(0o0002) try: - with open(log_file, "w") as f: - f.write(f"COMMAND: {' '.join(cmd)}\n") - f.flush() - proc = subprocess.Popen( - cmd, - shell=True, - stdout=f, - stderr=subprocess.STDOUT, - universal_newlines=True, - ) - ret_code = proc.wait() - if ret_code: - raise RuntimeError(f"Failed to run rpm-ostree, log_file: {log_file}") + shortcuts.run( + cmd, + show_cmd=True, + stdout=True, + logfile=log_file, + universal_newlines=True, + ) finally: os.umask(oldumask) diff --git a/tests/test_ostree_script.py b/tests/test_ostree_script.py index af07021b..6276f729 100644 --- a/tests/test_ostree_script.py +++ b/tests/test_ostree_script.py @@ -3,7 +3,6 @@ import json import os -import subprocess import yaml from unittest import mock @@ -48,9 +47,9 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): "[fedora-23]\nbaseurl=why-not-zoidberg?", ) - def assertCorrectPopenCall(self, mock_popen, extra_calls=[], extra_args=[]): + def assertCorrectCall(self, mock_run, extra_calls=[], extra_args=[]): self.assertCountEqual( - mock_popen.call_args_list, + mock_run.call_args_list, [ mock.call( [ @@ -65,26 +64,17 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] + extra_args + [self.topdir + "/fedora-atomic-docker-host.json"], - shell=True, - stdout=mock.ANY, - stderr=subprocess.STDOUT, + logfile=self.topdir + "/logs/Atomic/create-ostree-repo.log", + show_cmd=True, + stdout=True, universal_newlines=True, ) ] + extra_calls, ) - def test_binary_output(self): - os.environ.update({"PATH": f"{self.topdir}:{os.environ['PATH']}"}) - fake_script = os.path.join(self.topdir, "rpm-ostree") - with open(fake_script, "w") as f: - f.write("#!/bin/sh\n") - f.write("cat /bin/sh\n") - f.write("exit 0\n") - os.chmod(fake_script, 0o744) - - # It should not crash when rpm-ostree outputs binary data. - # https://pagure.io/releng/issue/12474 + @mock.patch("kobo.shortcuts.run") + def test_full_run(self, run): ostree.main( [ "tree", @@ -94,23 +84,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_full_run(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 - ostree.main( - [ - "tree", - "--repo=%s" % self.repo, - "--log-dir=%s" % os.path.join(self.topdir, "logs", "Atomic"), - "--treefile=%s/fedora-atomic-docker-host.json" % self.topdir, - ] - ) + self.assertCorrectCall(run) - self.assertCorrectPopenCall(mock_popen) - - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_run_on_existing_empty_dir(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_run_on_existing_empty_dir(self, run): os.mkdir(self.repo) ostree.main( @@ -122,11 +99,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall(mock_popen) + self.assertCorrectCall(run) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_run_on_initialized_repo(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_run_on_initialized_repo(self, run): helpers.touch(os.path.join(self.repo, "initialized")) ostree.main( @@ -138,12 +114,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall(mock_popen) + self.assertCorrectCall(run) - @mock.patch("pungi.ostree.tree.subprocess.Popen") @mock.patch("kobo.shortcuts.run") - def test_update_summary(self, mock_run, mock_popen): - mock_popen.return_value.wait.return_value = 0 + def test_update_summary(self, run): ostree.main( [ "tree", @@ -154,12 +128,9 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall( - mock_popen, - ) - self.assertCountEqual( - mock_run.call_args_list, - [ + self.assertCorrectCall( + run, + extra_calls=[ mock.call( ["ostree", "summary", "-u", "--repo=%s" % self.repo], logfile=self.topdir + "/logs/Atomic/ostree-summary.log", @@ -170,10 +141,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ], ) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_versioning_metadata(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 - + @mock.patch("kobo.shortcuts.run") + def test_versioning_metadata(self, run): ostree.main( [ "tree", @@ -184,13 +153,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall( - mock_popen, extra_args=["--add-metadata-string=version=24"] - ) + self.assertCorrectCall(run, extra_args=["--add-metadata-string=version=24"]) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_ostree_ref(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_ostree_ref(self, run): self._make_dummy_config_dir(self.topdir) treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.json") @@ -220,9 +186,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): # repos should stay unchanged self.assertEqual(original_repos, new_repos) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_run_with_yaml_file(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_run_with_yaml_file(self, run): self._make_dummy_config_dir(self.topdir) treefile = os.path.join(self.topdir, "fedora-atomic-docker-host.yaml") @@ -254,9 +219,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): # repos should stay unchanged self.assertEqual(original_repos, new_repos) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_force_new_commit(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_force_new_commit(self, run): helpers.touch(os.path.join(self.repo, "initialized")) ostree.main( @@ -269,11 +233,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall(mock_popen, extra_args=["--force-nocache"]) + self.assertCorrectCall(run, extra_args=["--force-nocache"]) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_unified_core(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_unified_core(self, run): helpers.touch(os.path.join(self.repo, "initialized")) ostree.main( @@ -286,11 +249,10 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): ] ) - self.assertCorrectPopenCall(mock_popen, extra_args=["--unified-core"]) + self.assertCorrectCall(run, extra_args=["--unified-core"]) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_extra_config_with_extra_repos(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_extra_config_with_extra_repos(self, run): configdir = os.path.join(self.topdir, "config") self._make_dummy_config_dir(configdir) treefile = os.path.join(configdir, "fedora-atomic-docker-host.json") @@ -348,9 +310,8 @@ class OstreeTreeScriptTest(helpers.PungiTestCase): for name in ("repo-0", "repo-1", "repo-2"): self.assertIn(name, repos) - @mock.patch("pungi.ostree.tree.subprocess.Popen") - def test_extra_config_with_keep_original_sources(self, mock_popen): - mock_popen.return_value.wait.return_value = 0 + @mock.patch("kobo.shortcuts.run") + def test_extra_config_with_keep_original_sources(self, run): configdir = os.path.join(self.topdir, "config") self._make_dummy_config_dir(configdir) treefile = os.path.join(configdir, "fedora-atomic-docker-host.json")