Merge lp:~benji/charm-tools/add-versionless-charm-in-bundle-warning into lp:charm-tools/1.2

[Benji York]

Add a bundle lint warning about revisionless charm URLs

Also:
  - added tests for the bundle linter
  - fixed some whitespace to be PEP-8 complient
  - turned a "not XXX in YYY" into the more idomatic "XXX not in YYY"

Add a bundle lint warning about revisionless charm URLs

Also:
  - added tests for the bundle linter
  - fixed some whitespace to be PEP-8 complient
  - turned a "not XXX in YYY" into the more idomatic "XXX not in YYY"

[Marco Ceppi (marcoceppi)]

LGTM

review: Approve

=== modified file 'charmtools/bundles.py'
--- charmtools/bundles.py	2014-01-08 22:43:34 +0000
+++ charmtools/bundles.py	2014-02-18 16:35:09 +0000
@@ -1,12 +1,16 @@
+import glob
+import json
 import os
+import re
 import yaml
-import glob
-import json
 
 from linter import Linter
 from charmworldlib import bundle as cw_bundle
 
 
+charm_url_includes_id = re.compile(r'-\d+$').search
+
+
 class BundleLinter(Linter):
     def validate(self, contents):
         for name, bdata in contents.items():
@@ -19,9 +23,13 @@
 
             if 'services' in bdata:
                 for svc, sdata in bdata['services'].items():
-                    if not 'annotations' in sdata:
+                    if 'annotations' not in sdata:
                         self.warn('%s: %s: No annotations found, will render '
-                                  'bad in GUI' % (name, svc))
+                                  'poorly in GUI' % (name, svc))
+                    if not charm_url_includes_id(sdata['charm']):
+                        self.warn(
+                            '%s: charm URL should include a revision' % svc)
+
             else:
                 if 'inherits' not in bdata:
                     self.err("%s: No services defined" % name)

=== modified file 'scripts/test'
--- scripts/test	2013-12-02 20:19:24 +0000
+++ scripts/test	2014-02-18 16:35:09 +0000
@@ -4,4 +4,4 @@
 then
     extra_args="--pdb-failures"
 fi
-bin/nosetests --exe --with-id $extra_args $@
+bin/nosetests --nocapture --exe --with-id $extra_args $@

=== added file 'tests/test_bundle_proof.py'
--- tests/test_bundle_proof.py	1970-01-01 00:00:00 +0000
+++ tests/test_bundle_proof.py	2014-02-18 16:35:09 +0000
@@ -0,0 +1,91 @@
+#!/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 charmtools.bundles
+import os.path
+import shutil
+import sys
+import tempfile
+import textwrap
+import unittest
+
+
+class TestCharmProof(unittest.TestCase):
+    def setUp(self):
+        self.linter = charmtools.bundles.BundleLinter()
+
+    def test_warn_on_charm_urls_without_revisions(self):
+        self.linter.validate({
+            'my-service': {
+                'services': {
+                    'memcached': {
+                        'charm': 'cs:precise/memcached',
+                        'num_units': 1,
+                    },
+                },
+            }})
+        self.assertIn(
+            'W: memcached: charm URL should include a revision',
+            self.linter.lint)
+
+    def test_no_warning_when_charm_urls_include_revisions(self):
+        self.linter.validate({
+            'my-service': {
+                'services': {
+                    'memcached': {
+                        'charm': 'cs:precise/memcached-99',
+                        'num_units': 1,
+                    },
+                },
+            }})
+        self.assertNotIn(
+            'W: memcached: charm URL should include a revision',
+            self.linter.lint)
+
+    def test_warn_on_missing_annotations(self):
+        self.linter.validate({
+            'my-service': {
+                'services': {
+                    'memcached': {
+                        'charm': 'cs:precise/memcached',
+                        'num_units': 1,
+                    },
+                },
+            }})
+        self.assertIn(
+            'W: my-service: memcached: No annotations found, will render '
+                'poorly in GUI',
+            self.linter.lint)
+
+    def test_no_warning_when_annotations_are_included(self):
+        self.linter.validate({
+            'my-service': {
+                'services': {
+                    'memcached': {
+                        'charm': 'cs:precise/memcached',
+                        'num_units': 1,
+                        'annotations': {
+                            'gui-x': '821.5',
+                            'gui-y': '669.2698359714502',
+                        },
+                    },
+                },
+            }})
+        self.assertNotIn(
+            'W: my-service: memcached: No annotations found, will render '
+                'poorly in GUI',
+            self.linter.lint)

=== modified file 'tests/test_charm_proof.py'
--- tests/test_charm_proof.py	2014-01-10 03:41:58 +0000
+++ tests/test_charm_proof.py	2014-02-18 16:35:09 +0000
@@ -34,7 +34,6 @@
 class TestCharmProof(TestCase):
     def setUp(self):
         self.charm_dir = mkdtemp()
-        self.config_path = join(self.charm_dir, 'config.yaml')
         self.linter = Linter()
 
     def tearDown(self):
@@ -255,5 +254,6 @@
             "for the tag 'tag:yaml.org,2002:python/name:__builtin__.int'")
         self.assertTrue(self.linter.lint[0].startswith(expected))
 
+
 if __name__ == '__main__':
     main()