Merge lp:~marcoceppi/charm-tools/fix-juju-test into lp:charm-tools/1.2

[Marco Ceppi]

Fixed juju test load point and destroy-environment

https://codereview.appspot.com/52460043/

[Marco Ceppi (marcoceppi)]

Reviewers: mp+201694_code.launchpad.net,

Message:
Please take a look.

Description:
Fixed juju test load point and destroy-environment

https://code.launchpad.net/~marcoceppi/charm-tools/fix-juju-test/+merge/201694

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/52460043/

Affected files (+23, -6 lines):
   A [revision details]
   M charmtools/test.py
   M setup.py
   M tests/test_juju_test.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: setup.py
=== modified file 'setup.py'
--- setup.py 2014-01-14 03:21:31 +0000
+++ setup.py 2014-01-15 00:18:41 +0000
@@ -34,7 +34,7 @@
              'charm = charmtools:charm',
              'juju-charm = charmtools:charm',
              'juju-bundle = charmtools:bundle',
- 'juju-test = charmtools:test',
+ 'juju-test = charmtools.test:main',
              'charm-get = charmtools.get:main',
              'charm-getall = charmtools.getall:main',
              'charm-proof = charmtools.proof:main',
@@ -49,7 +49,7 @@
              'charm-update = charmtools.update:main',
              'charm-version = charmtools.version:main',
              'charm-help = charmtools.cli:usage',
- 'charm-test = charmtools:test',
+ 'charm-test = charmtools.test:main',
              'charm-info = charmtools.info:main',
              'charm-generate = charmtools.generate:main',
              'charm-add = charmtools.generate:main',

Index: charmtools/test.py
=== modified file 'charmtools/test.py'
--- charmtools/test.py 2014-01-09 20:15:59 +0000
+++ charmtools/test.py 2014-01-15 00:18:41 +0000
@@ -184,8 +184,14 @@

      def destroy(self, juju_env):
          self.log.debug('Tearing down %s juju environment' % juju_env)
- cmd = ['juju', 'destroy-environment', '-e', juju_env]
+ cmd = ['juju', 'destroy-environment']
+
          if self.juju_version.major > 0:
+ if self.juju_version.minor < 17:
+ cmd.extend(['-y', '-e', juju_env])
+ else:
+ cmd.extend(['-y', juju_env])
+
              self.log.debug('Calling "%s"' % ' '.join(cmd))
              try:
                  subprocess.check_call(cmd)
@@ -194,6 +200,7 @@
          else:
              # Probably should use Popen instead of Shell=True. I'm just not
              # confident on properly mocking a Popen call just yet.
+ cmd.extend(['-e', juju_env])
              pycmd = 'echo y | %s' % ' '.join(cmd)
              self.log.debug('Calling "%s"' % pycmd)
              try:
@@ -234,8 +241,8 @@
                  if status['machines'][0]['agent-state'] == 'running':
                      bootstrapped = True

