From 787d12165af49a4abbde7d2c4a25c85d00aa000c Mon Sep 17 00:00:00 2001
From: david-sarah <david-sarah@jacaranda.org>
Date: Sun, 20 Feb 2011 17:58:17 -0800
Subject: [PATCH] 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          | 16 +++++---
 src/allmydata/test/test_version.py | 66 +++++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/__init__.py b/src/allmydata/__init__.py
index 83c867d7..0eb5b245 100644
--- a/src/allmydata/__init__.py
+++ b/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."
diff --git a/src/allmydata/test/test_version.py b/src/allmydata/test/test_version.py
index 6c009084..190bef81 100644
--- a/src/allmydata/test/test_version.py
+++ b/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
 
-- 
2.45.2