Merge lp:~smoser/curtin/trunk.vmtest-cleanup into lp:~curtin-dev/curtin/trunk

[Scott Moser]

tests: clean up usage of collect files

This adds a helper 'collect_path' to the vmtests base class, and then
uses it wherever appropriate. It also cleans up all code that
used read collect files to use load_collect_file.

Lastly, it updates output_files_exist and output_files_dont_exist
to show which files do or do not exist. Previously all you'd see
in the log was an assertion that something was or was not true.
Now, you'll see a list of files that do or do not exist, and not
just the first.

[lp:~smoser/curtin/trunk.vmtest-cleanup]

491. By Scott Moser on 2017-04-21

fix assertEqual.. should be expected, actual

492. By Scott Moser on 2017-04-21

add a message

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:492
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~smoser/curtin/trunk.vmtest-cleanup/+merge/322980/+edit-commit-message

https://jenkins.ubuntu.com/server/job/curtin-ci/465/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/465
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/465
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/465
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/465
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/465

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/465/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:492
https://jenkins.ubuntu.com/server/job/curtin-ci/466/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/466
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/466
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/466
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/466
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/466

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/466/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Looks, good, likely needs a rebase.

review: Approve

[lp:~smoser/curtin/trunk.vmtest-cleanup]

493. By Scott Moser on 2017-05-11

merge from trunk

[Scott Moser (smoser)]

merged with trunk. should be good now.

[lp:~smoser/curtin/trunk.vmtest-cleanup]

494. By Scott Moser on 2017-05-11

change collect_path to be a class method, so it can be used from class-methods.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:493
https://jenkins.ubuntu.com/server/job/curtin-ci/494/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/494/console
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/494/console
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/494/console
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/494/console

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/494/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:494
https://jenkins.ubuntu.com/server/job/curtin-ci/496/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/496
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/496
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/496
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/496

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/496/rebuild

review: Approve (continuous-integration)

[lp:~smoser/curtin/trunk.vmtest-cleanup]

495. By Scott Moser on 2017-05-12

fix some tests that did not collect fstab or ls_dname

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:495
https://jenkins.ubuntu.com/server/job/curtin-ci/499/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/499
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/499
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/499
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/499

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/499/rebuild

review: Approve (continuous-integration)

=== modified file 'tests/vmtests/__init__.py'
--- tests/vmtests/__init__.py	2017-04-21 20:08:30 +0000
+++ tests/vmtests/__init__.py	2017-05-12 19:12:20 +0000
@@ -942,19 +942,26 @@
         return boot_log_wrap(cls.__name__, myboot, cmd, console_log, timeout,
                              purpose)
 
+    @classmethod
+    def collect_path(cls, path):
+        # return a full path to the collected file
+        # prepending ./ makes '/root/file' or 'root/file' work as expected.
+        return os.path.normpath(os.path.join(cls.td.collect, "./" + path))
+
     # Misc functions that are useful for many tests
     def output_files_exist(self, files):
-        for f in files:
-            logger.debug('checking file %s', f)
-            self.assertTrue(os.path.exists(os.path.join(self.td.collect, f)))
+        missing = [f for f in files
+                   if not os.path.exists(self.collect_path(f))]
+        self.assertEqual([], missing,
+                         msg="expected collected files do not exist.")
 
     def output_files_dont_exist(self, files):
-        for f in files:
-            logger.debug('checking file %s', f)
-            self.assertFalse(os.path.exists(os.path.join(self.td.collect, f)))
+        found = [f for f in files if os.path.exists(self.collect_path(f))]
+        self.assertEqual([], found,
+                         msg="Collected files exist that should not.")
 
     def load_collect_file(self, filename, mode="r"):
-        with open(os.path.join(self.td.collect, filename), mode) as fp:
+        with open(self.collect_path(filename), mode) as fp:
             return fp.read()
 
     def load_log_file(self, filename):
