Repository: ioos/compliance-checker
Pull request #659 (2019-07-02 21:01:11)

Allows checkers using .name attr to be listed
Allows checkers using the `.name` attribute instead of the `._cc_spec` and
`cc_spec_version` to be listed in the checker output.  Checkers using
the former will raise a `DeprecationWarning` indicating the checker is
not preferred.  Also refactors some of the check suite logic for
printing listings and adds unit test for aforementioned behavior.
Addresses #610.


Patch:
diff --git a/cchecker.py b/cchecker.py
index f6a393a..faa37c5 100755
--- a/cchecker.py
+++ b/cchecker.py
@@ -95,13 +95,7 @@ def main():
 
     if args.list_tests:
         print("IOOS compliance checker available checker suites:")
-        for checker in sorted(check_suite.checkers.keys()):
-            version = getattr(check_suite.checkers[checker],
-                              '_cc_checker_version', "???")
-            if args.verbose:
-                print(" - {} (v{})".format(checker, version))
-            elif ':' in checker and not checker.endswith(':latest'):  # Skip the "latest" output
-                print(" - {}".format(checker))
+        check_suite._print_suites(args.verbose)
         return 0
 
     if args.download_standard_names:
diff --git a/compliance_checker/suite.py b/compliance_checker/suite.py
index 1fb5f93..8354401 100644
--- a/compliance_checker/suite.py
+++ b/compliance_checker/suite.py
@@ -37,6 +37,7 @@ if sys.stdout.encoding is None:
 if sys.stderr.encoding is None:
     sys.stderr = codecs.getwriter('utf8')(sys.stderr)
 
+
 class CheckSuite(object):
 
     checkers = {}       # Base dict of checker names to BaseCheck derived types, override this in your CheckSuite implementation
@@ -58,6 +59,23 @@ class CheckSuite(object):
 
         return cls.suite_generators
 
+    def _print_suites(self, verbose=0):
+        """
+        Prints out available check suites.  If the verbose argument is True,
+        includes the internal module version number of the check and also displays
+        "latest" meta-versions.
+        :param check_suite: Check suite object
+        :param verbose: Integer indicating whether to print verbose output
+        :type verbose: int
+        """
+        for checker in sorted(self.checkers.keys()):
+            version = getattr(self.checkers[checker],
+                            '_cc_checker_version', "???")
+            if verbose > 0:
+                print(" - {} (v{})".format(checker, version))
+            elif ':' in checker and not checker.endswith(':latest'):  # Skip the "latest" output
+                print(" - {}".format(checker))
+
     @classmethod
     def add_plugin_args(cls, parser):
         """
@@ -83,22 +101,42 @@ class CheckSuite(object):
         Helper method to retrieve all sub checker classes derived from various
         base classes.
         """
-        for x in working_set.iter_entry_points('compliance_checker.suites'):
+        cls._load_checkers(working_set
+                           .iter_entry_points('compliance_checker.suites'))
+
+    @classmethod
+    def _load_checkers(cls, checkers):
+        """
+        Loads up checkers in an iterable into the class checkers dict
+        :param checkers: An iterable containing the checker objects
+        """
+
+        for c in checkers:
             try:
-                xl = x.resolve()
-                cls.checkers[':'.join((xl._cc_spec, xl._cc_spec_version))] = xl
-            # TODO: remove this once all checkers move over to the new
-            #       _cc_spec, _cc_spec_version
-            except AttributeError:
-                # if there are versioned classes, it will get overwritten by the
-                # latest version later.  If there are not, it will be assigned
-                # the checker as the main class
-                # TODO: nix name attribute in plugins.  Keeping in for now
-                #       to provide backwards compatibility
-                cls.checkers[getattr(xl, 'name', None) or xl._cc_spec] = xl
+                check_obj = c.resolve()
+                if (hasattr(check_obj, '_cc_spec') and
+                    hasattr(check_obj, '_cc_spec_version')):
+                    check_version_str = ':'.join((check_obj._cc_spec,
+                                                  check_obj._cc_spec_version))
+                    cls.checkers[check_version_str] = check_obj
+                # TODO: remove this once all checkers move over to the new
+                #       _cc_spec, _cc_spec_version
+                else:
+                    # if _cc_spec and _cc_spec_version attributes aren't
+                    # present, fall back to using name attribute
+                    checker_name = (getattr(check_obj, 'name', None) or
+                                    getattr(check_obj, '_cc_spec', None))
+                    warnings.warn('Checker for {} should implement both '
+                                '"_cc_spec" and "_cc_spec_version" '
+                                'attributes. "name" attribute is deprecated. '
+                                'Assuming checker is latest version.',
+                                DeprecationWarning)
+                    # append "unknown" to version string since no versioning
+                    # info was provided
+                    cls.checkers["{}:unknown".format(checker_name)] = check_obj
 
             except Exception as e:
-                print("Could not load", x, ":", e, file=sys.stderr)
+                print("Could not load", c, ":", e, file=sys.stderr)
         # find the latest version of versioned checkers and set that as the
         # default checker for compliance checker if no version is specified
         ver_checkers = sorted([c.split(':', 1) for c


Test patch:
diff --git a/compliance_checker/tests/test_cli.py b/compliance_checker/tests/test_cli.py
index 18829e2..3b28997 100644
--- a/compliance_checker/tests/test_cli.py
+++ b/compliance_checker/tests/test_cli.py
@@ -13,8 +13,8 @@ from compliance_checker.runner import ComplianceChecker, CheckSuite
 from compliance_checker.tests.resources import STATIC_FILES
 import tempfile
 import os
-
-CheckSuite.load_all_available_checkers()
+from argparse import Namespace
+import pytest
 
 
 class TestCLI(TestCase):
@@ -24,6 +24,9 @@ class TestCLI(TestCase):
 
     def setUp(self):
         self.fid, self.path = tempfile.mkstemp()
+        # why is the class being written to
+        CheckSuite.checkers.clear()
+        CheckSuite.load_all_available_checkers()
 
     def tearDown(self):
         if os.path.isfile(self.path):
@@ -92,6 +95,41 @@ class TestCLI(TestCase):
             r = json.load(f)
             assert 'cf' in r
 
+
+    def test_list_checks(self):
+        """
+        Tests listing of both old-style, deprecated checkers using .name
+        attributes, and newer ones which use ._cc_spec and _cc_spec_version
+        attributes
+        """
+
+        # hack: use argparse.Namespace to mock checker object with attributes
+        # since SimpleNamespace is Python 3.3+ only
+        CheckSuite.checkers.clear()
+        # need to mock setuptools entrypoints here in order to load in checkers
+        def checker_1():
+            return Namespace(name='checker_1')
+        def checker_2():
+            return Namespace(_cc_spec='checker_2',
+                             _cc_spec_version='2.2')
+        mock_checkers = [Namespace(resolve=checker_1),
+                         Namespace(resolve=checker_2)]
+        with pytest.warns(DeprecationWarning):
+            CheckSuite._load_checkers(mock_checkers)
+
+        cs = CheckSuite()
+        saved = sys.stdout
+        try:
+            # ugly!  consider refactoring to use py.test capsys
+            fake_stdout = io.StringIO()
+            sys.stdout = fake_stdout
+            cs._print_suites()
+            assert (fake_stdout.getvalue() ==
+                    ' - checker_1:unknown\n - checker_2:2.2\n')
+        finally:
+            sys.stdout = saved
+            fake_stdout.close()
+
     def test_multiple_json_output(self):
         '''
         Tests that a suite can produce JSON output to a file
@@ -132,6 +170,7 @@ class TestCLI(TestCase):
             assert 'cf' in r
         finally:
             sys.stdout = saved
+            fake_stdout.close()
 
     def test_single_json_output_stdout(self):
         '''
@@ -153,6 +192,7 @@ class TestCLI(TestCase):
             assert 'cf' in r
         finally:
             sys.stdout = saved
+            fake_stdout.close()
 
     def test_text_output(self):
         '''