Merge lp:~michael.nelson/click-reviewers-tools/add-test-without-mocks into lp:click-reviewers-tools

[Michael Nelson]

Add non-mocked test example.

The purpose of this branch is to provide an example test for a click check which doesn't rely on any mocks. As someone unfamiliar with the code, it's quite difficult to follow everything that is currently mocked for each test, and I think it's much clearer and safer to test without mocks wherever possible.

After adding the initial test, I found it passed when run on it's own, but that there were test isolation issues causing failures when running all tests, related to the (currently) global vars UNPACK_DIR and RAW_UNPACK_DIR.

Note: I'm not using 'click build' to build the package because it will in fact fix issues that we're setting up for tests. Here's an example of the new test *failing* when 'click build' is used, because click build fixes the malicious setup, filtering out the .click directory:

http://paste.ubuntu.com/12796183/

Perhaps there's a way to get click build to not run any checks? Not sure.

Details of changes (in order of diff):
 1) Ensure UNPACK_DIR and RAW_UNPACK_DIR are reset to None when cleanup_unpack() is called (related to test-isolation, see comments)
 2) Moved the creation of all current mock patches from the module level to a function, create_patches, which can be called during setUp (avoids the cyclic import issue mentioned in the deleted comments)
 3) Refactored so that the code for starting and stopping the patches is in the one place - the setUp of the base test case.
 4) Added a new test case without any mocks with a single test.
 5) Added a tests/utils.py with a make_package function for creating potentially invalid packages. The make_package utility is taken from a script we use in myapps. It will need more work.

[Colin Watson (cjwatson)]

"click build" should not gain options to remove this filtering, no; they have no legitimate use outside of exercising pathological cases. This is a case where it's perfectly reasonable to use dpkg-deb in your test data construction.

[Michael Vogt (mvo)]

I really like the new tests/utils.py

review: Approve

[Jamie Strandboge (jdstrand)]

Thank you for this MP. I never really liked how the mocking got so complicated and this should provide a way to move away from that as we move forward. Code looks good. Thanks!

review: Approve

=== modified file 'clickreviews/cr_common.py'
--- clickreviews/cr_common.py	2015-09-28 21:03:58 +0000
+++ clickreviews/cr_common.py	2015-10-16 05:56:07 +0000
@@ -42,9 +42,11 @@
     global UNPACK_DIR
     if UNPACK_DIR is not None and os.path.isdir(UNPACK_DIR):
         recursive_rm(UNPACK_DIR)
+        UNPACK_DIR = None
     global RAW_UNPACK_DIR
     if RAW_UNPACK_DIR is not None and os.path.isdir(RAW_UNPACK_DIR):
         recursive_rm(RAW_UNPACK_DIR)
+        RAW_UNPACK_DIR = None
 atexit.register(cleanup_unpack)
 
 

=== modified file 'clickreviews/cr_tests.py'
--- clickreviews/cr_tests.py	2015-09-28 21:03:58 +0000
+++ clickreviews/cr_tests.py	2015-10-16 05:56:07 +0000
@@ -244,151 +244,131 @@
     return True
 
 