@@ -999,8 +1006,7 @@
             self.assertRegexpMatches(s, r)
 
     def get_blkid_data(self, blkid_file):
-        with open(os.path.join(self.td.collect, blkid_file)) as fp:
-            data = fp.read()
+        data = self.load_collect_file(blkid_file)
         ret = {}
         for line in data.splitlines():
             if line == "":
@@ -1010,29 +1016,32 @@
         return ret
 
     def test_fstab(self):
-        if (os.path.exists(self.td.collect + "fstab") and
-                self.fstab_expected is not None):
-            with open(os.path.join(self.td.collect, "fstab")) as fp:
-                fstab_lines = fp.readlines()
-            fstab_entry = None
-            for line in fstab_lines:
-                for device, mntpoint in self.fstab_expected.items():
-                        if device in line:
-                            fstab_entry = line
-                            self.assertIsNotNone(fstab_entry)
-                            self.assertEqual(fstab_entry.split(' ')[1],
-                                             mntpoint)
+        if self.fstab_expected is None:
+            return
+        path = self.collect_path("fstab")
+        if not os.path.exists(path):
+            return
+        fstab_entry = None
+        for line in util.load_file(path).splitlines():
+            for device, mntpoint in self.fstab_expected.items():
+                    if device in line:
+                        fstab_entry = line
+                        self.assertIsNotNone(fstab_entry)
+                        self.assertEqual(fstab_entry.split(' ')[1],
+                                         mntpoint)
 
     def test_dname(self):
-        fpath = os.path.join(self.td.collect, "ls_dname")
-        if (os.path.exists(fpath) and self.disk_to_check is not None):
-            with open(fpath, "r") as fp:
-                contents = fp.read().splitlines()
-            for diskname, part in self.disk_to_check:
-                if part is not 0:
-                    link = diskname + "-part" + str(part)
-                    self.assertIn(link, contents)
-                self.assertIn(diskname, contents)
+        if self.disk_to_check is None:
+            return
+        path = self.collect_path("ls_dname")
+        if not os.path.exists(path):
+            return
+        contents = util.load_file(path)
+        for diskname, part in self.disk_to_check:
+            if part is not 0:
+                link = diskname + "-part" + str(part)
+                self.assertIn(link, contents)
+            self.assertIn(diskname, contents)
 
     def test_reporting_data(self):
         with open(self.reporting_log, 'r') as fp:
@@ -1057,8 +1066,7 @@
         """ Check that curtin has removed /etc/network/interfaces.d/eth0.cfg
             by examining the output of a find /etc/network > find_interfaces.d
         """
-        fpath = os.path.join(self.td.collect, "find_interfacesd")
-        interfacesd = util.load_file(fpath)
+        interfacesd = self.load_collect_file("find_interfacesd")
         self.assertNotIn("/etc/network/interfaces.d/eth0.cfg",
                          interfacesd.split("\n"))
 
@@ -1116,7 +1124,7 @@
     @classmethod
     def collect_output(cls):
         logger.debug('Psuedo extracting output disk')
-        with open(os.path.join(cls.td.collect, "fstab"), "w") as fp:
+        with open(cls.collect_path("fstab"), "w") as fp:
             fp.write('\n'.join(("# psuedo fstab",
                                 "LABEL=root / ext4 defaults 0 1")))
 

=== modified file 'tests/vmtests/test_network.py'
--- tests/vmtests/test_network.py	2017-02-01 20:32:13 +0000
+++ tests/vmtests/test_network.py	2017-05-12 19:12:20 +0000
@@ -2,7 +2,6 @@
 from .releases import base_vm_classes as relbase
 
 import ipaddress
-import os
 import re
 import textwrap
 import yaml
@@ -46,9 +45,8 @@
         ])
 
     def test_etc_network_interfaces(self):
