Merge lp:~bcsaller/charm-tools/test-minimal-working-set into lp:charm-tools/1.2

[Benjamin Saller]

Don't bootstrap for non-executable tests

This makes an effort to skip tests that can't exec. It does
change the policy that tests that expect to execute must
be executable, but we don't know the interpreter w/o that
anyway.

This also moves the find_test tests from being all mocks
to using real code which is a win IMO.

https://codereview.appspot.com/54870043/

[Benjamin Saller (bcsaller)]

Reviewers: mp+202400_code.launchpad.net,

Message:
Please take a look.

Description:
Don't bootstrap for non-executable tests

This makes an effort to skip tests that can't exec. It does
change the policy that tests that expect to execute must
be executable, but we don't know the interpreter w/o that
anyway.

This also moves the find_test tests from being all mocks
to using real code which is a win IMO.

https://code.launchpad.net/~bcsaller/charm-tools/test-minimal-working-set/+merge/202400

(do not edit description out of merge proposal)

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

Affected files (+61, -46 lines):
   A [revision details]
   M charmtools/test.py
   M tests/test_juju_test.py
   A tests_functional/charms/test/tests/00_setup
   A tests_functional/charms/test/tests/helper.bash

[Marco Ceppi (marcoceppi)]

On 2014/01/21 06:08:01, bcsaller wrote:
> Please take a look.

LGTM +1

https://codereview.appspot.com/54870043/

[Benjamin Saller (bcsaller)]

*** Submitted:

Don't bootstrap for non-executable tests

This makes an effort to skip tests that can't exec. It does
change the policy that tests that expect to execute must
be executable, but we don't know the interpreter w/o that
anyway.

This also moves the find_test tests from being all mocks
to using real code which is a win IMO.

R=Marco Ceppi
CC=
https://codereview.appspot.com/54870043

https://codereview.appspot.com/54870043/

=== modified file 'charmtools/test.py'
--- charmtools/test.py	2014-01-15 00:18:41 +0000
+++ charmtools/test.py	2014-01-21 06:07:38 +0000
@@ -12,6 +12,7 @@
 import time
 import yaml
 from datetime import timedelta
+from collections import OrderedDict
 from contextlib import contextmanager
 
 TEST_PASS = '✔'
@@ -64,35 +65,30 @@
         self.args = arguments
         self.env = {'JUJU_HOME': os.path.expanduser('~/.juju')}
         self.log = logging.getLogger('juju-test.conductor')
-        self.tests = self.args.tests or self.find_tests()
-        self.tests_supplied = bool(self.args.tests)
+        self.tests = self.find_tests()
+        self.tests_requested = self.args.tests
         self.juju_version = None
         self.juju_env = self.args.juju_env
         self.errors = 0
         self.fails = 0
         self.passes = 0
 
+        if self.tests_requested:
+            self.tests_requested = [os.path.basename(t) for t in
+                                    self.tests_requested]
+
         if not self.tests:
             raise NoTests()
 
     def run(self):
         self.juju_version = get_juju_version()
-        available_tests = self.find_tests()
-
-        for test in self.tests:
-            if not os.path.basename(test) in available_tests:
-                self.log.error('%s is not an available test. Skipping' % test)
-                self.errors += 1
-                continue
-
-            if not os.path.isfile(test):
-                if not os.path.isfile(os.path.join('tests', test)):
-                    self.log.error('%s was not found. Skipping' % test)
-                    self.errors += 1
-                    continue
-                else:
-                    test = os.path.join('tests', test)
-
+        requested_tests = self.tests
+        if self.tests_requested:
+            for test in self.tests:
+                if test not in self.tests_requested:
+                    del self.tests[test]
+
+        for test in requested_tests.values():
             try:
                 self.bootstrap(self.juju_env, self.args.setup_timeout)
             except Exception, e:
@@ -130,9 +126,16 @@
             return None
 
         # Filter out only the files in tests/ then get the test names.
-        tests = [os.path.basename(t) for t in tests_dir if os.path.isfile(t)]
+        tests = [t for t in tests_dir if os.path.isfile(t)]
+        # only include executables
+        tests = [(os.path.basename(t), t) for t in tests if
+                 os.access(t, os.R_OK | os.X_OK)]
 
