From c6174f8068aa7018bc6675787397d6dec3e63f28 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Nov 09 2016 16:24:55 +0000
Subject: Handle RC milestone composes correctly


The CLI and `get_release` did not handle composes for the 'RC'
milestone correctly. With Pungi 4, the milestone for GA composes
is now 'RC', not 'Final'; we were handling this OK when parsing
a label or compose ID, I think, but the CLI and `get_release`
in old-style 'release, milestone, compose' usage were both
expecting 'Final' as the milestone (that's what we called these
before Pungi 4). So have `get_release` check for 'Final' and
convert it to 'RC', and have the CLI accept both 'Final' and
'RC' as synonyms. Tweak a test to cover this, and improve the
CLI help text while we're at it.

---

diff --git a/fedfind/cli.py b/fedfind/cli.py
index 0eb7c72..c00595f 100755
--- a/fedfind/cli.py
+++ b/fedfind/cli.py
@@ -133,8 +133,14 @@ def parse_args():
         type=_release, required=False, metavar="1-99 or Rawhide")
     parser_images.add_argument(
         '-m', '--milestone', help="A milestone to search (e.g. Alpha or "
-        "Beta)", choices=['Alpha', 'Beta', 'Final', 'Atomic', 'Branched',
-                          'Rawhide', 'Production'])
+        "Beta). 'Final' and 'RC' are synonyms. 'Atomic' is for two-week "
+        "nightly Atomic composes. 'Branched' and 'Rawhide' are the two "
+        "mainline nightly compose types, see the wiki for more details. "
+        "'Production' is for candidate composes on the compose server "
+        "(kojipkgs) and identified by a date and respin, as opposed to the "
+        "same composes synced to alt and identified by label.",
+        choices=['Alpha', 'Beta', 'Final', 'RC', 'Atomic', 'Branched',
+                 'Rawhide', 'Production'])
     parser_images.add_argument(
         '-c', '--compose', '--date', help="A compose or date to "
         "search, e.g. 1 or 20160314. You may also pass a compose and respin "
diff --git a/fedfind/release.py b/fedfind/release.py
index 784aa9b..cfe0c88 100644
--- a/fedfind/release.py
+++ b/fedfind/release.py
@@ -138,8 +138,8 @@ def get_release(release='', milestone='', compose='', respin=None,
 
     For milestone releases (Alpha/Beta), specify the milestone. We
     will guess the release in the same way as for TCs/RCs if it is not
-    passed. Note that milestone 'Final' will be treated the same way
-    as no milestone at all, i.e. returning a stable release.
+    passed. Note that milestone 'RC' / 'Final' will be treated the
+    same way as no milestone at all, i.e. returning a stable release.
 
     As things stand right now, TCs/RCs and Alpha/Beta releases are
     not actually handled, as Fedora has not yet decided how to handle
@@ -388,6 +388,11 @@ def get_release(release='', milestone='', compose='', respin=None,
     compose = guess_compose(release, milestone, compose)
     release = guess_release(release, milestone, compose)
 
+    # Handle pre-Pungi 4 'Final' milestone; in Pungi 4 world, 'RC' is
+    # the milestone for these
+    if str(milestone).lower() == 'final':
+        milestone = 'RC'
+
     if respin is None and '.' in compose:
         # as a bit of fudge, we'll see if we got a compose value that
         # looks like it includes a respin, and parse it out if so.
@@ -452,7 +457,7 @@ def get_release(release='', milestone='', compose='', respin=None,
                          "compose!")
 
     # Non-final milestones.
-    if milestone and str(milestone) != 'Final':
+    if milestone and str(milestone).lower() != 'rc':
         return Milestone(release, milestone)
 
     # Anything that makes it this far must be a stable release.
@@ -1438,7 +1443,7 @@ class Milestone(MirrorRelease):
     @cached_property
     def previous_release(self):
         """We're only going to bother handling the current convention
-        (Alpha, Beta, Final), otherwise we'd need a big list of the
+        (Alpha, Beta, RC/Final), otherwise we'd need a big list of the
         rules older releases followed and it's really not worth it.
         The result may well be a lie for an old release. For Alpha we
         return the previous stable release, not the previous Beta."""
diff --git a/tests/test_release.py b/tests/test_release.py
index 9887ffc..cf43cf9 100644
--- a/tests/test_release.py
+++ b/tests/test_release.py
@@ -376,15 +376,16 @@ class TestRelease:
     @mock.patch('fedfind.release.Compose.exists', True)
     def test_get_release_compose(self):
         """A production/candidate compose. This tests the case where
-        the compose is found on the mirror system.
+        the compose is found on the mirror system. Also tests that
+        'Final' is converted to 'RC'.
         """
-        ret = fedfind.release.get_release(24, 'Beta', 1, 1)
+        ret = fedfind.release.get_release(24, 'Final', 1, 1)
         assert isinstance(ret, fedfind.release.Compose)
         assert ret.release == '24'
-        assert ret.milestone == 'Beta'
+        assert ret.milestone == 'RC'
         assert ret.compose == '1'
         assert ret.respin == '1'
-        assert ret.label == 'Beta-1.1'
+        assert ret.label == 'RC-1.1'
 
     @mock.patch('fedfind.helpers.cid_from_label', return_value='Fedora-24-20160316.3')
     @mock.patch('fedfind.release.Compose.exists', False)