diff --git a/pungi/phases/init.py b/pungi/phases/init.py index 19631447..0df1ca22 100644 --- a/pungi/phases/init.py +++ b/pungi/phases/init.py @@ -42,7 +42,8 @@ class InitPhase(PhaseBase): def run(self): if self.compose.has_comps: # write global comps and arch comps, create comps repos - write_global_comps(self.compose) + global_comps = write_global_comps(self.compose) + validate_comps(global_comps) for arch in self.compose.get_arches(): write_arch_comps(self.compose, arch) create_comps_repo(self.compose, arch, None) @@ -88,6 +89,8 @@ def write_global_comps(compose): shutil.copy2(os.path.join(tmp_dir, comps_name), comps_file_global) shutil.rmtree(tmp_dir) + return comps_file_global + def write_arch_comps(compose, arch): comps_file_arch = compose.paths.work.comps(arch=arch) @@ -203,3 +206,9 @@ def validate_module_defaults(path): raise RuntimeError( "There are duplicated module defaults:\n%s" % "\n".join(errors) ) + + +def validate_comps(path): + """Check that there are whitespace issues in comps.""" + wrapper = CompsWrapper(path) + wrapper.validate() diff --git a/pungi/wrappers/comps.py b/pungi/wrappers/comps.py index b36f67ec..3160a509 100644 --- a/pungi/wrappers/comps.py +++ b/pungi/wrappers/comps.py @@ -49,6 +49,10 @@ TYPE_MAPPING = collections.OrderedDict([ ]) +class CompsValidationError(ValueError): + pass + + class CompsFilter(object): """ Processor for extended comps file. This class treats the input as just an @@ -230,6 +234,25 @@ class CompsWrapper(object): return [pkg.name for pkg in grp.packages] raise KeyError('No such group %r' % group) + def validate(self): + """Check that no package name contains whitespace, and raise a + RuntimeError if there is a problem. + """ + errors = [] + for group in self.get_comps_groups(): + for pkg in self.get_packages(group): + stripped_pkg = pkg.strip() + if pkg != stripped_pkg: + errors.append( + "Package name %s in group '%s' contains leading or trailing whitespace" + % (stripped_pkg, group) + ) + + if errors: + raise CompsValidationError( + "Comps file contains errors:\n%s" % "\n".join(errors) + ) + def write_comps(self, comps_obj=None, target_file=None): if not comps_obj: comps_obj = self.generate_comps() diff --git a/tests/fixtures/comps-ws.xml b/tests/fixtures/comps-ws.xml new file mode 100644 index 00000000..872525c1 --- /dev/null +++ b/tests/fixtures/comps-ws.xml @@ -0,0 +1,16 @@ + + + + + core + Core + Smallest possible installation + true + false + + foo + bar + baz + + + diff --git a/tests/test_comps_wrapper.py b/tests/test_comps_wrapper.py index 8e9570f3..46cc3b9f 100644 --- a/tests/test_comps_wrapper.py +++ b/tests/test_comps_wrapper.py @@ -12,7 +12,7 @@ import sys sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) -from pungi.wrappers.comps import CompsWrapper, CompsFilter +from pungi.wrappers.comps import CompsWrapper, CompsFilter, CompsValidationError from tests.helpers import FIXTURE_DIR COMPS_FILE = os.path.join(FIXTURE_DIR, 'comps.xml') @@ -20,6 +20,7 @@ COMPS_FORMATTED_FILE = os.path.join(FIXTURE_DIR, 'comps-formatted.xml') COMPS_GROUP_FILE = os.path.join(FIXTURE_DIR, 'comps-group.xml') COMPS_ENVIRONMENT_FILE = os.path.join(FIXTURE_DIR, 'comps-env.xml') COMPS_FILE_WITH_TYPO = os.path.join(FIXTURE_DIR, 'comps-typo.xml') +COMPS_FILE_WITH_WHITESPACE = os.path.join(FIXTURE_DIR, 'comps-ws.xml') class CompsWrapperTest(unittest.TestCase): @@ -98,6 +99,28 @@ class CompsWrapperTest(unittest.TestCase): 'Package dummy-bash in group core has unknown type', str(ctx.exception)) + def test_validate_correct(self): + comps = CompsWrapper(COMPS_FILE) + comps.validate() + + def test_validate_with_whitespace(self): + comps = CompsWrapper(COMPS_FILE_WITH_WHITESPACE) + with self.assertRaises(CompsValidationError) as ctx: + comps.validate() + + self.assertIn( + "Package name foo in group 'core' contains leading or trailing whitespace", + str(ctx.exception), + ) + self.assertIn( + "Package name bar in group 'core' contains leading or trailing whitespace", + str(ctx.exception), + ) + self.assertIn( + "Package name baz in group 'core' contains leading or trailing whitespace", + str(ctx.exception), + ) + COMPS_IN_FILE = os.path.join(FIXTURE_DIR, 'comps.xml.in') diff --git a/tests/test_initphase.py b/tests/test_initphase.py index 85fc3806..b2470c31 100644 --- a/tests/test_initphase.py +++ b/tests/test_initphase.py @@ -12,9 +12,10 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from pungi import Modulemd from pungi.phases import init -from tests.helpers import DummyCompose, PungiTestCase, touch +from tests.helpers import DummyCompose, PungiTestCase, touch, mk_boom +@mock.patch("pungi.phases.init.validate_comps") @mock.patch("pungi.phases.init.validate_module_defaults") @mock.patch("pungi.phases.init.write_module_defaults") @mock.patch("pungi.phases.init.write_global_comps") @@ -33,6 +34,7 @@ class TestInitPhase(PungiTestCase): write_global, write_defaults, validate_defaults, + validate_comps, ): compose = DummyCompose(self.topdir, {}) compose.has_comps = True @@ -72,6 +74,7 @@ class TestInitPhase(PungiTestCase): write_global, write_defaults, validate_defaults, + validate_comps, ): compose = DummyCompose(self.topdir, {}) compose.has_comps = True @@ -82,6 +85,9 @@ class TestInitPhase(PungiTestCase): phase.run() self.assertEqual(write_global.mock_calls, [mock.call(compose)]) + self.assertEqual( + validate_comps.call_args_list, [mock.call(write_global.return_value)] + ) self.assertEqual(write_prepopulate.mock_calls, [mock.call(compose)]) self.assertItemsEqual(write_arch.mock_calls, [mock.call(compose, 'x86_64'), mock.call(compose, 'amd64')]) @@ -110,6 +116,7 @@ class TestInitPhase(PungiTestCase): write_global, write_defaults, validate_defaults, + validate_comps, ): compose = DummyCompose(self.topdir, {}) compose.has_comps = False @@ -118,6 +125,7 @@ class TestInitPhase(PungiTestCase): phase.run() self.assertItemsEqual(write_global.mock_calls, []) + self.assertEqual(validate_comps.call_args_list, []) self.assertItemsEqual(write_prepopulate.mock_calls, [mock.call(compose)]) self.assertItemsEqual(write_arch.mock_calls, []) self.assertItemsEqual(create_comps.mock_calls, []) @@ -134,6 +142,7 @@ class TestInitPhase(PungiTestCase): write_global, write_defaults, validate_defaults, + validate_comps, ): compose = DummyCompose(self.topdir, {}) compose.has_comps = False @@ -142,6 +151,7 @@ class TestInitPhase(PungiTestCase): phase.run() self.assertItemsEqual(write_global.mock_calls, []) + self.assertEqual(validate_comps.call_args_list, []) self.assertItemsEqual(write_prepopulate.mock_calls, [mock.call(compose)]) self.assertItemsEqual(write_arch.mock_calls, []) self.assertItemsEqual(create_comps.mock_calls, []) @@ -517,5 +527,25 @@ class TestValidateModuleDefaults(PungiTestCase): init.validate_module_defaults(self.topdir) +@mock.patch("pungi.phases.init.CompsWrapper") +class TestValidateComps(unittest.TestCase): + def test_ok(self, CompsWrapper): + init.validate_comps("/path") + + self.assertEqual( + CompsWrapper.mock_calls, [mock.call("/path"), mock.call().validate()] + ) + + def test_fail(self, CompsWrapper): + CompsWrapper.return_value.validate.side_effect = mk_boom() + + with self.assertRaises(Exception): + init.validate_comps("/path") + + self.assertEqual( + CompsWrapper.mock_calls, [mock.call("/path"), mock.call().validate()] + ) + + if __name__ == "__main__": unittest.main()