-        return sorted(tests)
+        result = OrderedDict()
+        # keep sort order as well as indexed lookups
+        for basename, test in sorted(tests):
+            result[basename] = test
+        return result
 
     def safe_test_name(self, test_name):
         return test_name
@@ -615,6 +618,7 @@
     # TODO: Need to add some way to specify a particular test without
     #       colliding with CHARM
     parser.add_argument('tests', metavar="TEST", nargs='*',
+                        default=None,
                         help="tests to execute, relative to tests/ directory, \
                               default is all")
     return parser

=== modified file 'tests/test_juju_test.py'
--- tests/test_juju_test.py	2014-01-15 00:18:41 +0000
+++ tests/test_juju_test.py	2014-01-21 06:07:38 +0000
@@ -4,6 +4,7 @@
 import unittest
 import yaml
 
+from contextlib import contextmanager
 from charmtools import test as juju_test
 from mock import patch, call, Mock, MagicMock
 from StringIO import StringIO
@@ -30,6 +31,25 @@
 
 PARSED_ENVIRONMENTS_YAML = yaml.safe_load(RAW_ENVIRONMENTS_YAML)
 
+@contextmanager
+def cd(directory):
+    """A context manager to temporarily change current working dir, e.g.::
+
+        >>> import os
+        >>> os.chdir('/tmp')
+        >>> with cd('/bin'): print os.getcwd()
+        /bin
+        >>> print os.getcwd()
+        /tmp
+    """
+    cwd = os.getcwd()
+    os.chdir(directory)
+    try:
+        yield
+    finally:
+        os.chdir(cwd)
+
+
 
 class Arguments(object):
     def __init__(self, **kwargs):
@@ -82,32 +102,18 @@
         self.assertEqual(60, juju_test.convert_to_timedelta('1m'))
         self.assertEqual(60 * 60, juju_test.convert_to_timedelta('1h'))
 
-    @patch('glob.glob')
-    @patch('os.path.isfile')
-    def test_conductor_find_tests(self, mock_isfile, mock_glob):
-        tests_directory = ['tests/00-test', 'tests/02-juju', 'tests/lib',
-                           'tests/01-ubuntu']
-        test_names = ['00-test', '01-ubuntu', '02-juju']
-        files_exist = [True, True, False, True]
-        mock_isfile.side_effect = files_exist
-        mock_glob.return_value = tests_directory
-
-        args = Arguments(tests='dummy')
-        c = juju_test.Conductor(args)
-        results = c.find_tests()
-
-        mock_glob.assert_called_with('tests/*')
-        self.assertEqual(results, test_names)
-
-    @patch('glob.glob')
-    def test_conductor_find_tests_fails(self, mock_glob):
-        mock_glob.return_value = []
-
-        args = Arguments(tests='dummy')
-        c = juju_test.Conductor(args)
-        results = c.find_tests()
-
-        self.assertEqual(results, None)
+    def test_conductor_find_tests(self):
+        with cd('tests_functional/charms/test/'):
+            args = Arguments(tests="dummy")
+            c = juju_test.Conductor(args)
+            results = c.find_tests()
+            self.assertIn('00_setup', results)
+            self.assertNotIn('helpers.bash', results)
+
+    def test_conductor_find_tests_fails(self):
+        with cd('tests_functional/charms/mod-spdy/'):
+            args = Arguments(tests="dummy")
+            self.assertRaises(juju_test.NoTests, juju_test.Conductor, args)
 
     @patch.object(juju_test.Conductor, 'find_tests')
     def test_conductor_find_tests_exception(self, mfind_tests):

=== added directory 'tests_functional/charms/test/tests'
=== added file 'tests_functional/charms/test/tests/00_setup'
--- tests_functional/charms/test/tests/00_setup	1970-01-01 00:00:00 +0000
+++ tests_functional/charms/test/tests/00_setup	2014-01-21 06:07:38 +0000
@@ -0,0 +1,2 @@
+#!/bin/bash
+exit 0

=== added file 'tests_functional/charms/test/tests/helper.bash'
--- tests_functional/charms/test/tests/helper.bash	1970-01-01 00:00:00 +0000
+++ tests_functional/charms/test/tests/helper.bash	2014-01-21 06:07:38 +0000
@@ -0,0 +1,1 @@
+# non executable