Add unit tests for cross_check_pkg_resources_versus_import, and a regression test for ref #1355. This requires a little refactoring to make it testable.

src/allmydata/__init__.py

@@ -251,15 +251,21 @@ def cross_check_pkg_resources_versus_import():
     import pkg_resources
     from _auto_deps import install_requires
 
+    pkg_resources_vers_and_locs = dict([(p.project_name.lower(), (str(p.version), p.location))
+                                        for p in pkg_resources.require(install_requires)])
+
+    return cross_check(pkg_resources_vers_and_locs, _vers_and_locs_list)
+
+
+def cross_check(pkg_resources_vers_and_locs, imported_vers_and_locs_list):
+    """This function returns a list of errors due to any failed cross-checks."""
+
     errors = []
     not_pkg_resourceable = set(['sqlite3', 'python', 'platform', __appname__.lower()])
     not_import_versionable = set(['zope.interface', 'mock', 'pyasn1'])
     ignorable = set(['argparse', 'pyutil', 'zbase32', 'distribute'])
 
-    pkg_resources_vers_and_locs = dict([(p.project_name.lower(), (str(p.version), p.location))
-                                        for p in pkg_resources.require(install_requires)])
-
-    for name, (imp_ver, imp_loc, imp_comment) in _vers_and_locs_list:
+    for name, (imp_ver, imp_loc, imp_comment) in imported_vers_and_locs_list:
         name = name.lower()
         if name not in not_pkg_resourceable:
             if name not in pkg_resources_vers_and_locs:
@@ -306,7 +312,7 @@ def cross_check_pkg_resources_versus_import():
                                               "by pkg_resources, but version %r (normalized to %r, from %r) by import."
                                               % (name, pr_ver, str(pr_normver), pr_loc, imp_ver, str(imp_normver), imp_loc))
 
-    imported_packages = set([p.lower() for (p, _) in _vers_and_locs_list])
+    imported_packages = set([p.lower() for (p, _) in imported_vers_and_locs_list])
     for pr_name, (pr_ver, pr_loc) in pkg_resources_vers_and_locs.iteritems():
         if pr_name not in imported_packages and pr_name not in ignorable:
             errors.append("Warning: dependency %r (version %r) found by pkg_resources not found by import."

src/allmydata/test/test_version.py

@@ -1,7 +1,7 @@
 
 from twisted.trial import unittest
 
-from allmydata import check_requirement, PackagingError
+from allmydata import check_requirement, cross_check, PackagingError
 from allmydata.util.verlib import NormalizedVersion as V, \
                                   IrrationalVersionError, \
                                   suggest_normalized_version as suggest
@@ -30,6 +30,70 @@ class CheckRequirement(unittest.TestCase):
         self.failUnlessRaises(ImportError, check_requirement,
                               "foo >= 1.0", {"foo": (None, None, "foomodule")})
 
+    def test_cross_check_ticket_1355(self):
+        # The bug in #1355 is triggered when a version string from either pkg_resources or import
+        # is not parseable at all by normalized_version.
+
+        res = cross_check({"foo": ("unparseable", "")}, [("foo", ("1.0", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("by pkg_resources could not be parsed", res[0])
+
+        res = cross_check({"foo": ("1.0", "")}, [("foo", ("unparseable", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn(") could not be parsed", res[0])
+
+    def test_cross_check(self):
+        res = cross_check({}, [])
+        self.failUnlessEqual(res, [])
+
+        res = cross_check({}, [("sqlite3", ("1.0", "", "blah"))])
+        self.failUnlessEqual(res, [])
+
+        res = cross_check({"foo": ("unparseable", "")}, [])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("not found by import", res[0])
+
+        res = cross_check({"argparse": ("unparseable", "")}, [])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({}, [("foo", ("unparseable", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("not found by pkg_resources", res[0])
+
+        res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere", "distribute"))])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("location mismatch", res[0])
+
+        res = cross_check({"distribute": ("1.0", "/somewhere")}, [("setuptools", ("2.0", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("location mismatch", res[0])
+
+        res = cross_check({"zope.interface": ("1.0", "")}, [("zope.interface", ("unknown", "", None))])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({"foo": ("1.0", "")}, [("foo", ("unknown", "", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("could not find a version number", res[0])
+
+        # When pkg_resources and import both find a package, there is only a warning if both
+        # the version and the path fail to match.
+
+        res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("2.0", "/somewhere", None))])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("1.0", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({"foo": ("1.0-r123", "/somewhere")}, [("foo", ("1.0.post123", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 0)
+
+        res = cross_check({"foo": ("1.0", "/somewhere")}, [("foo", ("2.0", "/somewhere_different", None))])
+        self.failUnlessEqual(len(res), 1)
+        self.failUnlessIn("but version '2.0'", res[0])
+
 
 # based on https://bitbucket.org/tarek/distutilsversion/src/17df9a7d96ef/test_verlib.py