Repository: jameslamb/doppel-cli

Issue:
Add tests that doppel-test works on 1 or 3 or 10 files
Nothing about `doppel-cli` explicitly requires that you have exactly two comparison files!!

Need to add unit tests that `doppel-test` works with 1 file, 3 files, or 10 files! (just as examples of other non-2 counts to catch errors). Should establish tests like this to catch places where the project only works with exactly 2 things to compare.

Discussion:

Patch:
diff --git a/NEWS.md b/NEWS.md
index f8ab5bd..b24fddb 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -2,6 +2,7 @@
 
 ## development version
 
+* All errors comparing public methods or functions are now reported. Previously, if two issues existed (such as "different number of keyword args" and "different order of keyword args"), only one would be reported. ([#121](https://github.com/jameslamb/doppel-cli/pull/121))
 * Fixed bug which could cause `doppel-describe` to run endlessly in the presence of cyclic imports between submodules (e.g. in `pandas`). ([#120](https://github.com/jameslamb/doppel-cli/pull/120))
 * Fixed bug which could cause a `ValueError` in `doppel-describe` when a Python package used CPython and had builtins that could not be inspected. This showed up in popular packages such as `pandas` and `numpy`. ([#94](https://github.com/jameslamb/doppel-cli/pull/94))
 
diff --git a/doppel/reporters.py b/doppel/reporters.py
index 6b82bed..6323632 100644
--- a/doppel/reporters.py
+++ b/doppel/reporters.py
@@ -177,7 +177,7 @@ class SimpleReporter:
 
     def _check_function_args(self):
         """
-        For each function that is in both packages, check
+        For each function that is in all packages, check
         whether the arguments to the functions differ.
         """
         stdout.write("\nFunction Argument Names\n")
@@ -199,13 +199,19 @@ class SimpleReporter:
         rows = []
 
         for func_name in shared_functions:
+
+            identical_api = 'yes'
+
             args = [
                 func_blocks_by_package[p][func_name]['args']
                 for p in pkg_names
             ]
 
             # check 1: same number of arguments?
-            same_length = reduce(lambda a, b: len(a) == len(b), args)
+            same_length = all([
+                len(func_arg_list) == len(args[0])
+                for func_arg_list in args
+            ])
             if not same_length:
                 error_txt = "Function '{}()' exists in all packages but with differing number of arguments ({})."
                 error_txt = error_txt.format(
@@ -213,29 +219,32 @@ class SimpleReporter:
                     ",".join([str(len(a)) for a in args])
                 )
                 self.errors.append(DoppelTestError(error_txt))
-                rows.append([func_name, 'no'])
-                continue
+                identical_api = 'no'
 
             # check 2: same set of arguments
-            same_args = reduce(lambda a, b: sorted(a) == sorted(b), args)
+            same_args = all([
+                sorted(func_arg_list) == sorted(args[0])
+                for func_arg_list in args
+            ])
             if not same_args:
                 error_txt = "Function '{}()' exists in all packages but some arguments are not shared in all implementations."
                 error_txt = error_txt.format(func_name)
                 self.errors.append(DoppelTestError(error_txt))
-                rows.append([func_name, 'no'])
-                continue
+                identical_api = 'no'
 
-            # check 3: same set or arguments and same order
-            same_order = reduce(lambda a, b: a == b, args)
+            # check 3: same set of arguments and same order
+            same_order = all([
+                len(func_arg_list) == len(args[0]) and func_arg_list == args[0]
+                for func_arg_list in args
+            ])
             if not same_order:
                 error_txt = "Function '{}()' exists in all packages but with differing order of keyword arguments."
                 error_txt = error_txt.format(func_name)
                 self.errors.append(DoppelTestError(error_txt))
-                rows.append([func_name, 'no'])
-                continue
+                identical_api = 'no'
 
             # if you get here, we're gucci
-            rows.append([func_name, 'yes'])
+            rows.append([func_name, identical_api])
 
         # Report output
         stdout.write('\n{} of the {} functions shared across all packages have identical signatures\n\n'.format(
@@ -391,6 +400,8 @@ class SimpleReporter:
         for class_name, methods in shared_methods_by_class.items():
             for method_name in methods:
 
+                identical_api = 'yes'
+
                 display_name = "{}.{}()".format(
                     class_name,
                     method_name
@@ -403,7 +414,10 @@ class SimpleReporter:
                 ]
 
                 # check 1: same number of arguments?
-                same_length = reduce(lambda a, b: len(a) == len(b), args)
+                same_length = all([
+                    len(func_arg_list) == len(args[0])
+                    for func_arg_list in args
+                ])
                 if not same_length:
                     error_txt = "Public method '{}()' on class '{}' exists in all packages but with differing number of arguments ({})."
                     error_txt = error_txt.format(
@@ -412,11 +426,13 @@ class SimpleReporter:
                         ",".join([str(len(a)) for a in args])
                     )
                     self.errors.append(DoppelTestError(error_txt))
-                    rows.append([display_name, 'no'])
-                    continue
+                    identical_api = 'no'
 
                 # check 2: same set of arguments
-                same_args = reduce(lambda a, b: sorted(a) == sorted(b), args)
+                same_args = all([
+                    sorted(func_arg_list) == sorted(args[0])
+                    for func_arg_list in args
+                ])
                 if not same_args:
                     error_txt = "Public method '{}()' on class '{}' exists in all packages but some arguments are not shared in all implementations."
                     error_txt = error_txt.format(
@@ -424,11 +440,13 @@ class SimpleReporter:
                         class_name
                     )
                     self.errors.append(DoppelTestError(error_txt))
-                    rows.append([display_name, 'no'])
-                    continue
+                    identical_api = 'no'
 
                 # check 3: same set or arguments and same order
-                same_order = reduce(lambda a, b: a == b, args)
+                same_order = all([
+                    func_arg_list == args[0]
+                    for func_arg_list in args
+                ])
                 if not same_order:
                     error_txt = "Public method '{}()' on class '{}' exists in all packages but with differing order of keyword arguments."
                     error_txt = error_txt.format(
@@ -436,11 +454,10 @@ class SimpleReporter:
                         class_name
                     )
                     self.errors.append(DoppelTestError(error_txt))
-                    rows.append([display_name, 'no'])
-                    continue
+                    identical_api = 'no'
 
                 # if you get here, we're gucci
-                rows.append([display_name, 'yes'])
+                rows.append([display_name, identical_api])
 
         # Report output
         out = _OutputTable(headers=headers, rows=rows)


Test patch:
diff --git a/tests/test_reporters.py b/tests/test_reporters.py
index 5356494..15d4f38 100644
--- a/tests/test_reporters.py
+++ b/tests/test_reporters.py
@@ -1,5 +1,6 @@
 import unittest
 import copy
+import re
 from doppel import SimpleReporter
 from doppel import PackageAPI
 from doppel import DoppelTestError
@@ -74,11 +75,11 @@ PACKAGE_WITH_DIFFERENT_PM_ARG_NUMBER['name'] = 'pkg2'
 PACKAGE_WITH_DIFFERENT_PM_ARG_NUMBER['classes']['WaleFolarin']['public_methods']['no_days_off']['args'].append('about_nothing')
 
 PACKAGE_WITH_DIFFERENT_PM_ARGS = copy.deepcopy(BASE_PACKAGE_WITH_CLASSES)
-PACKAGE_WITH_DIFFERENT_PM_ARGS['name'] = 'pkg2'
+PACKAGE_WITH_DIFFERENT_PM_ARGS['name'] = 'pkg3'
 PACKAGE_WITH_DIFFERENT_PM_ARGS['classes']['WaleFolarin']['public_methods']['no_days_off']['args'] = ['days_to_take', 'outchyea']
 
 PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER = copy.deepcopy(BASE_PACKAGE_WITH_CLASSES)
-PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER['name'] = 'pkg2'
+PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER['name'] = 'pkg4'
 PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER['classes']['WaleFolarin']['public_methods']['no_days_off']['args'] = ['songs_to_record', 'days_to_take']
 
 # super different
@@ -123,7 +124,7 @@ class TestSimpleReporter(unittest.TestCase):
         """
         SimpleReporter should catch the condition where
         function implementations in two packages have different
-        number of keyword arguments
+        number of keyword arguments.
         """
         reporter = SimpleReporter(
             pkgs=[PackageAPI(BASE_PACKAGE), PackageAPI(PACKAGE_WITH_DIFFERENT_ARG_NUMBER)],
@@ -132,7 +133,7 @@ class TestSimpleReporter(unittest.TestCase):
         reporter._check_function_args()
         errors = reporter.errors
         self.assertTrue(
-            len(errors) == 1,
+            len(errors) == 3,
         )
         self.assertTrue(
             all([isinstance(x, DoppelTestError) for x in errors])
@@ -155,7 +156,7 @@ class TestSimpleReporter(unittest.TestCase):
         reporter._check_function_args()
         errors = reporter.errors
         self.assertTrue(
-            len(errors) == 1,
+            len(errors) == 2,
         )
         self.assertTrue(
             all([isinstance(x, DoppelTestError) for x in errors])
@@ -188,7 +189,7 @@ class TestSimpleReporter(unittest.TestCase):
 
     def test_function_all_wrong(self):
         """
-        SimpleReporter should only throw a single error
+        SimpleReporter should throw 3 errors
         if one package has a function with different args, more args
         and different order.
         """
@@ -199,7 +200,7 @@ class TestSimpleReporter(unittest.TestCase):
         reporter._check_function_args()
         errors = reporter.errors
         self.assertTrue(
-            len(errors) == 1,
+            len(errors) == 3,
         )
         self.assertTrue(
             all([isinstance(x, DoppelTestError) for x in errors])
@@ -236,7 +237,7 @@ class TestSimpleReporter(unittest.TestCase):
         reporter._check_class_public_method_args()
         errors = reporter.errors
         self.assertTrue(
-            len(errors) == 1,
+            len(errors) == 3,
         )
         self.assertTrue(
             all([isinstance(x, DoppelTestError) for x in errors])
@@ -259,7 +260,7 @@ class TestSimpleReporter(unittest.TestCase):
         reporter._check_class_public_method_args()
         errors = reporter.errors
         self.assertTrue(
-            len(errors) == 1,
+            len(errors) == 2,
         )
         self.assertTrue(
             all([isinstance(x, DoppelTestError) for x in errors])
@@ -339,3 +340,131 @@ class TestSimpleReporter(unittest.TestCase):
         # check packages
         reporter.compare()
         self.assertTrue(True)
+
+    def test_works_with_one_package(self):
+        """
+        SimpleReporter should not return any errors if you
+        only use a single package
+        """
+        reporter = SimpleReporter(
+            pkgs=[PackageAPI(BASE_PACKAGE_WITH_CLASSES)],
+            errors_allowed=0
+        )
+
+        # SimpleReporter has a sys.exit() in it. Mock that out
+        def f():
+            pass
+        reporter._respond = f
+
+        # check packages
+        reporter.compare()
+        self.assertTrue(len(reporter.pkgs) == 1)
+        self.assertTrue(reporter.errors == [])
+
+    def test_works_with_three_packages(self):
+        """
+        SimpleReporter should work correctly if you have
+        three packages
+        """
+        reporter = SimpleReporter(
+            pkgs=[
+                PackageAPI(BASE_PACKAGE_WITH_CLASSES),
+                PackageAPI(PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER),
+                PackageAPI(PACKAGE_WITH_DIFFERENT_PM_ARG_NUMBER)
+            ],
+            errors_allowed=100
+        )
+
+        # SimpleReporter has a sys.exit() in it. Mock that out
+        def f():
+            pass
+        reporter._respond = f
+
+        # check packages
+        reporter.compare()
+
+        # This check (exactly 3 errors) is important. To be sure
+        # that other problems aren't getting silenced by short-circuiting
+        self.assertTrue(len(reporter.errors) == 3)
+        self.assertTrue(len(reporter.pkgs) == 3)
+
+        # at least one should be the number-of-arguments error
+        self.assertTrue(
+            any([
+                bool(re.search('differing number of arguments', err.msg))
+                for err in reporter.errors
+            ])
+        )
+
+        # at least one should be the some-args-not-shared
+        self.assertTrue(
+            any([
+                bool(re.search('some arguments are not shared', err.msg))
+                for err in reporter.errors
+            ])
+        )
+
+        # at least one should be the different-order one
+        self.assertTrue(
+            any([
+                bool(re.search('differing order of keyword arguments', err.msg))
+                for err in reporter.errors
+            ])
+        )
+
+    def test_works_with_ten_packages(self):
+        """
+        SimpleReporter should work correctly if you have
+        ten packages (yes I know this is extreme)
+        """
+        pkgs = [
+            PackageAPI(BASE_PACKAGE_WITH_CLASSES),
+            PackageAPI(PACKAGE_WITH_DIFFERENT_PM_ARG_ORDER),
+            PackageAPI(PACKAGE_WITH_DIFFERENT_PM_ARG_NUMBER)
+        ]
+        for i in range(7):
+            new_pkg = copy.deepcopy(BASE_PACKAGE_WITH_CLASSES)
+            new_pkg['name'] = 'test_package_' + str(i)
+            pkgs.append(PackageAPI(new_pkg))
+
+        reporter = SimpleReporter(
+            pkgs=pkgs,
+            errors_allowed=100
+        )
+
+        # SimpleReporter has a sys.exit() in it. Mock that out
+        def f():
+            pass
+        reporter._respond = f
+
+        # check packages
+        reporter.compare()
+
+        # This check (exactly 3 errors) is important. To be sure
+        # that other problems aren't getting silenced by short-circuiting
+        self.assertTrue(len(reporter.errors) == 3)
+        self.assertTrue(len(reporter.pkgs) == 10)
+
+        # at least one should be the number-of-arguments error
+        self.assertTrue(
+            any([
+                bool(re.search('differing number of arguments', err.msg))
+                for err in reporter.errors
+            ])
+        )
+
+        # at least one should be the some-args-not-shared
+        self.assertTrue(
+            any([
+                bool(re.search('some arguments are not shared', err.msg))
+                for err in reporter.errors
+            ])
+        )
+
+        # at least one should be the different-order one
+        self.assertTrue(
+            any([
+                bool(re.search('differing order of keyword arguments', err.msg))
+                for err in reporter.errors
+            ])
+        )