Merge lp:~ajkavanagh/charm-helpers/fix-alphanumerica-comparisons into lp:charm-helpers

[Alex Kavanagh]

A bit of an reorganisation due to finding additional comparisons in charm-helpers itself.
This moves the BasicStringComparator to charmhelpers.core.strutils and the (renamed)
CompareHostRelease() class to charmhelpers.core.host(.ubuntu), but accessible from
charmhelpers.core.host.

This means a removal of a circular import and enables the use of the function from
non-openstack charms.

[David Ames (thedac)]

One fix request (import) and a couple of clarifying questions in-line.

review: Needs Fixing

[Alex Kavanagh (ajkavanagh)]

Good catch on the import. See my other comments addressing your questions. Thanks.

[lp:~ajkavanagh/charm-helpers/fix-alphanumerica-comparisons]

723. By Alex Kavanagh on 2017-03-26

Fix incorrect import for amulet/utils.py

[Alex Kavanagh (ajkavanagh)]

Having had a bit more time to think on the import question of CompareHostReleases() in non Ubuntu distros, I realise that you do need to provide a blank implementation of it, otherwise the modules where is _might_ be used will fail to import. This will break too much, so I'll provide a NotImplemented version of it.

[lp:~ajkavanagh/charm-helpers/fix-alphanumerica-comparisons]

724. By Alex Kavanagh on 2017-03-26

Provide "not-implemented" version of CompareHostReleases() for Centos

This provides a CompareHostReleases() class for Centos that raises
NotImplementedError() if it is used. This is provided so that modules
that import the class will still work on Centos, but functions that
try to use it won't. The functions that are using CompareHostReleases()
are currently specific to Ubuntu.

[David Ames (thedac)]

This looks good.
Merging.

review: Approve

=== modified file 'charmhelpers/contrib/hardening/ssh/checks/config.py'
--- charmhelpers/contrib/hardening/ssh/checks/config.py	2016-09-23 08:40:19 +0000
+++ charmhelpers/contrib/hardening/ssh/checks/config.py	2017-03-26 10:20:30 +0000
@@ -27,7 +27,10 @@
     apt_install,
     apt_update,
 )
-from charmhelpers.core.host import lsb_release
+from charmhelpers.core.host import (
+    lsb_release,
+    CompareHostReleases,
+)
 from charmhelpers.contrib.hardening.audits.file import (
     TemplatedFile,
     FileContentAudit,
@@ -68,7 +71,8 @@
                    'weak': default + ',hmac-sha1'}
 
         # Use newer ciphers on Ubuntu Trusty and above
-        if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':
+        _release = lsb_release()['DISTRIB_CODENAME'].lower()
+        if CompareHostReleases(_release) >= 'trusty':
             log("Detected Ubuntu 14.04 or newer, using new macs", level=DEBUG)
             macs = macs_66
 
@@ -96,7 +100,8 @@
                   'weak': weak}
 
         # Use newer kex on Ubuntu Trusty and above
-        if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':
+        _release = lsb_release()['DISTRIB_CODENAME'].lower()
+        if CompareHostReleases(_release) >= 'trusty':
             log('Detected Ubuntu 14.04 or newer, using new key exchange '
                 'algorithms', level=DEBUG)
             kex = kex_66
@@ -119,7 +124,8 @@
                       'weak': default + ',aes256-cbc,aes192-cbc,aes128-cbc'}
 
         # Use newer ciphers on ubuntu Trusty and above
-        if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':
+        _release = lsb_release()['DISTRIB_CODENAME'].lower()
+        if CompareHostReleases(_release) >= 'trusty':
             log('Detected Ubuntu 14.04 or newer, using new ciphers',
                 level=DEBUG)
             cipher = ciphers_66
@@ -291,7 +297,8 @@
         self.fail_cases = []
         settings = utils.get_settings('ssh')
 
-        if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':
+        _release = lsb_release()['DISTRIB_CODENAME'].lower()
+        if CompareHostReleases(_release) >= 'trusty':
             if not settings['server']['weak_hmac']:
                 self.pass_cases.append(r'^MACs.+,hmac-ripemd160$')
             else:
@@ -364,7 +371,8 @@
         self.fail_cases = []
         settings = utils.get_settings('ssh')
 