-# http://docs.python.org/3.4/library/unittest.mock-examples.html
-# Mock patching. Don't use decorators but instead patch in setUp() of the
-# child. Set up a list of patches, but don't start them. Create the helper
-# method mock_patch() to start all the patches. The child can do this in a
-# setUp() like so:
-#   import clickreviews.cr_tests as cr_tests
-#   class TestClickReviewFoo(cr_tests.TestClickReview):
-#       def setUp(self):
-#           # Monkey patch various file access classes. stop() is handled with
-#           # addCleanup in super()
-#           cr_tests.mock_patch()
-#           super()
-patches = []
-patches.append(patch('clickreviews.cr_common.ClickReview._check_path_exists',
-               _mock_func))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_control_file',
-    _extract_control_file))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_manifest_file',
-    _extract_manifest_file))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_package_yaml',
-    _extract_package_yaml))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_hashes_yaml',
-    _extract_hashes_yaml))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._path_join',
-    _path_join))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._get_sha512sum',
-    _get_sha512sum))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_statinfo',
-    _extract_statinfo))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._extract_click_frameworks',
-    _extract_click_frameworks))
-patches.append(patch('clickreviews.cr_common.unpack_click', _mock_func))
-patches.append(patch('clickreviews.cr_common.raw_unpack_pkg', _mock_func))
-patches.append(patch('clickreviews.cr_common.ClickReview._list_all_files',
-               _mock_func))
-patches.append(patch(
-    'clickreviews.cr_common.ClickReview._list_all_compiled_binaries',
-    _mock_func))
-
-# lint overrides
-patches.append(patch(
-               'clickreviews.cr_lint.ClickReviewLint._list_control_files',
-               _mock_func))
-patches.append(patch('clickreviews.cr_lint.ClickReviewLint._list_all_files',
-               _mock_func))
-patches.append(patch(
-    'clickreviews.cr_lint.ClickReview._list_all_compiled_binaries',
-    _mock_func))
-patches.append(patch(
-    'clickreviews.cr_lint.ClickReviewLint._extract_readme_md',
-    _extract_readme_md))
-patches.append(patch(
-    'clickreviews.cr_lint.ClickReviewLint._check_innerpath_executable',
-    _check_innerpath_executable))
-
-# security overrides
-patches.append(patch(
-    'clickreviews.cr_security.ClickReviewSecurity._extract_security_manifest',
-    _extract_security_manifest))
-patches.append(patch(
-    'clickreviews.cr_security.ClickReviewSecurity._get_security_manifest',
-    _get_security_manifest))
-patches.append(patch(
-    'clickreviews.cr_security.ClickReviewSecurity._extract_security_profile',
-    _extract_security_profile))
-patches.append(patch(
-    'clickreviews.cr_security.ClickReviewSecurity._get_security_profile',
-    _get_security_profile))
-
-# desktop overrides
-patches.append(patch(
-    'clickreviews.cr_desktop.ClickReviewDesktop._extract_desktop_entry',
-    _extract_desktop_entry))
-patches.append(patch(
-    'clickreviews.cr_desktop.ClickReviewDesktop._get_desktop_entry',
-    _get_desktop_entry))
-patches.append(patch(
-    'clickreviews.cr_desktop.ClickReviewDesktop._extract_webapp_manifests',
-    _extract_webapp_manifests))
-
-# url-dispatcher overrides
-patches.append(patch(
-    'clickreviews.cr_url_dispatcher.ClickReviewUrlDispatcher._extract_url_dispatcher',
-    _extract_url_dispatcher))
-
-# scope overrides
-patches.append(patch(
-    'clickreviews.cr_scope.ClickReviewScope._extract_scopes',
-    _extract_scopes))
-
-# content-hub overrides
-patches.append(patch(
-    'clickreviews.cr_content_hub.ClickReviewContentHub._extract_content_hub',
-    _extract_content_hub))
-
-# online accounts overrides
-patches.append(patch(
-    'clickreviews.cr_online_accounts.ClickReviewAccounts._extract_account',
-    _extract_account))
-
-# push-helper overrides
-patches.append(patch(
-    'clickreviews.cr_push_helper.ClickReviewPushHelper._extract_push_helper',
-    _extract_push_helper))
-
-# bin-path overrides
-patches.append(patch(
-    'clickreviews.cr_bin_path.ClickReviewBinPath._extract_bin_path',
-    _extract_bin_path))
-patches.append(patch(
-    'clickreviews.cr_bin_path.ClickReviewBinPath._check_bin_path_executable',
-    _check_bin_path_executable))
-
-# framework overrides
-patches.append(patch(
-    'clickreviews.cr_framework.ClickReviewFramework._extract_framework',
-    _extract_framework))
-patches.append(patch(
-    'clickreviews.cr_framework.ClickReviewFramework._extract_framework_policy',
-    _extract_framework_policy))
-patches.append(patch(
-    'clickreviews.cr_framework.ClickReviewFramework._has_framework_in_metadir',
-    _has_framework_in_metadir))
-
-
-def mock_patch():
-    '''Call in setup of child'''
-    global patches
-    for p in patches:
-        try:
-            p.start()
-        except ImportError:
-            # This is only needed because we are importing ClickReviewLint
-            # in the security tests and ClickReviewSecurity in the lint tests.
-            # If we move those patches outside of this file, then we can
-            # remove this.
-            pass
+def create_patches():
+    # http://docs.python.org/3.4/library/unittest.mock-examples.html
+    # Mock patching. Don't use decorators but instead patch in setUp() of the
+    # child.
+    patches = []
+    patches.append(patch('clickreviews.cr_common.ClickReview._check_path_exists',
+                   _mock_func))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_control_file',
+        _extract_control_file))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_manifest_file',
+        _extract_manifest_file))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_package_yaml',
+        _extract_package_yaml))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_hashes_yaml',
+        _extract_hashes_yaml))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._path_join',
+        _path_join))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._get_sha512sum',
+        _get_sha512sum))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_statinfo',
+        _extract_statinfo))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._extract_click_frameworks',
+        _extract_click_frameworks))
+    patches.append(patch('clickreviews.cr_common.unpack_click', _mock_func))
+    patches.append(patch('clickreviews.cr_common.raw_unpack_pkg', _mock_func))
+    patches.append(patch('clickreviews.cr_common.ClickReview._list_all_files',
+                   _mock_func))
+    patches.append(patch(
+        'clickreviews.cr_common.ClickReview._list_all_compiled_binaries',
+        _mock_func))
+
+    # lint overrides
+    patches.append(patch(
+                   'clickreviews.cr_lint.ClickReviewLint._list_control_files',
+                   _mock_func))
+    patches.append(patch('clickreviews.cr_lint.ClickReviewLint._list_all_files',
+                   _mock_func))
+    patches.append(patch(
+        'clickreviews.cr_lint.ClickReview._list_all_compiled_binaries',
+        _mock_func))
+    patches.append(patch(
+        'clickreviews.cr_lint.ClickReviewLint._extract_readme_md',
+        _extract_readme_md))
+    patches.append(patch(
+        'clickreviews.cr_lint.ClickReviewLint._check_innerpath_executable',
+        _check_innerpath_executable))
+
+    # security overrides
+    patches.append(patch(
+        'clickreviews.cr_security.ClickReviewSecurity._extract_security_manifest',
+        _extract_security_manifest))
+    patches.append(patch(
+        'clickreviews.cr_security.ClickReviewSecurity._get_security_manifest',
+        _get_security_manifest))
+    patches.append(patch(
+        'clickreviews.cr_security.ClickReviewSecurity._extract_security_profile',
+        _extract_security_profile))
+    patches.append(patch(
+        'clickreviews.cr_security.ClickReviewSecurity._get_security_profile',
+        _get_security_profile))
+
+    # desktop overrides
+    patches.append(patch(
+        'clickreviews.cr_desktop.ClickReviewDesktop._extract_desktop_entry',
+        _extract_desktop_entry))
+    patches.append(patch(
+        'clickreviews.cr_desktop.ClickReviewDesktop._get_desktop_entry',
+        _get_desktop_entry))
+    patches.append(patch(
+        'clickreviews.cr_desktop.ClickReviewDesktop._extract_webapp_manifests',
+        _extract_webapp_manifests))
+
+    # url-dispatcher overrides
+    patches.append(patch(
+        'clickreviews.cr_url_dispatcher.ClickReviewUrlDispatcher._extract_url_dispatcher',
+        _extract_url_dispatcher))
+
+    # scope overrides
+    patches.append(patch(
+        'clickreviews.cr_scope.ClickReviewScope._extract_scopes',
+        _extract_scopes))
+
+    # content-hub overrides
+    patches.append(patch(
+        'clickreviews.cr_content_hub.ClickReviewContentHub._extract_content_hub',
+        _extract_content_hub))
+
+    # online accounts overrides
+    patches.append(patch(
+        'clickreviews.cr_online_accounts.ClickReviewAccounts._extract_account',
+        _extract_account))
+
+    # push-helper overrides
+    patches.append(patch(
+        'clickreviews.cr_push_helper.ClickReviewPushHelper._extract_push_helper',
+        _extract_push_helper))
+
+    # bin-path overrides
+    patches.append(patch(
+        'clickreviews.cr_bin_path.ClickReviewBinPath._extract_bin_path',
+        _extract_bin_path))
+    patches.append(patch(
+        'clickreviews.cr_bin_path.ClickReviewBinPath._check_bin_path_executable',
+        _check_bin_path_executable))
+
+    # framework overrides
+    patches.append(patch(
+        'clickreviews.cr_framework.ClickReviewFramework._extract_framework',
+        _extract_framework))
+    patches.append(patch(
+        'clickreviews.cr_framework.ClickReviewFramework._extract_framework_policy',
+        _extract_framework_policy))
+    patches.append(patch(
+        'clickreviews.cr_framework.ClickReviewFramework._has_framework_in_metadir',
+        _has_framework_in_metadir))
+
+    return patches
 
 
 class TestClickReview(TestCase):
