Merge lp:~marcoceppi/charm-helpers/lp-1591968 into lp:charm-helpers

[Marco Ceppi]

Do not create dest_dir, but instead create dest (dirname of dest_dir).

[lp:~marcoceppi/charm-helpers/lp-1591968]

583. By Marco Ceppi <marco@T430> on 2016-06-13

test coverage

[Stuart Bishop (stub)]

Looks good. Unrelated tests are failing (bug reported), but that shouldn't block this.

review: Approve

=== modified file 'charmhelpers/fetch/bzrurl.py'
--- charmhelpers/fetch/bzrurl.py	2015-12-10 21:01:00 +0000
+++ charmhelpers/fetch/bzrurl.py	2016-06-13 13:53:01 +0000
@@ -59,8 +59,9 @@
             dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched",
                                     branch_name)
 
-        if not os.path.exists(dest_dir):
-            mkdir(dest_dir, perms=0o755)
+        if dest and not os.path.exists(dest):
+            mkdir(dest, perms=0o755)
+
         try:
             self.branch(source, dest_dir)
         except OSError as e:

=== modified file 'tests/fetch/test_bzrurl.py'
--- tests/fetch/test_bzrurl.py	2015-12-10 20:53:41 +0000
+++ tests/fetch/test_bzrurl.py	2016-06-13 13:53:01 +0000
@@ -94,14 +94,26 @@
             if dst:
                 shutil.rmtree(dst, ignore_errors=True)
 
+    def test_installs(self):
+        self.fh.branch = MagicMock()
+
+        for url in self.valid_urls:
+            branch_name = urlparse(url).path.strip("/").split("/")[-1]
+            dest = os.path.join('foo', 'fetched')
+            dest_dir = os.path.join(dest, os.path.basename(branch_name))
+            with patch.dict('os.environ', {'CHARM_DIR': 'foo'}):
+                where = self.fh.install(url)
+            self.assertEqual(where, dest_dir)
+
     @patch('charmhelpers.fetch.bzrurl.mkdir')
-    def test_installs(self, _mkdir):
+    def test_installs_dir(self, _mkdir):
         self.fh.branch = MagicMock()
 
         for url in self.valid_urls:
             branch_name = urlparse(url).path.strip("/").split("/")[-1]
-            dest = os.path.join('foo', 'fetched', os.path.basename(branch_name))
+            dest = os.path.join('opt', 'f')
+            dest_dir = os.path.join(dest, os.path.basename(branch_name))
             with patch.dict('os.environ', {'CHARM_DIR': 'foo'}):
-                where = self.fh.install(url)
-            self.assertEqual(where, dest)
-            _mkdir.assert_called_with(where, perms=0o755)
+                where = self.fh.install(url, dest)
+            self.assertEqual(where, dest_dir)
+            _mkdir.assert_called_with(dest, perms=0o755)