-        if lsb_release()['DISTRIB_CODENAME'].lower() >= 'trusty':
+        _release = lsb_release()['DISTRIB_CODENAME'].lower()
+        if CompareHostReleases(_release) >= 'trusty':
             if not settings['server']['weak_hmac']:
                 self.pass_cases.append(r'^MACs.+,hmac-ripemd160$')
             else:

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2017-03-14 11:46:12 +0000
+++ charmhelpers/contrib/network/ip.py	2017-03-26 10:20:30 +0000
@@ -31,6 +31,7 @@
 
 from charmhelpers.core.host import (
     lsb_release,
+    CompareHostReleases,
 )
 
 try:
@@ -537,11 +538,8 @@
 
 def assert_charm_supports_ipv6():
     """Check whether we are able to support charms ipv6."""
-    # workaround circular import -- means we have a code organisation issue,
-    # but charms depend on this location for this function ...
-    from charmhelpers.contrib.openstack.utils import CompareUbuntuReleases
     release = lsb_release()['DISTRIB_CODENAME'].lower()
-    if CompareUbuntuReleases(release) < "trusty":
+    if CompareHostReleases(release) < "trusty":
         raise Exception("IPv6 is not supported in the charms for Ubuntu "
                         "versions less than Trusty 14.04")
 

=== modified file 'charmhelpers/contrib/openstack/amulet/utils.py'
--- charmhelpers/contrib/openstack/amulet/utils.py	2017-02-27 22:11:45 +0000
+++ charmhelpers/contrib/openstack/amulet/utils.py	2017-03-26 10:20:30 +0000
@@ -40,6 +40,7 @@
     AmuletUtils
 )
 from charmhelpers.core.decorators import retry_on_exception
+from charmhelpers.core.host import CompareHostReleases
 
 DEBUG = logging.DEBUG
 ERROR = logging.ERROR
@@ -1255,7 +1256,7 @@
         contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf',
                                            fatal=True)
         ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs')
-        if ubuntu_release <= 'trusty':
+        if CompareHostReleases(ubuntu_release) <= 'trusty':
             memcache_listen_addr = 'ip6-localhost'
         else:
             memcache_listen_addr = '::1'

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2017-03-14 11:46:12 +0000
+++ charmhelpers/contrib/openstack/context.py	2017-03-26 10:20:30 +0000
@@ -59,6 +59,7 @@
     write_file,
     pwgen,
     lsb_release,
+    CompareHostReleases,
 )
 from charmhelpers.contrib.hahelpers.cluster import (
     determine_apache_port,
@@ -94,7 +95,6 @@
     git_determine_usr_bin,
     git_determine_python_path,
     enable_memcache,
-    CompareUbuntuReleases,
 )
 from charmhelpers.core.unitdata import kv
 
@@ -1606,7 +1606,7 @@
             # Trusty version of memcached does not support ::1 as a listen
             # address so use host file entry instead
             release = lsb_release()['DISTRIB_CODENAME'].lower()
-            if CompareUbuntuReleases(release) > 'trusty':
+            if CompareHostReleases(release) > 'trusty':
                 ctxt['memcache_server'] = '::1'
             else:
                 ctxt['memcache_server'] = 'ip6-localhost'

=== modified file 'charmhelpers/contrib/openstack/neutron.py'
--- charmhelpers/contrib/openstack/neutron.py	2016-11-25 22:50:01 +0000
+++ charmhelpers/contrib/openstack/neutron.py	2017-03-26 10:20:30 +0000
@@ -23,7 +23,10 @@
     ERROR,
 )
 
-from charmhelpers.contrib.openstack.utils import os_release
+from charmhelpers.contrib.openstack.utils import (
+    os_release,
+    CompareOpenStackReleases,
+)
 
 
 def headers_package():
