Merge lp:~javier.collado/utah/iso_validation_files_list into lp:utah

[Javier Collado]

This branch applies some refactoring to the `files_list` test case basically to
print a readable error in case of failure.

Aside from that, there are other small changes like moving the test case
comments to docstrings so that they can be used in the output to provide a
better description of the test case together with its id.

[Max Brustkern (nuclearbob)]

I'm getting an error for:
AssertionError: 'isolinux/adtxt.cfg' not found in:
.
./.disk
...
./isolinux/adtxt.cfg
...

The same image passes with what's currently in the dev branch.

If I have time this week, I'll see if I can find a solution, otherwise, we can revisit it when Javier returns. The docstring changes and general principle of this are good.

review: Needs Fixing

[lp:~javier.collado/utah/iso_validation_files_list]

798. By Javier Collado on 2013-01-03

Fixed typos

799. By Javier Collado on 2013-01-03

Added help to name parameter

800. By Javier Collado on 2013-01-03

Fixed matching problem using path normalization

Interestingly, the output of `bsdtar -t -f` using a desktop i386 image contains
a list of files prefixed with `./`. However, the output using a desktop amd64
image doesn't contain such a prefix. This turns out to be a problem when
matching the complete path (previously just a substring was matched). To fix
the problem `os.path.normpath` is used to get the same path regardless of the
output of `bsdtar -f -t`.

[Javier Collado (javier.collado)]

Thanks for finding that bug. It was introduced when I decided to use a set with
all the file names in the image to match the ones in the `.list` file
(previously the file name was looked up using a string search in the whole
`bsdtar`). Using `os.path.normpath` as explained in the commit message works
fine (tested with desktop/server i386/amd64 images).

review: Needs Resubmitting

[Max Brustkern (nuclearbob)]

This seems to be working well now for me. Good stuff.

review: Approve

=== modified file 'utah/isotest/iso_static_validation.py'
--- utah/isotest/iso_static_validation.py	2012-12-13 17:30:08 +0000
+++ utah/isotest/iso_static_validation.py	2013-01-03 11:57:21 +0000
@@ -25,7 +25,7 @@
 # as published by the Free Software Foundation, either version 3 of
 # the License, or (at your option) any later version.
 #
-# iso_satatic_validation is distributed in the hope that it will
+# iso_static_validation is distributed in the hope that it will
 # be useful, but WITHOUT ANY WARRANTY; without even the implied warranty
 # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
@@ -76,7 +76,7 @@
 
 #Read the iso name and initialise global variables
 parser = argparse.ArgumentParser()
-parser.add_argument('--name')
+parser.add_argument('--name', help='Path to the ISO file to be tested')
 args = parser.parse_args()
 
 
@@ -137,8 +137,10 @@
             unittest.fail("Setup error")
 
     def setUp(self):
-        logging.info('\n{separator}\n{id}\n{separator}'
-                     .format(separator='-' * 80, id=self.id()))
+        logging.info('\n{separator}\n{id}\n{description}\n{separator}'
+                     .format(separator='-' * 80,
+                             id=self.id(),
+                             description=self.shortDescription() or ''))
         self.block_size = ONE_MB_BLOCK
         self.iso_location = iso_location
         logging.debug('Using iso at: ' + self.iso_location)
@@ -170,8 +172,8 @@
         else:
             self.fail("Image name be in the form of release-variant-arch.iso")
 
-    # Test if the sha check sum of the iso matches that is given in the server
     def test_sha256_checksum(self):
+        """Test if the ISO SHA checksum matches what is given in the server."""
         current_iso_digest = {}
         sha256sum_url = os.path.join(self.url, 'current', 'SHA256SUMS')
         try:
@@ -208,8 +210,8 @@
         logging.debug("Local File Checksum: '%s'", sha256)
         self.assertEqual(iso_sha256, sha256)
 
-    # Test if the list file in the repository matches the content of the iso
     def test_files_list(self):
