From 1efd4a98731eb27b6ace3c0bfca8f5290ffe67e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 10 Mar 2016 15:04:52 +0100 Subject: [PATCH] [init] Remove duplicated checks for comps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check once before iterating through the variants. This greatly simplifies the tests as each function now has one less code path. Signed-off-by: Lubomír Sedlář --- pungi/phases/init.py | 46 +++++++++--------------- tests/test_initphase.py | 78 ++++++++++++----------------------------- 2 files changed, 38 insertions(+), 86 deletions(-) diff --git a/pungi/phases/init.py b/pungi/phases/init.py index 26f56706..0fe40745 100644 --- a/pungi/phases/init.py +++ b/pungi/phases/init.py @@ -159,23 +159,24 @@ class InitPhase(PhaseBase): return False def run(self): - # write global comps and arch comps - write_global_comps(self.compose) - for arch in self.compose.get_arches(): - write_arch_comps(self.compose, arch) + if self.compose.has_comps: + # write global comps and arch comps + write_global_comps(self.compose) + for arch in self.compose.get_arches(): + write_arch_comps(self.compose, arch) - # create comps repos - for arch in self.compose.get_arches(): - create_comps_repo(self.compose, arch) + # create comps repos + for arch in self.compose.get_arches(): + create_comps_repo(self.compose, arch) - # write variant comps - for variant in self.compose.get_variants(): - should_preserve = variant.uid in self.compose.conf.get('keep_original_comps', []) - for arch in variant.arches: - if should_preserve: - copy_variant_comps(self.compose, arch, variant) - else: - write_variant_comps(self.compose, arch, variant) + # write variant comps + for variant in self.compose.get_variants(): + should_preserve = variant.uid in self.compose.conf.get('keep_original_comps', []) + for arch in variant.arches: + if should_preserve: + copy_variant_comps(self.compose, arch, variant) + else: + write_variant_comps(self.compose, arch, variant) # download variants.xml / product.xml? @@ -184,9 +185,6 @@ class InitPhase(PhaseBase): def write_global_comps(compose): - if not compose.has_comps: - return - comps_file_global = compose.paths.work.comps(arch="global") msg = "Writing global comps file: %s" % comps_file_global @@ -210,9 +208,6 @@ def write_global_comps(compose): def write_arch_comps(compose, arch): - if not compose.has_comps: - return - comps_file_arch = compose.paths.work.comps(arch=arch) msg = "Writing comps file for arch '%s': %s" % (arch, comps_file_arch) @@ -227,9 +222,6 @@ def write_arch_comps(compose, arch): def write_variant_comps(compose, arch, variant): - if not compose.has_comps: - return - comps_file = compose.paths.work.comps(arch=arch, variant=variant) msg = "Writing comps file (arch: %s, variant: %s): %s" % (arch, variant, comps_file) @@ -257,18 +249,12 @@ def write_variant_comps(compose, arch, variant): def copy_variant_comps(compose, arch, variant): - if not compose.has_comps: - return - global_comps = compose.paths.work.comps(arch="global") comps_file = compose.paths.work.comps(arch=arch, variant=variant) shutil.copy(global_comps, comps_file) def create_comps_repo(compose, arch): - if not compose.has_comps: - return - createrepo_c = compose.conf.get("createrepo_c", True) createrepo_checksum = compose.conf["createrepo_checksum"] repo = CreaterepoWrapper(createrepo_c=createrepo_c) diff --git a/tests/test_initphase.py b/tests/test_initphase.py index 47ecb03d..891a2c43 100755 --- a/tests/test_initphase.py +++ b/tests/test_initphase.py @@ -34,6 +34,7 @@ class TestInitPhase(PungiTestCase): @mock.patch('pungi.phases.init.write_prepopulate_file') def test_run(self, write_prepopulate, write_variant, create_comps, write_arch, write_global): compose = DummyCompose(self.topdir, {}) + compose.has_comps = True phase = init.InitPhase(compose) phase.run() @@ -61,6 +62,7 @@ class TestInitPhase(PungiTestCase): compose = DummyCompose(self.topdir, { 'keep_original_comps': ['Everything'], }) + compose.has_comps = True phase = init.InitPhase(compose) phase.run() @@ -78,6 +80,26 @@ class TestInitPhase(PungiTestCase): [mock.call(compose, 'x86_64', compose.variants['Everything']), mock.call(compose, 'amd64', compose.variants['Everything'])]) + @mock.patch('pungi.phases.init.copy_variant_comps') + @mock.patch('pungi.phases.init.write_global_comps') + @mock.patch('pungi.phases.init.write_arch_comps') + @mock.patch('pungi.phases.init.create_comps_repo') + @mock.patch('pungi.phases.init.write_variant_comps') + @mock.patch('pungi.phases.init.write_prepopulate_file') + def test_run_without_comps(self, write_prepopulate, write_variant, create_comps, + write_arch, write_global, copy_comps): + compose = DummyCompose(self.topdir, {}) + compose.has_comps = False + phase = init.InitPhase(compose) + phase.run() + + self.assertEqual(write_global.mock_calls, []) + self.assertEqual(write_prepopulate.mock_calls, [mock.call(compose)]) + self.assertItemsEqual(write_arch.mock_calls, []) + self.assertItemsEqual(create_comps.mock_calls, []) + self.assertItemsEqual(write_variant.mock_calls, []) + self.assertItemsEqual(copy_comps.mock_calls, []) + def test_validate_keep_original_comps_missing(self): compose = DummyCompose(self.topdir, MIN_CONFIG) phase = init.InitPhase(compose) @@ -98,19 +120,9 @@ class TestInitPhase(PungiTestCase): class TestWriteArchComps(PungiTestCase): - @mock.patch('pungi.phases.init.run') - def test_does_not_run_without_comps(self, run): - compose = DummyCompose(self.topdir, {}) - compose.has_comps = False - - init.write_arch_comps(compose, 'x86_64') - - self.assertEqual(run.mock_calls, []) - @mock.patch('pungi.phases.init.run') def test_run(self, run): compose = DummyCompose(self.topdir, {}) - compose.has_comps = True compose.DEBUG = False init.write_arch_comps(compose, 'x86_64') @@ -123,7 +135,6 @@ class TestWriteArchComps(PungiTestCase): @mock.patch('pungi.phases.init.run') def test_run_in_debug(self, run): compose = DummyCompose(self.topdir, {}) - compose.has_comps = True compose.DEBUG = True touch(self.topdir + '/work/x86_64/comps/comps-x86_64.xml') @@ -134,21 +145,11 @@ class TestWriteArchComps(PungiTestCase): class TestCreateCompsRepo(PungiTestCase): - @mock.patch('pungi.phases.init.run') - def test_does_not_run_without_comps(self, run): - compose = DummyCompose(self.topdir, {}) - compose.has_comps = False - - init.create_comps_repo(compose, 'x86_64') - - self.assertEqual(run.mock_calls, []) - @mock.patch('pungi.phases.init.run') def test_run(self, run): compose = DummyCompose(self.topdir, { 'createrepo_checksum': 'sha256', }) - compose.has_comps = True compose.DEBUG = False init.create_comps_repo(compose, 'x86_64') @@ -167,7 +168,6 @@ class TestCreateCompsRepo(PungiTestCase): compose = DummyCompose(self.topdir, { 'createrepo_checksum': 'sha256', }) - compose.has_comps = True compose.DEBUG = True os.makedirs(self.topdir + '/work/x86_64/comps_repo/repodata') @@ -178,22 +178,10 @@ class TestCreateCompsRepo(PungiTestCase): class TestWriteGlobalComps(PungiTestCase): - @mock.patch('shutil.copy2') - @mock.patch('pungi.phases.init.get_file_from_scm') - def test_does_not_run_without_comps(self, get_file, copy2): - compose = DummyCompose(self.topdir, {'comps_file': 'some-file.xml'}) - compose.has_comps = False - - init.write_global_comps(compose) - - self.assertEqual(get_file.mock_calls, []) - self.assertEqual(copy2.mock_calls, []) - @mock.patch('shutil.copy2') @mock.patch('pungi.phases.init.get_file_from_scm') def test_run_in_debug(self, get_file, copy2): compose = DummyCompose(self.topdir, {'comps_file': 'some-file.xml'}) - compose.has_comps = True compose.DEBUG = True touch(self.topdir + '/work/global/comps/comps-global.xml') @@ -205,7 +193,6 @@ class TestWriteGlobalComps(PungiTestCase): @mock.patch('pungi.phases.init.get_file_from_scm') def test_run_local_file(self, get_file): compose = DummyCompose(self.topdir, {'comps_file': 'some-file.xml'}) - compose.has_comps = True compose.DEBUG = False def gen_file(src, dest, logger=None): @@ -221,20 +208,10 @@ class TestWriteGlobalComps(PungiTestCase): class TestWriteVariantComps(PungiTestCase): - @mock.patch('pungi.phases.init.run') - def test_does_not_run_without_comps(self, run): - compose = DummyCompose(self.topdir, {}) - compose.has_comps = False - - init.write_variant_comps(compose, 'x86_64', compose.variants['Server']) - - self.assertEqual(run.mock_calls, []) - @mock.patch('pungi.phases.init.run') @mock.patch('pungi.phases.init.CompsWrapper') def test_run(self, CompsWrapper, run): compose = DummyCompose(self.topdir, {}) - compose.has_comps = True compose.DEBUG = False variant = compose.variants['Server'] @@ -257,7 +234,6 @@ class TestWriteVariantComps(PungiTestCase): @mock.patch('pungi.phases.init.CompsWrapper') def test_run_in_debug(self, CompsWrapper, run): compose = DummyCompose(self.topdir, {}) - compose.has_comps = True compose.DEBUG = True variant = compose.variants['Server'] touch(self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml') @@ -276,19 +252,9 @@ class TestWriteVariantComps(PungiTestCase): class TestCopyVariantComps(PungiTestCase): - @mock.patch('shutil.copy') - def test_does_not_run_without_comps(self, copy): - compose = DummyCompose(self.topdir, {}) - compose.has_comps = False - - init.copy_variant_comps(compose, 'x86_64', compose.variants['Server']) - - self.assertEqual(copy.mock_calls, []) - @mock.patch('shutil.copy') def test_run(self, copy): compose = DummyCompose(self.topdir, {}) - compose.has_comps = True variant = compose.variants['Server'] init.copy_variant_comps(compose, 'x86_64', variant)