@@ -198,7 +201,8 @@
         },
         'plumgrid': {
             'config': '/etc/neutron/plugins/plumgrid/plumgrid.ini',
-            'driver': 'neutron.plugins.plumgrid.plumgrid_plugin.plumgrid_plugin.NeutronPluginPLUMgridV2',
+            'driver': ('neutron.plugins.plumgrid.plumgrid_plugin'
+                       '.plumgrid_plugin.NeutronPluginPLUMgridV2'),
             'contexts': [
                 context.SharedDBContext(user=config('database-user'),
                                         database=config('database'),
@@ -225,7 +229,7 @@
             'server_services': ['neutron-server']
         }
     }
-    if release >= 'icehouse':
+    if CompareOpenStackReleases(release) >= 'icehouse':
         # NOTE: patch in ml2 plugin for icehouse onwards
         plugins['ovs']['config'] = '/etc/neutron/plugins/ml2/ml2_conf.ini'
         plugins['ovs']['driver'] = 'neutron.plugins.ml2.plugin.Ml2Plugin'
@@ -233,10 +237,10 @@
                                              'neutron-plugin-ml2']
         # NOTE: patch in vmware renames nvp->nsx for icehouse onwards
         plugins['nvp'] = plugins['nsx']
-    if release >= 'kilo':
+    if CompareOpenStackReleases(release) >= 'kilo':
         plugins['midonet']['driver'] = (
             'neutron.plugins.midonet.plugin.MidonetPluginV2')
-    if release >= 'liberty':
+    if CompareOpenStackReleases(release) >= 'liberty':
         plugins['midonet']['driver'] = (
             'midonet.neutron.plugin_v1.MidonetPluginV2')
         plugins['midonet']['server_packages'].remove(
@@ -244,10 +248,11 @@
         plugins['midonet']['server_packages'].append(
             'python-networking-midonet')
         plugins['plumgrid']['driver'] = (
-            'networking_plumgrid.neutron.plugins.plugin.NeutronPluginPLUMgridV2')
+            'networking_plumgrid.neutron.plugins'
+            '.plugin.NeutronPluginPLUMgridV2')
         plugins['plumgrid']['server_packages'].remove(
             'neutron-plugin-plumgrid')
-    if release >= 'mitaka':
+    if CompareOpenStackReleases(release) >= 'mitaka':
         plugins['nsx']['server_packages'].remove('neutron-plugin-vmware')
         plugins['nsx']['server_packages'].append('python-vmware-nsx')
         plugins['nsx']['config'] = '/etc/neutron/nsx.ini'

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2017-03-20 13:44:46 +0000
+++ charmhelpers/contrib/openstack/utils.py	2017-03-26 10:20:30 +0000
@@ -33,9 +33,7 @@
 
 from charmhelpers.contrib.network import ip
 
-from charmhelpers.core import (
-    unitdata,
-)
+from charmhelpers.core import unitdata
 
 from charmhelpers.core.hookenv import (
     action_fail,
@@ -55,6 +53,8 @@
     application_version_set,
 )
 
