Move logic from _get_uri_reference...

Into new _choose_reference() method. This is because uri_reference is
irrelevant to ShamanProject, but we still want to use the same rules for
deciding whether to search for ref/tag/branch/sha1 when more than one is
specified.

Signed-off-by: Zack Cerza <zack@redhat.com>

diff --git a/teuthology/packaging.py b/teuthology/packaging.py
index 239de6c44d41755f36b2120edca19aba3a01bb2b..d4985c4dd293876a21eda10278eaf320f798077b 100644 (file)
--- a/teuthology/packaging.py
+++ b/teuthology/packaging.py
@@ -644,6 +644,21 @@ class GitbuilderProject(object):
 
         :returns: A string URI. Ex: ref/master
         """
+        ref_name, ref_val = self._choose_reference().items()[0]
+        if ref_name == 'sha1':
+            return 'sha1/%s' % ref_val
+        else:
+            return 'ref/%s' % ref_val
+
+    def _choose_reference(self):
+        """
+        Since it's only meaningful to search for one of:
+            ref, tag, branch, sha1
+        Decide which to use.
+
+        :returns: a single-key dict containing the name and value of the
+                  reference to use, e.g. {'branch': 'master'}
+        """
         tag = branch = sha1 = None
         if self.remote:
             tag = _get_config_value_for_remote(self.ctx, self.remote,
@@ -665,28 +680,28 @@ class GitbuilderProject(object):
             # filter(None,) filters for truth
             if len(filter(None, vars)) > 1:
                 log.warning(
-                    'More than one of ref, tag, branch, or sha1 supplied; using %s',
-                     attrname
+                    "More than one of ref, tag, branch, or sha1 supplied; "
+                    "using %s",
+                    attrname
                 )
                 for n, v in zip(names, vars):
                     log.info('%s: %s' % (n, v))
 
         if ref:
-            uri = 'ref/' + ref
             warn('ref')
+            return dict(ref=ref)
         elif tag:
-            uri = 'ref/' + tag
             warn('tag')
+            return dict(tag=tag)
         elif branch:
-            uri = 'ref/' + branch
             warn('branch')
+            return dict(branch=branch)
         elif sha1:
-            uri = 'sha1/' + sha1
             warn('sha1')
+            return dict(sha1=sha1)
         else:
             log.warning("defaulting to master branch")
-            uri = getattr(self, 'ref', 'ref/master')
-        return uri
+            return dict(branch='master')
 
     def _get_base_url(self):
         """

diff --git a/teuthology/test/test_packaging.py b/teuthology/test/test_packaging.py
index fe3193fe374e6e5f292a3486c336a97cdcbf901b..9c99739ca683124c8d776145bb293d9bf78eb895 100644 (file)
--- a/teuthology/test/test_packaging.py
+++ b/teuthology/test/test_packaging.py
@@ -386,18 +386,18 @@ class TestBuilderProject(object):
         assert expected_log in caplog.text()
 
     REFERENCE_MATRIX = [
-        ('the_ref', 'the_tag', 'the_branch', 'the_sha1', 'ref/the_ref'),
-        (None, 'the_tag', 'the_branch', 'the_sha1', 'ref/the_tag'),
-        (None, None, 'the_branch', 'the_sha1', 'ref/the_branch'),
-        (None, None, None, 'the_sha1', 'sha1/the_sha1'),
-        (None, None, 'the_branch', None, 'ref/the_branch'),
+        ('the_ref', 'the_tag', 'the_branch', 'the_sha1', dict(ref='the_ref')),
+        (None, 'the_tag', 'the_branch', 'the_sha1', dict(tag='the_tag')),
+        (None, None, 'the_branch', 'the_sha1', dict(branch='the_branch')),
+        (None, None, None, 'the_sha1', dict(sha1='the_sha1')),
+        (None, None, 'the_branch', None, dict(branch='the_branch')),
     ]
 
     @pytest.mark.parametrize(
         "ref, tag, branch, sha1, expected",
         REFERENCE_MATRIX,
     )
-    def test_uri_reference(self, ref, tag, branch, sha1, expected):
+    def test_choose_reference(self, ref, tag, branch, sha1, expected):
         config = dict(
             os_type='ubuntu',
             os_version='16.04',
@@ -411,7 +411,7 @@ class TestBuilderProject(object):
         if sha1:
             config['sha1'] = sha1
         gp = self.klass("ceph", config)
-        assert gp.uri_reference == expected
+        assert gp._choose_reference() == expected
 
     def test_get_package_version_found(self):
         rem = self._get_remote()