Merge lp:~marcoceppi/charm-tools/require-series into lp:charm-tools/1.3

[Marco Ceppi]

Makes series flag required for charm promulgate

[Tim Van Steenburgh (tvansteenburgh)]

Syntax error (see inline diff comment). Also it would be nice to add a unit test for get_lp_charm_series().

review: Needs Fixing

[lp:~marcoceppi/charm-tools/require-series]

328. By Marco Ceppi on 2014-06-18

Added unit tests, fixed missing ,

329. By Marco Ceppi on 2014-06-18

I am a giddy goat

[Tim Van Steenburgh (tvansteenburgh)]

+1 LGTM!

review: Approve

=== modified file 'charmtools/promulgate.py'
--- charmtools/promulgate.py	2013-12-02 20:25:28 +0000
+++ charmtools/promulgate.py	2014-06-18 19:48:09 +0000
@@ -19,22 +19,18 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-from launchpadlib.launchpad import Launchpad
-from lazr.restfulclient.errors import BadRequest, NotFound
-
 import os
 import sys
-
-from optparse import OptionParser
-
-from bzrlib import bzrdir
-
 import yaml
-
 import logging
-
 import subprocess
 
+from optparse import OptionParser
+from bzrlib import bzrdir
+
+from launchpadlib.launchpad import Launchpad
+from lazr.restfulclient.errors import BadRequest, NotFound
+
 DISTRIBUTION = 'charms'
 REVIEW_TEAM_NAME = 'charmers'
 OFFICIAL_BRANCH_POCKET = 'Release'
@@ -50,7 +46,7 @@
         'from the bzr configuration if omitted.')
 
     parser.add_option(
-        '-s', '--series', dest='series', default=None,
+        '-s', '--series', dest='series', default=None, required=True,
         help='The distribution series on which to set the official branch. '
         'Defaults to setting it in the current development series.')
 
@@ -151,19 +147,18 @@
 def get_lp_charm_series(lp, series):
     charm_distro = lp.distributions[DISTRIBUTION]
     if series is None:
-        charm_series = charm_distro.current_series
-    else:
-        try:
-            charm_series = charm_distro.getSeries(
-                name_or_version=series)
-        except (BadRequest, NotFound), e:
-            # XXX flacoste 2011-06-15 bug=797917
-            # Should only be NotFound.
-            if e.content.startswith('No such distribution series:'):
-                logging.error("can't find series '%s'", series)
-                raise
-            else:
-                raise
+        raise ValueError("Series can not be None")
+    try:
+        charm_series = charm_distro.getSeries(
+            name_or_version=series)
+    except (BadRequest, NotFound), e:
+        # XXX flacoste 2011-06-15 bug=797917
+        # Should only be NotFound.
+        if e.content.startswith('No such distribution series:'):
+            logging.error("can't find series '%s'", series)
+            raise
+        else:
+            raise
     return charm_series
 
 

=== added file 'tests/test_charm_promulgate.py'
--- tests/test_charm_promulgate.py	1970-01-01 00:00:00 +0000
+++ tests/test_charm_promulgate.py	2014-06-18 19:48:09 +0000
@@ -0,0 +1,51 @@
+#!/usr/bin/python
+
+#    Copyright (C) 2013  Canonical Ltd.
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation, either version 3 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import sys
+import unittest
+
+from mock import Mock
+from charmtools.promulgate import get_lp_charm_series
+
+
+class NotFound(Exception):
+    content = None
+    def __init__(self, content):
+        self.content = content
+
+
+class TestCharmProof(unittest.TestCase):
+    def test_get_lp_charm_series(self):
+        lp = Mock()
+        charms = Mock()
+        charms.getSeries.return_value = "trusty"
+        lp.distributions = {'charms': charms}
+        self.assertEqual('trusty', get_lp_charm_series(lp, 'trusty'))
+
+    def test_get_lp_charm_series_none(self):
+        lp = Mock()
+        charms = Mock()
+        charms.getSeries.return_value = "trusty"
+        lp.distributions = {'charms': charms}
+        self.assertRaises(ValueError, get_lp_charm_series, lp, None)
+
+    def test_get_lp_charm_series_404(self):
+        lp = Mock()
+        charms = Mock()
+        charms.getSeries.side_effect = NotFound('No such distribution series:')
+        lp.distributions = {'charms': charms}
+        self.assertRaises(NotFound, get_lp_charm_series, lp, 'not-series')