+from charmhelpers.core.strutils import BasicStringComparator
+
 from charmhelpers.contrib.storage.linux.lvm import (
     deactivate_lvm_volume_group,
     is_lvm_physical_volume,
@@ -97,24 +97,6 @@
 DISTRO_PROPOSED = ('deb http://archive.ubuntu.com/ubuntu/ %s-proposed '
                    'restricted main multiverse universe')
 
-UBUNTU_RELEASES = (
-    'lucid',
-    'maverick',
-    'natty',
-    'oneiric',
-    'precise',
-    'quantal',
-    'raring',
-    'saucy',
-    'trusty',
-    'utopic',
-    'vivid',
-    'wily',
-    'xenial',
-    'yakkety',
-    'zesty',
-)
-
 OPENSTACK_RELEASES = (
     'diablo',
     'essex',
@@ -272,59 +254,6 @@
 DEFAULT_LOOPBACK_SIZE = '5G'
 
 
-class BasicStringComparator(object):
-    """Provides a class that will compare strings from an iterator type object.
-    Used to provide > and < comparisons on strings that may not necessarily be
-    alphanumerically ordered.  e.g. OpenStack or Ubuntu releases AFTER the
-    z-wrap.
-    """
-
-    _list = None
-
-    def __init__(self, item):
-        if self._list is None:
-            raise Exception("Must define the _list in the class definition!")
-        try:
-            self.index = self._list.index(item)
-        except Exception:
-            raise KeyError("Item '{}' is not in list '{}'"
-                           .format(item, self._list))
-
-    def __eq__(self, other):
-        assert isinstance(other, str) or isinstance(other, self.__class__)
-        return self.index == self._list.index(other)
-
-    def __ne__(self, other):
-        return not self.__eq__(other)
-
-    def __lt__(self, other):
-        assert isinstance(other, str) or isinstance(other, self.__class__)
-        return self.index < self._list.index(other)
-
-    def __ge__(self, other):
-        return not self.__lt__(other)
-
-    def __gt__(self, other):
-        assert isinstance(other, str) or isinstance(other, self.__class__)
-        return self.index > self._list.index(other)
-
-    def __le__(self, other):
-        return not self.__gt__(other)
-
-    def __str__(self):
-        """Always give back the item at the index so it can be used in
-        comparisons like:
-
-        s_mitaka = CompareOpenStack('mitaka')
-        s_newton = CompareOpenstack('newton')
-
-        assert s_newton > s_mitaka
-
-        @returns: <string>
-        """
-        return self._list[self.index]
-
-
 class CompareOpenStackReleases(BasicStringComparator):
     """Provide comparisons of OpenStack releases.
 
@@ -336,17 +265,6 @@
     _list = OPENSTACK_RELEASES
 
 
-class CompareUbuntuReleases(BasicStringComparator):
-    """Provide comparisons of Ubuntu releases.
-
-    Use in the form of
-
-    if CompareUbuntuReleases(release) > 'trusty':
-        # do something with mitaka
-    """
-    _list = UBUNTU_RELEASES
-
-
 def error_out(msg):
     juju_log("FATAL ERROR: %s" % msg, level='ERROR')
     sys.exit(1)
@@ -2081,9 +1999,7 @@
     if not _release:
         _release = get_os_codename_install_source(source)
 
-    # TODO: this should be changed to a numeric comparison using a known list
-    # of releases and comparing by index.
-    return _release >= 'mitaka'
+    return CompareOpenStackReleases(_release) >= 'mitaka'
 
 
 def token_cache_pkgs(source=None, release=None):

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2017-03-08 06:54:54 +0000
+++ charmhelpers/core/host.py	2017-03-26 10:20:30 +0000
@@ -45,6 +45,7 @@
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 elif __platform__ == "centos":
     from charmhelpers.core.host_factory.centos import (
@@ -52,6 +53,7 @@
         add_new_group,
         lsb_release,
         cmp_pkgrevno,
+        CompareHostReleases,
     )  # flake8: noqa -- ignore F401 for this import
 
 UPDATEDB_PATH = '/etc/updatedb.conf'

=== modified file 'charmhelpers/core/host_factory/centos.py'
--- charmhelpers/core/host_factory/centos.py	2016-08-11 15:47:44 +0000
+++ charmhelpers/core/host_factory/centos.py	2017-03-26 10:20:30 +0000
@@ -2,6 +2,22 @@
 import yum
 import os
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Host releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+
+    def __init__(self, item):
+        raise NotImplementedError(
+            "CompareHostReleases() is not implemented for CentOS")
+
 
 def service_available(service_name):
     # """Determine whether a system service is available."""

=== modified file 'charmhelpers/core/host_factory/ubuntu.py'
--- charmhelpers/core/host_factory/ubuntu.py	2016-08-11 15:47:44 +0000
+++ charmhelpers/core/host_factory/ubuntu.py	2017-03-26 10:20:30 +0000
@@ -1,5 +1,37 @@
 import subprocess
 
+from charmhelpers.core.strutils import BasicStringComparator
+
+
+UBUNTU_RELEASES = (
+    'lucid',
+    'maverick',
+    'natty',
+    'oneiric',
+    'precise',
+    'quantal',
+    'raring',
+    'saucy',
+    'trusty',
+    'utopic',
+    'vivid',
+    'wily',
+    'xenial',
+    'yakkety',
+    'zesty',
+)
+
+
+class CompareHostReleases(BasicStringComparator):
+    """Provide comparisons of Ubuntu releases.
+
+    Use in the form of
+
+    if CompareHostReleases(release) > 'trusty':
+        # do something with mitaka
+    """
+    _list = UBUNTU_RELEASES
+
 
 def service_available(service_name):
     """Determine whether a system service is available"""

=== modified file 'charmhelpers/core/strutils.py'
--- charmhelpers/core/strutils.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/core/strutils.py	2017-03-26 10:20:30 +0000
@@ -68,3 +68,56 @@
         msg = "Unable to interpret string value '%s' as bytes" % (value)
         raise ValueError(msg)
     return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)])
+
+
+class BasicStringComparator(object):
+    """Provides a class that will compare strings from an iterator type object.
+    Used to provide > and < comparisons on strings that may not necessarily be
+    alphanumerically ordered.  e.g. OpenStack or Ubuntu releases AFTER the
+    z-wrap.
+    """
+
+    _list = None
+
+    def __init__(self, item):
+        if self._list is None:
+            raise Exception("Must define the _list in the class definition!")
+        try:
+            self.index = self._list.index(item)
+        except Exception:
+            raise KeyError("Item '{}' is not in list '{}'"
+                           .format(item, self._list))
+
+    def __eq__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index == self._list.index(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __lt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index < self._list.index(other)
+
+    def __ge__(self, other):
+        return not self.__lt__(other)
+
+    def __gt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index > self._list.index(other)
+
+    def __le__(self, other):
+        return not self.__gt__(other)
+
+    def __str__(self):
+        """Always give back the item at the index so it can be used in
+        comparisons like:
+
+        s_mitaka = CompareOpenStack('mitaka')
+        s_newton = CompareOpenstack('newton')
+
+        assert s_newton > s_mitaka
+
+        @returns: <string>
+        """
+        return self._list[self.index]

=== modified file 'tests/contrib/openstack/test_os_utils.py'
--- tests/contrib/openstack/test_os_utils.py	2017-03-20 13:44:46 +0000
+++ tests/contrib/openstack/test_os_utils.py	2017-03-26 10:20:30 +0000
@@ -9,71 +9,10 @@
     def setUp(self):
         super(UtilsTests, self).setUp()
 
-    def test_basic_string_comparator_class_fails_instantiation(self):
-        try:
-            utils.BasicStringComparator('hello')
-            raise Exception("instantiating BasicStringComparator should fail")
-        except Exception as e:
-            assert (str(e) == "Must define the _list in the class definition!")
-
-    def test_basic_string_comparator_class(self):
-
-        class MyComparator(utils.BasicStringComparator):
-
-            _list = ('zomg', 'bartlet', 'over', 'and')
-
-        x = MyComparator('zomg')
-        self.assertEquals(x.index, 0)
-        y = MyComparator('over')
-        self.assertEquals(y.index, 2)
-        self.assertTrue(x == 'zomg')
-        self.assertTrue(x != 'bartlet')
-        self.assertTrue(x == x)
-        self.assertTrue(x != y)
-        self.assertTrue(x < y)
-        self.assertTrue(y > x)
-        self.assertTrue(x < 'bartlet')
-        self.assertTrue(y > 'bartlet')
-        self.assertTrue(x >= 'zomg')
-        self.assertTrue(x <= 'zomg')
-        self.assertTrue(x >= x)
-        self.assertTrue(x <= x)
-        self.assertTrue(y >= 'zomg')
-        self.assertTrue(y <= 'over')
-        self.assertTrue(y >= x)
-        self.assertTrue(x <= y)
-        # ensure that something not in the list dies
-        try:
-            MyComparator('nope')
-            raise Exception("MyComparator('nope') should have failed")
-        except Exception as e:
-            self.assertTrue(isinstance(e, KeyError))
-
-    def test_basic_string_comparator_fails_different_comparators(self):
-
-        class MyComparator1(utils.BasicStringComparator):
-
-            _list = ('the truth is out there'.split(' '))
-
-        class MyComparator2(utils.BasicStringComparator):
-
-            _list = ('no one in space can hear you scream'.split(' '))
-
-        x = MyComparator1('is')
-        y = MyComparator2('you')
-        try:
-            x > y
-            raise Exception("Comparing different comparators should fail")
-        except Exception as e:
-            self.assertTrue(isinstance(e, AssertionError))
-
     def test_compare_openstack_comparator(self):
         self.assertTrue(utils.CompareOpenStackReleases('mitaka') < 'newton')
         self.assertTrue(utils.CompareOpenStackReleases('pike') > 'essex')
 
-    def test_compare_ubuntu_releases(self):
-        self.assertTrue(utils.CompareUbuntuReleases('yakkety') < 'zesty')
-
     @mock.patch.object(utils, 'config')
     @mock.patch('charmhelpers.contrib.openstack.utils.relation_set')
     @mock.patch('charmhelpers.contrib.openstack.utils.relation_ids')
@@ -226,19 +165,19 @@
                              _os_release):
         # Check call with 'release'
         self.assertFalse(utils.enable_memcache(release='icehouse'))
-        self.assertTrue(utils.enable_memcache(release='zebra'))
+        self.assertTrue(utils.enable_memcache(release='ocata'))
         # Check call with 'source'
         _os_release.return_value = None
         _get_os_codename_install_source.return_value = 'icehouse'
         self.assertFalse(utils.enable_memcache(source='distro'))
         _os_release.return_value = None
-        _get_os_codename_install_source.return_value = 'zebra'
+        _get_os_codename_install_source.return_value = 'ocata'
         self.assertTrue(utils.enable_memcache(source='distro'))
         # Check call with 'package'
         _os_release.return_value = 'icehouse'
         _get_os_codename_install_source.return_value = None
         self.assertFalse(utils.enable_memcache(package='pkg1'))
-        _os_release.return_value = 'zebra'
+        _os_release.return_value = 'ocata'
         _get_os_codename_install_source.return_value = None
         self.assertTrue(utils.enable_memcache(package='pkg1'))
 

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2017-03-08 06:54:54 +0000
+++ tests/core/test_host.py	2017-03-26 10:20:30 +0000
@@ -1814,4 +1814,13 @@
 
         self.assertTrue(handle.read.call_count == 1)
         self.assertTrue(handle.seek.call_count == 1)
-        handle.write.assert_called_once_with('PRUNEPATHS="/tmp /srv/node /tmp/test"')
+        handle.write.assert_called_once_with(
+            'PRUNEPATHS="/tmp /srv/node /tmp/test"')
+
+
+class TestHostCompator(TestCase):
+
+    def test_compare_ubuntu_releases(self):
+        from charmhelpers.osplatform import get_platform
+        if get_platform() == 'ubuntu':
+            self.assertTrue(host.CompareHostReleases('yakkety') < 'zesty')

=== modified file 'tests/core/test_strutils.py'
--- tests/core/test_strutils.py	2015-09-17 11:04:18 +0000
+++ tests/core/test_strutils.py	2017-03-26 10:20:30 +0000
@@ -47,3 +47,61 @@
 
         self.assertRaises(ValueError, strutils.bytes_from_string, None)
         self.assertRaises(ValueError, strutils.bytes_from_string, 'foo')
+
+    def test_basic_string_comparator_class_fails_instantiation(self):
+        try:
+            strutils.BasicStringComparator('hello')
+            raise Exception("instantiating BasicStringComparator should fail")
+        except Exception as e:
+            assert (str(e) == "Must define the _list in the class definition!")
+
+    def test_basic_string_comparator_class(self):
+
+        class MyComparator(strutils.BasicStringComparator):
+
+            _list = ('zomg', 'bartlet', 'over', 'and')
+
+        x = MyComparator('zomg')
+        self.assertEquals(x.index, 0)
+        y = MyComparator('over')
+        self.assertEquals(y.index, 2)
+        self.assertTrue(x == 'zomg')
+        self.assertTrue(x != 'bartlet')
+        self.assertTrue(x == x)
+        self.assertTrue(x != y)
+        self.assertTrue(x < y)
+        self.assertTrue(y > x)
+        self.assertTrue(x < 'bartlet')
+        self.assertTrue(y > 'bartlet')
+        self.assertTrue(x >= 'zomg')
+        self.assertTrue(x <= 'zomg')
+        self.assertTrue(x >= x)
+        self.assertTrue(x <= x)
+        self.assertTrue(y >= 'zomg')
+        self.assertTrue(y <= 'over')
+        self.assertTrue(y >= x)
+        self.assertTrue(x <= y)
+        # ensure that something not in the list dies
+        try:
+            MyComparator('nope')
+            raise Exception("MyComparator('nope') should have failed")
+        except Exception as e:
+            self.assertTrue(isinstance(e, KeyError))
+
+    def test_basic_string_comparator_fails_different_comparators(self):
+
+        class MyComparator1(strutils.BasicStringComparator):
+
+            _list = ('the truth is out there'.split(' '))
+
+        class MyComparator2(strutils.BasicStringComparator):
+
+            _list = ('no one in space can hear you scream'.split(' '))
+
+        x = MyComparator1('is')
+        y = MyComparator2('you')
+        try:
+            x > y
+            raise Exception("Comparing different comparators should fail")
+        except Exception as e:
+            self.assertTrue(isinstance(e, AssertionError))