- def load_environments_yaml(self, juju_home='~'):
- env_yaml_file =
os.path.join(os.path.expanduser(juju_home), '.juju',
+ def load_environments_yaml(self, juju_home='~/.juju'):
+ env_yaml_file = os.path.join(os.path.expandus...

[Benjamin Saller (bcsaller)]

LGTM

Thanks for getting these fixes in.

https://codereview.appspot.com/52460043/diff/1/charmtools/test.py
File charmtools/test.py (right):

https://codereview.appspot.com/52460043/diff/1/charmtools/test.py#newcode634
charmtools/test.py:634: test_cfg = None
Thanks

https://codereview.appspot.com/52460043/diff/1/tests/test_juju_test.py
File tests/test_juju_test.py (left):

https://codereview.appspot.com/52460043/diff/1/tests/test_juju_test.py#oldcode207
tests/test_juju_test.py:207:
Thanks for adding this.

https://codereview.appspot.com/52460043/

[Marco Ceppi (marcoceppi)]

*** Submitted:

Fixed juju test load point and destroy-environment

R=benjamin.saller
CC=
https://codereview.appspot.com/52460043

https://codereview.appspot.com/52460043/

=== modified file 'charmtools/test.py'
--- charmtools/test.py	2014-01-09 20:15:59 +0000
+++ charmtools/test.py	2014-01-15 00:20:52 +0000
@@ -184,8 +184,14 @@
 
     def destroy(self, juju_env):
         self.log.debug('Tearing down %s juju environment' % juju_env)
-        cmd = ['juju', 'destroy-environment', '-e', juju_env]
+        cmd = ['juju', 'destroy-environment']
+
         if self.juju_version.major > 0:
+            if self.juju_version.minor < 17:
+                cmd.extend(['-y', '-e', juju_env])
+            else:
+                cmd.extend(['-y', juju_env])
+
             self.log.debug('Calling "%s"' % ' '.join(cmd))
             try:
                 subprocess.check_call(cmd)
@@ -194,6 +200,7 @@
         else:
             # Probably should use Popen instead of Shell=True. I'm just not
             # confident on properly mocking a Popen call just yet.
+            cmd.extend(['-e', juju_env])
             pycmd = 'echo y | %s' % ' '.join(cmd)
             self.log.debug('Calling "%s"' % pycmd)
             try:
@@ -234,8 +241,8 @@
                 if status['machines'][0]['agent-state'] == 'running':
                     bootstrapped = True
 
-    def load_environments_yaml(self, juju_home='~'):
-        env_yaml_file = os.path.join(os.path.expanduser(juju_home), '.juju',
+    def load_environments_yaml(self, juju_home='~/.juju'):
+        env_yaml_file = os.path.join(os.path.expanduser(juju_home),
                                      'environments.yaml')
         if not os.path.exists(env_yaml_file):
             raise IOError("%s file does not exist" % env_yaml_file)
@@ -624,6 +631,7 @@
 def main():
     p = setup_parser()
     args = p.parse_args()
+    test_cfg = None
     if args.description:
         print p.description
         sys.exit()
 
=== modified file 'setup.py'
--- setup.py	2014-01-14 03:21:31 +0000
+++ setup.py	2014-01-15 00:20:52 +0000
@@ -34,7 +34,7 @@
             'charm = charmtools:charm',
             'juju-charm = charmtools:charm',
             'juju-bundle = charmtools:bundle',
-            'juju-test = charmtools:test',
+            'juju-test = charmtools.test:main',
             'charm-get = charmtools.get:main',
             'charm-getall = charmtools.getall:main',
             'charm-proof = charmtools.proof:main',
@@ -49,7 +49,7 @@
             'charm-update = charmtools.update:main',
             'charm-version = charmtools.version:main',
             'charm-help = charmtools.cli:usage',
-            'charm-test = charmtools:test',
+            'charm-test = charmtools.test:main',
             'charm-info = charmtools.info:main',
             'charm-generate = charmtools.generate:main',
             'charm-add = charmtools.generate:main',
 
=== modified file 'tests/test_juju_test.py'
--- tests/test_juju_test.py	2014-01-08 21:35:31 +0000
+++ tests/test_juju_test.py	2014-01-15 00:20:52 +0000
@@ -203,7 +203,14 @@
         c.destroy(good_env)
 
         mock_check_call.assert_called_once_with(['juju', 'destroy-environment',
-                                                 '-e', good_env])
+                                                 '-y', '-e', good_env])
+
+        mock_check_call.reset_mock()
+        c.juju_version = juju_test.JujuVersion(major=1, minor=17, patch=0)
+        c.destroy(good_env)
+
+        mock_check_call.assert_called_once_with(['juju', 'destroy-environment',
+                                                 '-y', good_env])
 
         mock_check_call.reset_mock()
         c.juju_version = juju_test.JujuVersion(major=0, minor=8, patch=0)