#25 Fix tests and turn off UTF8ENCODE fix
Closed 4 years ago by bcl. Opened 4 years ago by bcl.
bcl/rpmfluff master-rpm-strings  into  master

Cleanup some pylint warnings
Brian C. Lane • 4 years ago  
Turn off the UTF8ENCODE fix
Brian C. Lane • 4 years ago  
tests: Remove leftover pdb import
Brian C. Lane • 4 years ago  
file modified
+1
@@ -30,6 +30,7 @@ 

  BuildRequires:  python3-rpm

  Requires:       rpm-build

  Requires:       createrepo_c

+ Requires:       rpm >=  4.14.2.1-7

  

  %description -n python3-%{modname} %{_description}

  

file modified
+40 -41
@@ -32,7 +32,7 @@ 

  import rpm

  import subprocess

  

- UTF8ENCODE = None

+ UTF8ENCODE = False

  

  def _which(cmd):

      """Hacked Python 3.3+ shutil.which() with limited functionality."""
@@ -99,13 +99,14 @@ 

      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"""
@@ -177,7 +178,6 @@ 

              # 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):
@@ -195,7 +195,6 @@ 

      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
@@ -218,7 +217,6 @@ 

      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
@@ -280,9 +278,6 @@ 

      def clean(self):

          raise NotImplementedError

  

-     def check(self):

-         raise NotImplementedError

- 

      def do_make(self):

          raise NotImplementedError

  
@@ -353,9 +348,9 @@ 

          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:
@@ -420,6 +415,7 @@ 

      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 +431,7 @@ 

          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 +933,7 @@ 

              specFile.write("%changelog\n")

              specFile.write(self.section_changelog)

              specFile.write("\n")

+         specFile.close()

  

          return specFileName

  
@@ -1110,7 +1108,7 @@ 

          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,
@@ -1190,8 +1188,8 @@ 

          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),
@@ -1205,7 +1203,7 @@ 

                                 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:
@@ -1225,7 +1223,7 @@ 

                             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)
@@ -1288,7 +1286,7 @@ 

                                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:
@@ -1399,6 +1397,7 @@ 

          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):
@@ -1428,12 +1427,12 @@ 

          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()
@@ -1444,9 +1443,9 @@ 

              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'], 'test-build')

+             self.assertEqual(h['version'], '0.1')

+             self.assertEqual(h['release'], '1')

  

      def test_add_requires(self):

          self.rpmbuild.add_requires("test-requirement")
@@ -1583,15 +1582,15 @@ 

              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')
@@ -1724,13 +1723,13 @@ 

              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'])
@@ -1761,9 +1760,9 @@ 

          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):
@@ -1807,12 +1806,12 @@ 

              # 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'], '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()),
@@ -1829,9 +1828,9 @@ 

              # 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'], '0.1')

+             self.assertEqual(h['release'], '1')

  

  

      def test_simple_compilation(self):
@@ -1854,7 +1853,7 @@ 

              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)

  
@@ -2025,7 +2024,7 @@ 

          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)
@@ -2045,7 +2044,7 @@ 

          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()

The fix does not work with rpm-4.14.2.1-7 which just appeared in rawhide, so disable it until this gets sorted out.
Also fixed some test warnings that made it hard to read the results, and reverted the NVR test's byte arrays back to strings.
Add a Requires for the newer version of rpm to avoid confusion.

1 new commit added

  • Cleanup some pylint warnings
4 years ago

Comited 3 patches which are fixing pylint issues, pdb leftover, opened files and assert straight away. Thank you!

I have been looking into rest of the patches and I'm not getting why these are needed?

Could you please also try with this commit - if it changes something?:

https://pagure.io/rpmfluff/c/3148594b69614f00c0d266f886be84add1ba0fa8?branch=master

Thank you in advance,
Jan

I have been looking into rest of the patches and I'm not getting why these are needed?
Could you please also try with this commit - if it changes something?:
https://pagure.io/rpmfluff/c/3148594b69614f00c0d266f886be84add1ba0fa8?branch=master

Ah, good! That fixes it for me, so the patch setting it to False can be dropped.

The other patches fix warnings from pylint.

Pull-Request has been closed by bcl

4 years ago
Metadata