@@ -1107,9 +1087,10 @@
 
     def setUp(self):
         '''Make sure our patches are applied everywhere'''
-        global patches
+        patches = create_patches()
         for p in patches:
-            self.addCleanup(p.stop())
+            p.start()
+            self.addCleanup(p.stop)
 
     def tearDown(self):
         '''Make sure we reset everything to known good values'''

=== modified file 'clickreviews/tests/test_aaa_example_cr_skeleton.py'
--- clickreviews/tests/test_aaa_example_cr_skeleton.py	2015-08-18 16:05:06 +0000
+++ clickreviews/tests/test_aaa_example_cr_skeleton.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewSkeleton(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_check_foo(self):
         '''Test check_foo()'''

=== modified file 'clickreviews/tests/test_cr_bin_path.py'
--- clickreviews/tests/test_cr_bin_path.py	2015-07-07 20:02:36 +0000
+++ clickreviews/tests/test_cr_bin_path.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewBinPath(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def _set_binary(self, entries, name=None):
         d = dict()

=== modified file 'clickreviews/tests/test_cr_common.py'
--- clickreviews/tests/test_cr_common.py	2015-08-18 16:05:06 +0000
+++ clickreviews/tests/test_cr_common.py	2015-10-16 05:56:07 +0000
@@ -5,10 +5,7 @@
 class ClickReviewTestCase(cr_tests.TestClickReview):
 
     def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
+        super().setUp()
         self.review = ClickReview('app.click', 'review_type')
 
     def test_add_result_default_manual_review(self):

=== modified file 'clickreviews/tests/test_cr_content_hub.py'
--- clickreviews/tests/test_cr_content_hub.py	2014-11-21 18:23:51 +0000
+++ clickreviews/tests/test_cr_content_hub.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewContentHub(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_check_unknown_keys_none(self):
         '''Test check_unknown() - no unknown'''

=== modified file 'clickreviews/tests/test_cr_desktop.py'
--- clickreviews/tests/test_cr_desktop.py	2015-08-18 16:05:06 +0000
+++ clickreviews/tests/test_cr_desktop.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewDesktop(cr_tests.TestClickReview):
     """Tests for the desktop review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_check_desktop_file(self):
         '''Test check_desktop_file()'''

=== modified file 'clickreviews/tests/test_cr_framework.py'
--- clickreviews/tests/test_cr_framework.py	2015-04-10 12:59:40 +0000
+++ clickreviews/tests/test_cr_framework.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewFramework(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_framework_hook_obsolete(self):
         '''Test check_framework_hook_obsolete()'''

=== modified file 'clickreviews/tests/test_cr_lint.py'
--- clickreviews/tests/test_cr_lint.py	2015-10-01 13:05:13 +0000
+++ clickreviews/tests/test_cr_lint.py	2015-10-16 05:56:07 +0000
@@ -14,24 +14,24 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from unittest import TestCase
 from unittest.mock import patch
 
+from clickreviews.cr_common import cleanup_unpack
 from clickreviews.cr_lint import ClickReviewLint
 from clickreviews.cr_lint import MINIMUM_CLICK_FRAMEWORK_VERSION
 from clickreviews.frameworks import FRAMEWORKS_DATA_URL, USER_DATA_FILE
+from clickreviews.tests import utils
 import clickreviews.cr_tests as cr_tests
 
 import os
+import shutil
 import stat
+import tempfile
 
 
 class TestClickReviewLint(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def _create_hashes_yaml(self):
         # find cr_tests.py since that is what _get_statinfo() is mocked to
@@ -1863,3 +1863,32 @@
         r = c.click_report
         expected_counts = {'info': None, 'warn': 0, 'error': 1}
         self.check_results(r, expected_counts)
+
+
+class ClickReviewLintTestCase(TestCase):
+    """Tests without mocks where they are not needed."""
+    def setUp(self):
+        # XXX cleanup_unpack() is required because global variables
+        # UNPACK_DIR, RAW_UNPACK_DIR are initialised to None at module
+        # load time, but updated when a real (non-Mock) test runs, such as
+        # here. While, at the same time, two of the existing tests using
+        # mocks depend on both global vars being None. Ideally, those
+        # global vars should be refactored away.
+        self.addCleanup(cleanup_unpack)
+        super().setUp()
+
+    def mkdtemp(self):
+        """Create a temp dir which is cleaned up after test."""
+        tmp_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tmp_dir)
+        return tmp_dir
+
+    def test_check_dot_click_root(self):
+        package = utils.make_package(extra_files=['.click/'],
+                                     output_dir=self.mkdtemp())
+        c = ClickReviewLint(package)
+
+        c.check_dot_click()
+
+        errors = list(c.click_report['error'].keys())
+        self.assertEqual(errors, ['lint:dot_click'])

=== modified file 'clickreviews/tests/test_cr_online_accounts.py'
--- clickreviews/tests/test_cr_online_accounts.py	2015-09-10 08:32:34 +0000
+++ clickreviews/tests/test_cr_online_accounts.py	2015-10-16 05:56:07 +0000
@@ -22,11 +22,6 @@
 
 class TestClickReviewAccounts(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def _stub_application(self, root=None, id=None, do_subtree=True):
         '''Stub application xml'''

=== modified file 'clickreviews/tests/test_cr_push_helper.py'
--- clickreviews/tests/test_cr_push_helper.py	2014-11-21 18:47:07 +0000
+++ clickreviews/tests/test_cr_push_helper.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewPushHelper(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_check_unknown_keys_none(self):
         '''Test check_unknown() - no unknown'''

=== modified file 'clickreviews/tests/test_cr_scope.py'
--- clickreviews/tests/test_cr_scope.py	2015-01-20 13:04:26 +0000
+++ clickreviews/tests/test_cr_scope.py	2015-10-16 05:56:07 +0000
@@ -21,11 +21,6 @@
 
 class TestClickReviewScope(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def _create_scope(self, config_dict=None):
         '''Create a scope to pass to tests'''

=== modified file 'clickreviews/tests/test_cr_security.py'
--- clickreviews/tests/test_cr_security.py	2015-10-13 21:35:38 +0000
+++ clickreviews/tests/test_cr_security.py	2015-10-16 05:56:07 +0000
@@ -24,10 +24,7 @@
 class TestClickReviewSecurity(cr_tests.TestClickReview):
     """Tests for the security lint review tool."""
     def setUp(self):
-        #  Monkey patch various file access classes. stop() is handled with
-        #  addCleanup in super()
-        cr_tests.mock_patch()
-        super()
+        super().setUp()
 
         self.default_security_json = "%s.apparmor" % \
             self.default_appname

=== modified file 'clickreviews/tests/test_cr_systemd.py'
--- clickreviews/tests/test_cr_systemd.py	2015-10-13 18:58:02 +0000
+++ clickreviews/tests/test_cr_systemd.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewSystemd(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def _create_ports(self):
         ports = {'internal': {'int1': {"port": '8081/tcp', "negotiable": True}},

=== modified file 'clickreviews/tests/test_cr_url_dispatcher.py'
--- clickreviews/tests/test_cr_url_dispatcher.py	2015-04-10 10:36:49 +0000
+++ clickreviews/tests/test_cr_url_dispatcher.py	2015-10-16 05:56:07 +0000
@@ -20,11 +20,6 @@
 
 class TestClickReviewUrlDispatcher(cr_tests.TestClickReview):
     """Tests for the lint review tool."""
-    def setUp(self):
-        # Monkey patch various file access classes. stop() is handled with
-        # addCleanup in super()
-        cr_tests.mock_patch()
-        super()
 
     def test_check_required(self):
         '''Test check_required() - has protocol'''

=== modified file 'clickreviews/tests/test_modules.py'
--- clickreviews/tests/test_modules.py	2014-09-02 16:32:51 +0000
+++ clickreviews/tests/test_modules.py	2015-10-16 05:56:07 +0000
@@ -7,8 +7,7 @@
     '''Tests for the modules module.'''
     def setUp(self):
         self.cr_modules = modules.get_modules()
-        cr_tests.mock_patch()
-        super()
+        super().setUp()
 
     def test_number_of_suitable_modules(self):
         path = clickreviews.__path__[0]

=== added file 'clickreviews/tests/utils.py'
--- clickreviews/tests/utils.py	1970-01-01 00:00:00 +0000
+++ clickreviews/tests/utils.py	2015-10-16 05:56:07 +0000
@@ -0,0 +1,189 @@
+'''utils.py: test utils for click reviewer tools'''
+#
+# Copyright (C) 2013-2015 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; version 3 of the License.
+#
+# 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 glob
+import json
+import os
+import shutil
+import subprocess
+import tempfile
+
+
+def make_package(name='test', package_format='click', package_types=None,
+                 version='1.0', title="An application",
+                 framework='ubuntu-sdk-15.04', extra_files=None, output_dir=None):
+    """Return the path to a click/snap package with the given data.
+
+    Caller is responsible for deleting the output_dir afterwards.
+    """
+    is_snap = (package_format == "snap")
+    build_dir = tempfile.mkdtemp()
+    package_types = package_types or []
+
+    try:
+        make_dir_structure(build_dir, extra_files=extra_files)
+        write_icon(build_dir)
+        write_manifest(build_dir, name, version,
+                       title, framework, package_types,
+                       is_snap)
+        if is_snap:
+            write_meta_data(build_dir, name, version,
+                            title, framework)
+        write_control(build_dir, name, version, title)
+        write_preinst(build_dir)
+        write_apparmor_profile(build_dir, name)
+        write_other_files(build_dir)
+        pkg_path = build_package(build_dir, name, version, package_format,
+                                 output_dir=output_dir)
+    finally:
+        shutil.rmtree(build_dir)
+
+    return pkg_path
+
+
+def make_dir_structure(path, extra_files=None):
+    extra_files = extra_files or []
+    directories = ['DEBIAN', 'meta']
+    directories.extend(
+        [os.path.dirname(extra_file) for extra_file in extra_files])
+
+    for directory in directories:
+        directory = os.path.join(path, directory)
+        if not os.path.exists(directory):
+            os.makedirs(directory)
+
+    for extra_file in extra_files:
+        dirname, basename = os.path.split(extra_file)
+        if basename != '':
+            with open(os.path.join(path, extra_file), 'wb'):
+                pass
+
+
+def write_icon(path):
+    # XXX: Update to use a test icon in the branch to guarantee an icon.
+    icons = glob.glob('/usr/share/icons/hicolor/256x256/apps/*.png')
+    if len(icons) > 0:
+        source_path = icons[0]
+        target_path = os.path.join(path, 'meta', 'icon.png')
+        shutil.copyfile(source_path, target_path)
+
+
+def write_manifest(path, name, version, title, framework, types, is_snap):
+    manifest_content = {
+        'framework': framework,
+        'maintainer': 'Someone <[e-mail]>',
+        'name': name,
+        'title': title,
+        'version': version,
+        'icon': 'meta/icon.png',
+        'hooks': {
+            'app': {
+                'apparmor': 'meta/{}.apparmor'.format(name),
+                },
+            },
+        'description': 'This is a test app.',
+        }
+    if types:
+        if is_snap:
+            manifest_content.update({'type': types[0]})
+        else:
+            if "scope" in types:
+                manifest_content['hooks']['app'].update({'scope': ""})
+            if "application" in types:
+                manifest_content['hooks']['app'].update({'desktop': ""})
+
+    manifest_paths = [
+        os.path.join(path, 'DEBIAN', 'manifest'),
+        os.path.join(path, 'manifest.json'),
+    ]
+    for manifest_path in manifest_paths:
+        with open(manifest_path, 'w') as f:
+            json.dump(manifest_content, f)
+
+
+def write_meta_data(path, name, version, title, framework):
+    yaml_path = os.path.join(path, 'meta', 'package.yaml')
+    content = """architectures:
+icon: meta/icon.png
+name: {}
+version: "{}",
+framework: {},
+vendor: 'Someone <[e-mail]>',
+""".format(name, version, framework)
+
+    with open(yaml_path, 'w') as f:
+        f.write(content)
+    with open(os.path.join(path, 'meta', 'readme.md'), 'w') as f:
+        f.write(title)
+
+
+def write_control(path, name, version, title):
+    control_path = os.path.join(path, 'DEBIAN', 'control')
+    control_content = {
+        'Package': name,
+        'Version': version,
+        'Click-Version': '0.4',
+        'Architecture': 'all',
+        'Maintainer': 'Someone <[e-mail]>',
+        'Installed-Size': '123',
+        'Description': title,
+        }
+    with open(control_path, 'w') as f:
+        for key, value in control_content.items():
+            f.write(key + ": " + value + "\n")
+
+
+def write_preinst(path):
+    preinst_path = os.path.join(path, 'DEBIAN', 'preinst')
+    with open(preinst_path, 'w') as f:
+        f.write("""#! /bin/sh
+echo "Click packages may not be installed directly using dpkg."
+echo "Use 'click install' instead."
+exit 1
+""")
+    os.chmod(preinst_path, 0o775)
+
+
+def write_apparmor_profile(path, name):
+    profile_path = os.path.join(path, 'meta', '{}.apparmor'.format(name))
+    profile = {
+        'policy_version': 1.3,
+        'policy_groups': [],
+    }
+    with open(profile_path, 'w') as f:
+        json.dump(profile, f)
+
+
+def write_other_files(path):
+    def write_empty_file(path, perms=0o664):
+        with open(path, 'wb'):
+            pass
+        os.chmod(path, perms)
+    write_empty_file(os.path.join(path, 'DEBIAN', 'md5sums'))
+
+
+def build_package(path, name, version, format, output_dir=None):
+    filename = "{}_{}_all.{}".format(name, version, format)
+    output_dir = output_dir or tempfile.mkdtemp()
+    output_path = os.path.join(output_dir, filename)
+
+    # Note: We're not using 'click build' here as it corrects errors (such
+    # as filtering out a .click directory present in the build). We want
+    # to test with manually constructed, potentially tampered-with
+    # clicks/snaps. Ideally, we'd be using click rather than dpkg to
+    # construct the click without filtering any files in the build dir.
+    subprocess.check_call(['dpkg-deb', '-b', path, output_path])
+    return output_path