+        """Test if the list repository file matches the content of the ISO."""
         if self.st_variant == 'server':
             list_url = os.path.join(self.url, 'current',
                                     self.st_release + '-' + 'server' + '-' +
@@ -233,13 +235,18 @@
         self.assertEqual(stderr, '')
 
         logging.debug('Checking that each list entry is in the iso')
+        iso_files = set(os.path.normpath(path)
+                        for path in stdout.splitlines())
         for list_entry in list_repository:
-            self.assertIn(list_entry[1:].rstrip(), stdout)
+            fname = os.path.normpath(list_entry[1:].rstrip())
+            self.assertIn(fname, iso_files,
+                          '{!r} not found in:\n{}'
+                          .format(fname, stdout))
 
-    # Test if the manfest is the same as that is given in the server
     @unittest.skipUnless(ubiquity_image,
                          "manifest only for ubiquity based images")
     def test_manifest(self):
+        """Test if the ISO manifest matches the one in the server."""
         manifest_url = os.path.join(self.url, 'current',
                                     self.st_release + '-' +
                                     self.st_variant + '-' +
@@ -259,9 +266,12 @@
         for manifest_entry in manifest:
             self.assertIn(manifest_entry, manifest_local)
 
-    # Test if the buildid present in the download server is
-    # the same as that given in distro info
     def test_build_id(self):
+        """Test if the ISO buildid matches the one in the server.
+
+        The buildid is extracted from the distro info file.
+
+        """
         logging.debug('Extract distro information of the image from the iso')
         buildid_path = self.iso.extract("./.disk/info", self.temp_dir)
         buildinfo = open(buildid_path)
@@ -284,11 +294,11 @@
             logging.debug('Checking buildids are the same in iso and repo')
             self.assertIn(id_server, build_info_line)
 
-    # Test if  wubi is present for desktop or dvd and it is a valid PE
     @unittest.skipUnless(st_variant == 'desktop' and
                          st_arch != 'powerpc',
                          "Skipping images that aren't ubiquity-based.")
     def test_wubi(self):
+        """Test if  wubi is present for desktop or dvd and it is a valid PE."""
         logging.debug('Extracting wubi from the desktop images')
         wubi_path = self.iso.extract('./wubi.exe', self.temp_dir)
         cmd = ["file", wubi_path]
@@ -299,10 +309,10 @@
         self.assertEqual(stderr, '')
         self.assertIn(" PE32 executable (GUI)", stdout)
 
-    # Test if the relevant files are present in the iso for desktop iso
     @unittest.skipUnless(st_variant == 'desktop' and
                          st_arch != 'powerpc', "Skipping for non desktop iso")
     def test_files_ubiquity(self):
+        """Test if the relevant files are present in the ISO for desktop."""
         (stdout, stderr) = self.iso.listfiles()
         logging.debug('Check for error in extracting file list from the iso')
         self.assertEqual(stderr, '')
@@ -320,10 +330,10 @@
             else:
                 self.assertIn(path, stdout)
 
-    # Test if the relevant important files are present in the iso for dvd iso
     @unittest.skipUnless(st_variant == 'dvd',
                          "This file list test is only specific for dvd")
     def test_files_ubiquity_dvd(self):
+        """Test if the relevant important files are present in DVD ISO."""
         (stdout, stderr) = self.iso.listfiles()
         logging.debug('Check for error in extracting file list from the iso')
         self.assertEqual(stderr, '')
@@ -332,13 +342,16 @@
             logging.debug('Check if relevant files are present in the iso')
             self.assertIn(list_entry.rstrip(), stdout)
 
-    # Test if the relevant files are present in the iso for d-i based images,
-    # two list of files are used
-    # file_list_di for i386 and amd64
-    # file_list_di_powerpc for powerpc and amd64+mac
     # Test is skipped for ubiquity based images
     @unittest.skipIf(ubiquity_image, "Skipping for ubiquity images")
     def test_files_di(self):
+        """Test if the relevant files are present for d-i based images ISO.
+
+        Two list of files are used:
+        file_list_di for i386 and amd64
+        file_list_di_powerpc for powerpc and amd64+mac
+
+        """
         (stdout, stderr) = self.iso.listfiles()
         logging.debug('Check for error in extracting file list from the iso')
         self.assertEqual(stderr, '')
@@ -361,10 +374,10 @@
             else:
                 self.assertIn(path, stdout)
 
-    # Test if vmlinuz is a valid executable
     @unittest.skipUnless(st_arch != 'powerpc',
                          "vmlinuz is not present only for powerpc images")
     def test_vmlinuz(self):
+        """Test if vmlinuz is a valid executable."""
         logging.debug('Extract kernel image from the iso')
         path = iso.kernelpath()
         vmlinuz_path = self.iso.extract(path, self.temp_dir)
@@ -376,10 +389,13 @@
         self.assertEqual(stderr, '')
         self.assertIn('Linux kernel', stdout)
 
-    # Test if filesystem.squashfs is present and
-    # if that is a squash fs for ubiquity based images
     @unittest.skipUnless(ubiquity_image, "Skipping for non ubiquity images")
     def test_filesystem_squashfs(self):
+        """Test if filesystem.squashfs is present and its contents.
+
+        The squash fs must be for ubiquity based images.
+
+        """
         logging.debug('Extracting the filesystem.squashfs')
         squashfs_path = self.iso.extract('casper/filesystem.squashfs',
                                          self.temp_dir)