From ab28ccc9b0ee52de1883769b6575d179ec567740 Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 15 2019 22:51:50 +0000 Subject: [PATCH 1/6] tests: Close open files and use assertEqual --- diff --git a/rpmfluff.py b/rpmfluff.py index 54a1833..f709c3c 100644 --- a/rpmfluff.py +++ b/rpmfluff.py @@ -420,6 +420,7 @@ class SourceFile: def write_file(self, sourcesDir): dstFile = self._get_dst_file(sourcesDir) dstFile.write(self.content) + dstFile.close() class GeneratedSourceFile: def __init__(self, sourceName, fileConstraints): @@ -435,6 +436,7 @@ class GeneratedSourceFile: dstFile = self._get_dst_file(sourcesDir) for c in self.fileConstraints: c.affect_file(dstFile) + dstFile.close() class ExternalSourceFile: def __init__(self, sourceName, path): @@ -936,6 +938,7 @@ class SimpleRpmBuild(RpmBuild): specFile.write("%changelog\n") specFile.write(self.section_changelog) specFile.write("\n") + specFile.close() return specFileName @@ -1399,6 +1402,7 @@ class TestSimpleRpmBuild(unittest.TestCase): ts.setVSFlags(-1) # disable all verifications fd = os.open(rpmFilename, os.O_RDONLY) h = ts.hdrFromFdno(fd) + os.close(fd) self.assertIn(text, str(h[tagId])) def assert_is_dir(self, dirname): @@ -1444,9 +1448,9 @@ class TestSimpleRpmBuild(unittest.TestCase): rpmFile = os.path.join(rpmsDir, arch, "test-build-0.1-1.%s.rpm"%arch) self.assert_is_file(rpmFile) h = get_rpm_header(rpmFile) - self.assertEquals(h['name'], b'test-build') - self.assertEquals(h['version'], b'0.1') - self.assertEquals(h['release'], b'1') + self.assertEqual(h['name'], b'test-build') + self.assertEqual(h['version'], b'0.1') + self.assertEqual(h['release'], b'1') def test_add_requires(self): self.rpmbuild.add_requires("test-requirement") @@ -1724,13 +1728,13 @@ class TestSimpleRpmBuild(unittest.TestCase): self.assert_header_has_item(rpmFile, rpm.RPMTAG_POSTUN, script) def test_subpackage_names_A(self): - self.assertEquals(self.rpmbuild.get_subpackage_names(), ["test-subpackage-names-A"]) + self.assertEqual(self.rpmbuild.get_subpackage_names(), ["test-subpackage-names-A"]) def test_subpackage_names_B(self): self.rpmbuild.add_devel_subpackage() self.rpmbuild.add_subpackage('ssl') self.rpmbuild.makeDebugInfo=True - self.assertEquals(self.rpmbuild.get_subpackage_names(), ['test-subpackage-names-B', + self.assertEqual(self.rpmbuild.get_subpackage_names(), ['test-subpackage-names-B', 'test-subpackage-names-B-devel', 'test-subpackage-names-B-ssl', 'test-subpackage-names-B-debuginfo']) @@ -1807,9 +1811,9 @@ class TestSimpleRpmBuild(unittest.TestCase): # FIXME: sort out architecture properly rpmFile = self.rpmbuild.get_built_rpm(arch) h = get_rpm_header(rpmFile) - self.assertEquals(h['name'], b'test-multiple-sources') - self.assertEquals(h['version'], b'0.1') - self.assertEquals(h['release'], b'1') + self.assertEqual(h['name'], b'test-multiple-sources') + self.assertEqual(h['version'], b'0.1') + self.assertEqual(h['release'], b'1') def test_generated_tarball(self): pkgName = b'test-generated-tarball' @@ -1829,9 +1833,9 @@ class TestSimpleRpmBuild(unittest.TestCase): # FIXME: sort out architecture properly rpmFile = self.rpmbuild.get_built_rpm(arch) h = get_rpm_header(rpmFile) - self.assertEquals(h['name'], pkgName) - self.assertEquals(h['version'], b'0.1') - self.assertEquals(h['release'], b'1') + self.assertEqual(h['name'], pkgName) + self.assertEqual(h['version'], b'0.1') + self.assertEqual(h['release'], b'1') def test_simple_compilation(self): @@ -2045,7 +2049,7 @@ class TestSimpleRpmBuild(unittest.TestCase): self.rpmbuild.make() srpmHdr = self.rpmbuild.get_built_srpm_header() - self.assertEquals(3, srpmHdr[rpm.RPMTAG_EPOCH]) + self.assertEqual(3, srpmHdr[rpm.RPMTAG_EPOCH]) def test_add_manpage(self): self.rpmbuild.add_manpage() From 64cfa00862367905de8221d3fc8e5f1a997aacfe Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 15 2019 23:04:49 +0000 Subject: [PATCH 2/6] tests: Revert to strings for NVR results --- diff --git a/rpmfluff.py b/rpmfluff.py index f709c3c..e8214ba 100644 --- a/rpmfluff.py +++ b/rpmfluff.py @@ -1448,9 +1448,9 @@ class TestSimpleRpmBuild(unittest.TestCase): rpmFile = os.path.join(rpmsDir, arch, "test-build-0.1-1.%s.rpm"%arch) self.assert_is_file(rpmFile) h = get_rpm_header(rpmFile) - self.assertEqual(h['name'], b'test-build') - self.assertEqual(h['version'], b'0.1') - self.assertEqual(h['release'], b'1') + self.assertEqual(h['name'], 'test-build') + self.assertEqual(h['version'], '0.1') + self.assertEqual(h['release'], '1') def test_add_requires(self): self.rpmbuild.add_requires("test-requirement") @@ -1811,12 +1811,12 @@ class TestSimpleRpmBuild(unittest.TestCase): # FIXME: sort out architecture properly rpmFile = self.rpmbuild.get_built_rpm(arch) h = get_rpm_header(rpmFile) - self.assertEqual(h['name'], b'test-multiple-sources') - self.assertEqual(h['version'], b'0.1') - self.assertEqual(h['release'], b'1') + self.assertEqual(h['name'], 'test-multiple-sources') + self.assertEqual(h['version'], '0.1') + self.assertEqual(h['release'], '1') def test_generated_tarball(self): - pkgName = b'test-generated-tarball' + pkgName = 'test-generated-tarball' self.rpmbuild.add_generated_tarball('test-tarball-0.1.tar.gz', 'test-tarball-0.1', [GeneratedSourceFile("test-1", make_elf()), @@ -1834,8 +1834,8 @@ class TestSimpleRpmBuild(unittest.TestCase): rpmFile = self.rpmbuild.get_built_rpm(arch) h = get_rpm_header(rpmFile) self.assertEqual(h['name'], pkgName) - self.assertEqual(h['version'], b'0.1') - self.assertEqual(h['release'], b'1') + self.assertEqual(h['version'], '0.1') + self.assertEqual(h['release'], '1') def test_simple_compilation(self): From 68cf138a3530c7ce21268f543323df30bd411d12 Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 15 2019 23:05:14 +0000 Subject: [PATCH 3/6] tests: Remove leftover pdb import --- diff --git a/rpmfluff.py b/rpmfluff.py index e8214ba..9e6db55 100644 --- a/rpmfluff.py +++ b/rpmfluff.py @@ -177,7 +177,6 @@ class CheckTrigger(Check): # No match: try next one: index += 1 # We din't find the trigger: - import pdb; pdb.set_trace() raise FailedCheck(self, 'trigger for event "%s" on "%s" not found within RPM'%(self.trigger.event, self.trigger.triggerConds)) class CheckRequires(Check): From a86350f7fd3fa5dbc8e0b64aaeb8a82af771efcc Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 15 2019 23:11:54 +0000 Subject: [PATCH 4/6] Turn off the UTF8ENCODE fix When running with rpm-4.14.2.1-7.fc31.x86_64 most of the tests fail because they get converted to bytes, but strings are expected. --- diff --git a/rpmfluff.py b/rpmfluff.py index 9e6db55..1dca3e1 100644 --- a/rpmfluff.py +++ b/rpmfluff.py @@ -32,7 +32,7 @@ import sys import rpm import subprocess -UTF8ENCODE = None +UTF8ENCODE = False def _which(cmd): """Hacked Python 3.3+ shutil.which() with limited functionality.""" From 3c63182ed0afb758c86326dcf3de7fbb8d052c93 Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 15 2019 23:36:24 +0000 Subject: [PATCH 5/6] Make sure correct version of rpm is used --- diff --git a/python-rpmfluff.spec b/python-rpmfluff.spec index 37783a4..97358db 100644 --- a/python-rpmfluff.spec +++ b/python-rpmfluff.spec @@ -30,6 +30,7 @@ BuildRequires: python3-devel BuildRequires: python3-rpm Requires: rpm-build Requires: createrepo_c +Requires: rpm >= 4.14.2.1-7 %description -n python3-%{modname} %{_description} From 9feecc25410b4fc83e795f66513a8e8a36b94017 Mon Sep 17 00:00:00 2001 From: Brian C. Lane Date: Apr 16 2019 16:49:34 +0000 Subject: [PATCH 6/6] Cleanup some pylint warnings --- diff --git a/rpmfluff.py b/rpmfluff.py index 1dca3e1..806e17c 100644 --- a/rpmfluff.py +++ b/rpmfluff.py @@ -99,13 +99,14 @@ class FailedCheck(Exception): def __init__(self, check, extraInfo=None): self.check = check self.extraInfo = extraInfo + super(FailedCheck, self).__init__() def __str__(self): - str = self.check.get_failure_message() + s = self.check.get_failure_message() if self.extraInfo: - return "%s (%s)"%(str, self.extraInfo) + return "%s (%s)"%(s, self.extraInfo) else: - return str + return s class CheckPayloadFile(Check): """Check that a built package contains a specified payload file or directory""" @@ -194,7 +195,6 @@ class CheckRequires(Check): def check(self, build): rpmHdr = build.get_built_rpm_header(self.arch, self.packageName) # Search by event type and trigger condition: - index = 0 for t in rpmHdr[rpm.RPMTAG_REQUIRES]: if t == self.requires: return # found a match @@ -217,7 +217,6 @@ class CheckProvides(Check): def check(self, build): rpmHdr = build.get_built_rpm_header(self.arch, self.packageName) # Search by event type and trigger condition: - index = 0 for t in rpmHdr[rpm.RPMTAG_PROVIDES]: if t == self.provides: return # found a match @@ -279,9 +278,6 @@ class Buildable: def clean(self): raise NotImplementedError - def check(self): - raise NotImplementedError - def do_make(self): raise NotImplementedError @@ -352,9 +348,9 @@ class RpmBuild(Buildable): self.check_results() def __write_log(self, log, arch): - dir = self.get_build_log_dir(arch) - if not os.path.exists(dir): - os.makedirs(dir) + log_dir = self.get_build_log_dir(arch) + if not os.path.exists(log_dir): + os.makedirs(log_dir) filename = self.get_build_log_path(arch) f = open(filename, "wb") for line in log: @@ -1112,7 +1108,7 @@ class SimpleRpmBuild(RpmBuild): Given a file at installPath, add commands to installation to ensure the directory holding it exists. """ - (head, tail) = os.path.split(installPath) + (head, _tail) = os.path.split(installPath) self.section_install += "mkdir -p $RPM_BUILD_ROOT/%s\n"%head def add_mode(self, @@ -1192,8 +1188,8 @@ class SimpleRpmBuild(RpmBuild): import random random.seed() content = '' - for i in range(size): - content = content + chr(random.randrange(32, 127)) + for _ in range(size): + content = content + chr(random.randrange(32, 127)) name = "%s-%s-%s-%s-%s-%s.txt" % (self.epoch, self.name, self.version, expand_macros(self.release), self.get_build_archs()[0], len(self.sources)) self.add_installed_file(installPath = 'usr/share/doc/%s' % name, sourceFile = SourceFile(name, content), @@ -1207,7 +1203,7 @@ class SimpleRpmBuild(RpmBuild): createParentDirs=True, subpackageSuffix=None): """Add a simple source file to the sources, build it, and install it somewhere, using the given compilation flags""" - sourceId = self.add_source(SourceFile(sourceFileName, sourceContent)) + _sourceId = self.add_source(SourceFile(sourceFileName, sourceContent)) self.section_build += "%if 0%{?__isa_bits} == 32\n%define mopt -m32\n%endif\n" self.section_build += "gcc %%{?mopt} %s %s\n"%(compileFlags, sourceFileName) if createParentDirs: @@ -1227,7 +1223,7 @@ class SimpleRpmBuild(RpmBuild): subpackageSuffix=None): """Add a simple source file to the sources, build it as a library, and install it somewhere, using the given compilation flags""" - sourceId = self.add_source(SourceFile(sourceFileName, sourceContent)) + _sourceId = self.add_source(SourceFile(sourceFileName, sourceContent)) self.section_build += "gcc --shared -fPIC -o %s %s %s\n"%(libraryName, compileFlags, sourceFileName) if createParentDirs: self.create_parent_dirs(installPath) @@ -1290,7 +1286,7 @@ class SimpleRpmBuild(RpmBuild): createParentDirs=True, installPath='/usr/share', subpackageSuffix=None): - sourceIndex = self.add_source(GeneratedTarball(tarballName, internalPath, contents)) + _sourceIndex = self.add_source(GeneratedTarball(tarballName, internalPath, contents)) if extract: self.section_build += "tar -zxvf %s\n" % tarballName if createParentDirs: @@ -1431,12 +1427,12 @@ class TestSimpleRpmBuild(unittest.TestCase): buildDir = os.path.join(tmpDir, "BUILD") self.assert_is_dir(buildDir) - sourcesDir = os.path.join(tmpDir, "SOURCES") + _sourcesDir = os.path.join(tmpDir, "SOURCES") self.assert_is_dir(buildDir) srpmsDir = os.path.join(tmpDir, "SRPMS") self.assert_is_dir(srpmsDir) - srpmFile = os.path.join(srpmsDir, "test-build-0.1-1.src.rpm") + _srpmFile = os.path.join(srpmsDir, "test-build-0.1-1.src.rpm") self.assert_is_file(os.path.join(srpmsDir, "test-build-0.1-1.src.rpm")) rpmsDir = self.rpmbuild.get_rpms_dir() @@ -1586,15 +1582,15 @@ class TestSimpleRpmBuild(unittest.TestCase): self.assert_header_has_item(rpmFile, rpm.RPMTAG_PACKAGER, packager) def test_add_license(self): - license = 'SomeLicense' - self.rpmbuild.addLicense(license) + licenseName = 'SomeLicense' + self.rpmbuild.addLicense(licenseName) self.rpmbuild.make() # FIXME: sort out architecture properly for arch in [expectedArch]: rpmFile = self.rpmbuild.get_built_rpm(arch) self.assert_is_file(rpmFile) - self.assert_header_has_item(rpmFile, rpm.RPMTAG_LICENSE, license) + self.assert_header_has_item(rpmFile, rpm.RPMTAG_LICENSE, licenseName) def test_archs_build(self): archs = ('i386', 'x86_64', 'ppc') @@ -1764,9 +1760,9 @@ class TestSimpleRpmBuild(unittest.TestCase): self.rpmbuild.add_installed_file("/foo.so", GeneratedSourceFile("foo.so", make_elf()), mode="0755") self.rpmbuild.make() - rpm = self.rpmbuild.get_built_rpm(self.rpmbuild.get_build_archs()[0]) + rpm_name = self.rpmbuild.get_built_rpm(self.rpmbuild.get_build_archs()[0]) files = subprocess.check_output(["rpm", "-qp", "--qf", - "[%{FILENAMES} %{FILEMODES:perms}\n]", rpm]) + "[%{FILENAMES} %{FILEMODES:perms}\n]", rpm_name]) assert files.split(b"\n")[0].strip().decode() == '/foo.so -rwxr-xr-x' def test_escape_path(self): @@ -1857,7 +1853,7 @@ class TestSimpleRpmBuild(unittest.TestCase): h = get_rpm_header(rpmFile) files = list(h.fiFromHeader()) self.assertEqual(1, len(files)) - (filename, size, mode, mtime, flags, rdev, inode, FNlink, Fstate, vflags, user, group, md5sum) = files[0] + (filename, _size, mode, _mtime, _flags, _rdev, _inode, _FNlink, _Fstate, _vflags, _user, _group, _md5sum) = files[0] self.assertEqual("/var/spool/foo", filename) self.assertEqual(0o041777, mode) @@ -2028,7 +2024,7 @@ class TestSimpleRpmBuild(unittest.TestCase): hdr = self.rpmbuild.get_built_rpm_header(expectedArch) files = list(hdr.fiFromHeader()) self.assertEqual(1, len(files)) - (filename, size, mode, mtime, flags, rdev, inode, FNlink, Fstate, vflags, user, group, md5sum) = files[0] + (filename, _size, _mode, _mtime, _flags, _rdev, _inode, _FNlink, _Fstate, _vflags, user, group, _md5sum) = files[0] self.assertEqual('/var/www/html/index.html', filename) self.assertEqual('apache', user) self.assertEqual('apache', group)