From ca1fc41f9e7dc976066505edabe9bea256fdcc38 Mon Sep 17 00:00:00 2001
From: Francis Ginther <francis.ginther@canonical.com>
Date: Fri, 5 Jun 2015 11:08:48 -0500
Subject: [PATCH] Refactor the __fetch_manifest() code so that the a successful
 response is immediately handled and we don't end up with an undefined
 'response' on an exception. Also update the tests to return two mocked
 reponses to match the size of the url_list.

---
 boottest.py            | 20 ++++++++++----------
 tests/test_boottest.py | 10 ++++++++--
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/boottest.py b/boottest.py
index 87d122c..a1bb2cf 100644
--- a/boottest.py
+++ b/boottest.py
@@ -92,21 +92,21 @@ class TouchManifest(object):
             try:
                 response = urllib.urlopen(url)
                 if response.code == 200:
+                    # Only [re]create the manifest file if one was successfully
+                    # downloaded. This allows for an existing image to be used
+                    # if the download fails.
+                    path_dir = os.path.dirname(self.path)
+                    if not os.path.exists(path_dir):
+                        os.makedirs(path_dir)
+                    with open(self.path, 'w') as fp:
+                        fp.write(response.read())
+                    success = True
                     break
+
             except IOError as e:
                 print("W: [%s] - error connecting to %s: %s" % (
                         time.asctime(), self.path, e))
 
-        # Only [re]create the manifest file if one was successfully downloaded
-        # this allows for an existing image to be used if the download fails.
-        if response.code == 200:
-            path_dir = os.path.dirname(self.path)
-            if not os.path.exists(path_dir):
-                os.makedirs(path_dir)
-            with open(self.path, 'w') as fp:
-                fp.write(response.read())
-            success = True
-
         return success
 
     def _load(self):
diff --git a/tests/test_boottest.py b/tests/test_boottest.py
index d001e96..9e4014a 100644
--- a/tests/test_boottest.py
+++ b/tests/test_boottest.py
@@ -49,7 +49,10 @@ class TestTouchManifest(unittest.TestCase):
         self.addCleanup(shutil.rmtree, self.path)
         _p = mock.patch('urllib.urlopen')
         self.mocked_urlopen = _p.start()
-        self.mocked_urlopen.side_effect = [FakeResponse(code=404),]
+        self.mocked_urlopen.side_effect = [
+            FakeResponse(code=404),
+            FakeResponse(code=404),
+        ]
         self.addCleanup(_p.stop)
         self.fetch_retries_orig = boottest.FETCH_RETRIES
         def restore_fetch_retries():
@@ -90,7 +93,10 @@ class TestTouchManifest(unittest.TestCase):
         self.assertIn('foo', manifest)
 
     def test_fetch_exception(self):
-        self.mocked_urlopen.side_effect = [IOError("connection refused")]
+        self.mocked_urlopen.side_effect = [
+            IOError("connection refused"),
+            IOError("connection refused"),
+        ]
         manifest = boottest.TouchManifest('not-real', 'not-real')
         self.assertEqual(0, len(manifest._manifest))