Python3 support, testcoverage and bugfixes (#10)

* Bypass bootstrap

* Add tox config with coverage; initial python3 compatibility fixes

* Namespace "set" doctest queries to bring them in line with the "value" doctests

* Bringing testcoverage on `set` to 100% surfaces multiple bugs.
Just goes to show: untested code is buggy code.

* Increase doctest coverage on query.Result and query.NoResult

* Turn tests into a package to make space for more test modules

* Reproduce doctest fixture in unittest land

* Test cover query.And 100%, remove unreachable code

* Test cover query.Or 100%

* Test cover query.Difference 100%

* Test cover query.In 100%

* Test cover query.Term 100%

* Test cover query.Timer 100%

* Test cover query.TimingAwareCache 100%

* Remove unused transaction cache

* Test cover query.Query 100%, clarify `timing` arg type

* Update changelog

* Add pypy to tox runs

* Support python 3.4, 3.5, 3.6 in addition to python 2.7

.gitignore

@@ -1,11 +1,18 @@
+*.egg-info/
 *.py[co]
+.coverage*
+.installed.cfg
+.tox/
 __pycache__/
+bin/
 build/
+develop-eggs/
 dist/
-*.egg-info/
-.tox/
-bin/
 eggs/
-develop-eggs/
+htmlcov/
+include/
+lib/
 parts/
-.installed.cfg
+pip-selfcheck.json
+pyvenv.cfg
+share/

.travis.yml

@@ -1,8 +1,20 @@
 language: python
+python:
+    - 2.7
+    - 3.4
+    - 3.5
+    - 3.6
+    - pypy
+    - pypy3
 install:
-    - python bootstrap.py
-    - bin/buildout
+    - pip install -U pip setuptools
+    - pip install -U zope.testrunner coverage coveralls
+    - pip install -U -e .[test]
 script:
-    - bin/test -pvc
+    - coverage run -m zope.testrunner --test-path=src
+after_success:
+    - coveralls
 notifications:
     email: false
+cache: pip
+

CHANGES.txt

@@ -1,10 +1,35 @@
 CHANGES
 =======
 
-2.7 (unreleased)
+3.0 (unreleased)
 ----------------
 
-- Nothing changed yet.
+- Support for python 3.4, 3.5 and 3.6 in addition to python 2.7
+
+- Cleanup in preparation for python3 support:
+
+  Bugfixes:
+  o API change: fix And(weighted=) keyword argument typo
+  o API change: remove utterly broken ``include_minimum`` and ``include_maximum``
+    arguments to SetBetween(), provide ``exclude_min`` and ``exclude_max`` instead.
+  o API change: fix broken SetBetween.apply(): introduce ``cache`` arg
+  o Fix ExtentNone() super delegation bug
+  o Fix TimingAwareCaching.report() edge condition bug
+
+  Major:
+  o Remove unsupported transaction_cache
+
+  Minor:
+  o Clarify HURRY_QUERY_TIMING environment and searchResults(timing=) type
+  o Fix TimingAwareCaching.report() output typo
+  o Clarify Query.searchResults(caching=) argument type
+  o Remove unreachable code path from And()
+
+  Dev:
+  o Maximize test coverage
+  o Add Travis and Tox testing configurations
+  o Bypass bootstrap.py
+  o Various python3 compatibility preparations
 
 
 2.6 (2018-01-10)

buildout.cfg

@@ -4,6 +4,7 @@ parts = releaser test
 versions = versions
 
 [versions]
+hurry.query =
 
 [releaser]
 recipe = zc.recipe.egg
@@ -12,6 +13,5 @@ eggs = zest.releaser
 [test]
 recipe = zc.recipe.testrunner
 eggs =
-  hurry.query
   hurry.query [test]
-defaults = ['--tests-pattern', '^f?tests$', '-v']
+defaults = ['-v', '--auto-color']

requirements.txt

@@ -0,0 +1,4 @@
+# NOTE: setuptools and zc.buildout versions must be in sync with:
+# ztk-versions.cfg
+setuptools==38.2.4
+zc.buildout==2.10.0

setup.py

@@ -16,21 +16,29 @@
 $Id$
 """
 import os
+import sys
 from setuptools import setup, find_packages
 
+if sys.version_info.major > 2:
+    import html
+
 
 def read(*rnames):
     text = open(os.path.join(os.path.dirname(__file__), *rnames)).read()
-    text = unicode(text, 'utf-8').encode('ascii', 'xmlcharrefreplace')
+    if sys.version_info.major == 2:
+        text = unicode(text, 'utf-8').encode('ascii', 'xmlcharrefreplace')
+    else:
+        text = html.escape(text)
     return text
 
 tests_require = [
+    'testfixtures',
     'zope.container',
     ]
 
 setup(
     name="hurry.query",
-    version='2.7.dev0',
+    version='3.0.dev0',
     author='Infrae',
     author_email='faassen@startifact.com',
     description="Higher level query system for the zope.catalog",
@@ -41,13 +49,22 @@ def read(*rnames):
     keywords="zope zope3 catalog index query",
     classifiers=[
         'Development Status :: 5 - Production/Stable',
+        'Natural Language :: English',
+        'Operating System :: OS Independent',
+        'Topic :: Internet :: WWW/HTTP',
         'Environment :: Web Environment',
         'Intended Audience :: Developers',
         'License :: OSI Approved :: Zope Public License',
         'Programming Language :: Python',
-        'Natural Language :: English',
-        'Operating System :: OS Independent',
-        'Topic :: Internet :: WWW/HTTP',
+        'Programming Language :: Python :: 2',
+        'Programming Language :: Python :: 2.7',
+        'Programming Language :: Python :: 3',
+        'Programming Language :: Python :: 3.4',
+        'Programming Language :: Python :: 3.5',
+        'Programming Language :: Python :: 3.6',
+        'Programming Language :: Python :: Implementation',
+        'Programming Language :: Python :: Implementation :: CPython',
+        'Programming Language :: Python :: Implementation :: PyPy',
         'Framework :: Zope3'],
     url='http://pypi.python.org/pypi/hurry.query',
     packages=find_packages('src'),

src/hurry/query/query.py

@@ -31,74 +31,23 @@
 from zope.catalog.interfaces import ICatalog
 from zope.catalog.text import ITextIndex
 from zope.component import getUtility, getSiteManager, IComponentLookup
-from zope.interface import implements
+from zope.interface import implementer
 from zope.intid.interfaces import IIntIds
 from zope.index.interfaces import IIndexSort
 from zope.index.text.parsetree import ParseError
 from zope.location.location import located, LocationProxy
 from hurry.query import interfaces
 
-import transaction
-import threading
 
 logger = logging.getLogger('hurry.query')
-HURRY_QUERY_TIMING = False
+HURRY_QUERY_TIMING = 0.0  # log queries taking longer than this, in seconds
 if 'HURRY_QUERY_TIMING' in os.environ:
     try:
         HURRY_QUERY_TIMING = float(os.environ['HURRY_QUERY_TIMING'])
     except (ValueError, TypeError):
         pass
 
 
-class Cache(threading.local):
-    implements(transaction.interfaces.IDataManager)
-
-    def __init__(self, manager):
-        self._manager = manager
-        self.reset()
-
-    def sortKey(self):
-        return 'A' * 26
-
-    def use(self, context):
-        if not self._joined:
-            self._joined = True
-            transaction = self._manager.get()
-            transaction.join(self)
-        if context is not self._context:
-            # The context changed, reset the cache as we might access
-            # different indexes.
-            self.cache = {}
-            self._context = context
-        return self.cache
-
-    def tpc_begin(self, transaction):
-        pass
-
-    def tpc_vote(self, transaction):
-        pass
-
-    def tpc_finish(self, transaction):
-        self.reset()
-
-    def tpc_abort(self, transaction):
-        self.reset()
-
-    def abort(self, transaction):
-        self.reset()
-
-    def commit(self, transaction):
-        pass
-
-    def reset(self):
-        self._joined = False
-        self._context = None
-        self.cache = {}
-
-
-transaction_cache = Cache(transaction.manager)
-
-
 class Locator(object):
 
     def __init__(self, container, get):
@@ -111,8 +60,8 @@ def __call__(self, oid):
             LocationProxy(contained), self.container, contained.__name__)
 
 
+@implementer(interfaces.IResults)
 class Results(object):
-    implements(interfaces.IResults)
 
     def __init__(self, context, all_results, selected_results,
                  wrapper=None, locate_to=None):
@@ -151,8 +100,8 @@ def __iter__(self):
             yield self.get(uid)
 
 
+@implementer(interfaces.IResults)
 class NoResults(object):
-    implements(interfaces.IResults)
 
     count = 0
     total = 0
@@ -221,51 +170,51 @@ def get(self, key):
     def report(self, over=0):
         all_timing = sorted(self.timing.values(), key=lambda t: t.start_order)
         if not len(all_timing):
-            return
+            return  # pragma: no cover (peephole optimizer interferes)
         total_post = 0 if self.post is None else self.post.total
         total_terms = all_timing[0].total
         if (total_terms + total_post) < over:
             return
         indent = 0
         order = [all_timing[0].end_order]
         logger.info(
-            'Catalog query toke {:.4f}s for terms, {:.4f}s to finish.'.format(
+            'Catalog query took {:.4f}s for terms, {:.4f}s to finish.'.format(
                 total_terms, total_post))
         for timing in all_timing:
-            if timing.start_order < order[-1]:
+            if order == [] or timing.start_order < order[-1]:
                 indent += 4
                 order.append(timing.end_order)
+            total = timing.total and '{:.4f}s'.format(timing.total) or '?'
             logger.info(
-                '{} {:.4f}s: {}.'.format(
-                    ' ' * indent, timing.total, str(timing.key)))
-            if timing.end_order > order[-1]:
+                '{} {}: {}.'.format(
+                    ' ' * indent, total, str(timing.key)))
+            if timing.end_order and len(order) \
+               and timing.end_order > order[-1]:
                 indent -= 4
                 order.pop()
 
 
+@implementer(interfaces.IQuery)
 class Query(object):
-    implements(interfaces.IQuery)
 
     def searchResults(
             self, query, context=None, sort_field=None, limit=None,
-            reverse=False, start=0, caching=False, timing=HURRY_QUERY_TIMING,
+            reverse=False, start=0, caching=None, timing=HURRY_QUERY_TIMING,
             wrapper=None, locate_to=None):
 
         if context is None:
             context = getSiteManager()
         else:
             context = IComponentLookup(context)
 
-        if caching is True:
-            cache = transaction_cache.use(context)
-        elif caching is False:
+        if caching in (True, False, None):
             cache = {}
         else:
             # A custom cache object was injected, use it.
             cache = caching
 
         timer = None
-        if timing is not False:
+        if timing:
             timer = cache = TimingAwareCache(cache)
         all_results = query.cached_apply(cache, context)
         if not all_results:
@@ -328,8 +277,8 @@ def searchResults(
             context, all_results, selected_results, wrapper, locate_to)
 
 
+@implementer(interfaces.ITerm)
 class Term(object):
-    implements(interfaces.ITerm)
 
     def key(self, context=None):
         raise NotImplementedError()
@@ -369,7 +318,7 @@ class And(Term):
 
     def __init__(self, *terms, **kwargs):
         self.terms = terms
-        self.weighted = kwargs.get('weigthed', False)
+        self.weighted = kwargs.get('weighted', False)
 
     def apply(self, cache, context=None):
         results = []
@@ -380,9 +329,6 @@ def apply(self, cache, context=None):
                 return result
             results.append(result)
 
-        if len(results) == 0:
-            return IFSet()
-
         if len(results) == 1:
             return results[0]
 
@@ -438,14 +384,15 @@ def __init__(self, *terms):
 
     def apply(self, cache, context=None):
         results = []
+
         for index, term in enumerate(self.terms):
             result = term.cached_apply(cache, context)
             # If we do not have any results for the first index, just
             # return an empty set and stop here.
             if not result:
                 if not index:
                     return IFSet()
-                continue
+                continue  # pragma: no cover (peephole optimizer interferes)
             results.append(result)
 
         result = results.pop(0)
@@ -500,9 +447,9 @@ def key(self, context=None):
 
 class IndexTerm(Term):
 
-    def __init__(self, (catalog_name, index_name)):
-        self.catalog_name = catalog_name
-        self.index_name = index_name
+    def __init__(self, catalog_name__and__index_name):
+        self.catalog_name = catalog_name__and__index_name[0]
+        self.index_name = catalog_name__and__index_name[1]
 
     def getIndex(self, context):
         catalog = getUtility(ICatalog, self.catalog_name, context)