-        with open(os.path.join(self.td.collect, "interfaces")) as fp:
-            eni = fp.read()
-            logger.debug('etc/network/interfaces:\n{}'.format(eni))
+        eni = self.load_collect_file("interfaces")
+        logger.debug('etc/network/interfaces:\n{}'.format(eni))
 
         expected_eni = self.get_expected_etc_network_interfaces()
         eni_lines = eni.split('\n')
@@ -56,9 +54,8 @@
             self.assertTrue(line in eni_lines)
 
     def test_etc_resolvconf(self):
-        with open(os.path.join(self.td.collect, "resolv.conf")) as fp:
-            resolvconf = fp.read()
-            logger.debug('etc/resolv.conf:\n{}'.format(resolvconf))
+        resolvconf = self.load_collect_file("resolv.conf")
+        logger.debug('etc/resolv.conf:\n{}'.format(resolvconf))
 
         resolv_lines = resolvconf.split('\n')
         logger.debug('resolv.conf lines:\n{}'.format(resolv_lines))
@@ -140,34 +137,30 @@
         logger.debug('expected_network_state:\n{}'.format(
             yaml.dump(network_state, default_flow_style=False, indent=4)))
 
-        with open(os.path.join(self.td.collect, "ip_a")) as fp:
-            ip_a = fp.read()
-            logger.debug('ip a:\n{}'.format(ip_a))
+        ip_a = self.load_collect_file("ip_a")
+        logger.debug('ip a:\n{}'.format(ip_a))
 
         ip_dict = helpers.ip_a_to_dict(ip_a)
         print('parsed ip_a dict:\n{}'.format(
             yaml.dump(ip_dict, default_flow_style=False, indent=4)))
 
-        with open(os.path.join(self.td.collect, "ip_route_show")) as fp:
-            ip_route_show = fp.read()
-            logger.debug("ip route show:\n{}".format(ip_route_show))
-            for line in [line for line in ip_route_show.split('\n')
-                         if 'src' in line]:
-                m = re.search(r'^(?P<network>\S+)\sdev\s' +
-                              r'(?P<devname>\S+)\s+' +
-                              r'proto kernel\s+scope link' +
-                              r'\s+src\s(?P<src_ip>\S+)',
-                              line)
-                route_info = m.groupdict('')
-                logger.debug(route_info)
-
-        with open(os.path.join(self.td.collect, "route_n")) as fp:
-            route_n = fp.read()
-            logger.debug("route -n:\n{}".format(route_n))
-
-        with open(os.path.join(self.td.collect, "route_6_n")) as fp:
-            route_6_n = fp.read()
-            logger.debug("route -6 -n:\n{}".format(route_6_n))
+        ip_route_show = self.load_collect_file("ip_route_show")
+        logger.debug("ip route show:\n{}".format(ip_route_show))
+        for line in [line for line in ip_route_show.split('\n')
+                     if 'src' in line]:
+            m = re.search(r'^(?P<network>\S+)\sdev\s' +
+                          r'(?P<devname>\S+)\s+' +
+                          r'proto kernel\s+scope link' +
+                          r'\s+src\s(?P<src_ip>\S+)',
+                          line)
+            route_info = m.groupdict('')
+            logger.debug(route_info)
+
+        route_n = self.load_collect_file("route_n")
+        logger.debug("route -n:\n{}".format(route_n))
+
+        route_6_n = self.load_collect_file("route_6_n")
+        logger.debug("route -6 -n:\n{}".format(route_6_n))
 
         routes = {
             '4': route_n,

=== modified file 'tests/vmtests/test_network_bridging.py'
--- tests/vmtests/test_network_bridging.py	2017-02-08 20:51:48 +0000
+++ tests/vmtests/test_network_bridging.py	2017-05-12 19:12:20 +0000
@@ -3,7 +3,6 @@
 from .test_network import TestNetworkBaseTestsAbs
 from curtin import util
 
-import os
 import textwrap
 
 
@@ -107,22 +106,17 @@
                                  "sysfs_br0_eth2"])
 
     def test_bridge_utils_installed(self):
-        with open(os.path.join(self.td.collect,
-                               "bridge-utils_installed")) as fp:
-            status = fp.read().strip()
-            logger.debug('bridge-utils installed: {}'.format(status))
-            self.assertEqual('install ok installed', status)
+        status = self.load_collect_file("bridge-utils_installed").strip()
+        logger.debug('bridge-utils installed: {}'.format(status))
+        self.assertEqual('install ok installed', status)
 
     def test_bridge_params(self):
         """ Test if configure bridge params match values on the device """
 
         def _load_sysfs_bridge_data():
-            sysfs_br0 = sysfs_to_dict(os.path.join(self.td.collect,
-                                                   "sysfs_br0"))
-            sysfs_br0_eth1 = sysfs_to_dict(os.path.join(self.td.collect,
-                                                        "sysfs_br0_eth1"))
-            sysfs_br0_eth2 = sysfs_to_dict(os.path.join(self.td.collect,
-                                                        "sysfs_br0_eth2"))
+            sysfs_br0 = sysfs_to_dict(self.collect_path("sysfs_br0"))
+            sysfs_br0_eth1 = sysfs_to_dict(self.collect_path("sysfs_br0_eth1"))
+            sysfs_br0_eth2 = sysfs_to_dict(self.collect_path("sysfs_br0_eth2"))
             return {
                 'br0': sysfs_br0,
                 'eth1': sysfs_br0_eth1,

=== modified file 'tests/vmtests/test_network_enisource.py'
--- tests/vmtests/test_network_enisource.py	2017-02-07 16:52:56 +0000
+++ tests/vmtests/test_network_enisource.py	2017-05-12 19:12:20 +0000
@@ -2,7 +2,6 @@
 from .releases import base_vm_classes as relbase
 from .test_network import TestNetworkBaseTestsAbs
 
-import os
 import subprocess
 import yaml
 
@@ -38,7 +37,7 @@
         """ Compare injected configuration as parsed by curtin matches
             how ifup configured the interface."""
         # interfaces uses absolute paths, fix for test-case
-        interfaces = os.path.join(self.td.collect, "interfaces")
+        interfaces = self.collect_path("interfaces")
         cmd = ['sed', '-i.orig', '-e', 's,/etc/network/,,g',
                '{}'.format(interfaces)]
         subprocess.check_call(cmd, stderr=subprocess.STDOUT)

=== modified file 'tests/vmtests/test_network_mtu.py'
--- tests/vmtests/test_network_mtu.py	2017-02-01 20:32:13 +0000
+++ tests/vmtests/test_network_mtu.py	2017-05-12 19:12:20 +0000
@@ -1,8 +1,6 @@
 from .releases import base_vm_classes as relbase
 from .test_network_ipv6 import TestNetworkIPV6Abs
-from curtin import util
 
-import os
 import textwrap
 
 
@@ -46,8 +44,7 @@
         }
         mtu_val = {}
         for fnk in mtu_fn.keys():
-            fn = os.path.join(self.td.collect, mtu_fn[fnk])
-            mtu_val.update({fnk: int(util.load_file(fn))})
+            mtu_val.update({fnk: int(self.load_collect_file(mtu_fn[fnk]))})
 
         return mtu_val
 

=== modified file 'tests/vmtests/test_nvme.py'
--- tests/vmtests/test_nvme.py	2017-04-10 16:50:47 +0000
+++ tests/vmtests/test_nvme.py	2017-05-12 19:12:20 +0000
@@ -42,7 +42,7 @@
         self.output_files_exist(["ls_nvme", "ls_dname", "ls_dev_nvme"])
 
     def test_nvme_device_names(self):
-        ls_nvme = os.path.join(self.td.collect, 'ls_nvme')
+        ls_nvme = self.collect_path('ls_nvme')
         # trusty and vivid do not have sys/class/nvme but
         # nvme devices do work
         if os.path.getsize(ls_nvme) > 0: