Fix a bug in defining strategies on subtypes

If you had a pattern of usage that looks like this:

    mapper = SpecificationMapper()
    mapper.define_specification_for_instances(BBA, const(0))
    mapper.define_specification_for_instances(A, const(1))
    mapper.define_specification_for_instances(BBB, const(2))
    assert mapper.specification_for(BBB()) == 2

This would *sometimes* produce the wrong answer and sometimes not,
depending on the vagaries of dict iteration order (which is
randomized for each process I think but haven't checked). Essentially
the culprit was an entirely bogus implementation of topologically
sorting by the subclass relationship. There is now a non-bogus
implementation.

CHANGES.txt

@@ -117,4 +117,12 @@
       tests run with a seed calculated from the function they're testing so you
       should still get a good distribution of test cases.
     * Add a mechanism for more conveniently defining tests which just sample
-      from some elements.
+      from some collection.
+    * Fix for a really subtle bug deep in the internals of the strategy table.
+      In some circumstances if you were to define instance stratagies for both
+      a parent class and one or more of its subclasses you would under some
+      circumstances get the strategy for the wrong superclass of an instance.
+      It is very unlikely anyone has ever encountered this in the wild, but it
+      is conceivably possible given that a mix of namedtuple and tuple are used
+      fairly extensively inside hypothesis which do exhibit this pattern of
+      strategy.

src/hypothesis/internal/specmapper.py

@@ -105,8 +105,9 @@ def __find_specification_handlers_for(self, descriptor):
                 yield h
 
     def __instance_handlers(self, tk):
-        for c, hs in sorted(
-                self.instance_mappers.items(), key=lambda x: ClassSorter(x[0])
+        for c, hs in sort_in_subclass_order(
+                self.instance_mappers.items(),
+                lambda x: x[0],
         ):
             if issubclass(tk, c):
                 for h in reversed(hs):
@@ -116,19 +117,36 @@ def missing_specification(self, descriptor):
         raise MissingSpecification(descriptor)
 
 
-@total_ordering
-class ClassSorter(object):
-
-    def __init__(self, cls):
-        self.cls = cls
-
-    def __lt__(self, that):
-        if issubclass(self.cls, that.cls):
-            return True
-        elif issubclass(that.cls, self.cls):
-            return False
-        else:
-            return self.cls.__name__ < that.cls.__name__
+def sort_in_subclass_order(xs, get_class=lambda x: x):
+    if len(xs) <= 1:
+        return list(xs)
+    by_class = {}
+    for x in xs:
+        c = get_class(x)
+        by_class.setdefault(c, []).append(x)
+    classes = list(by_class.keys())
+    subclasses = {}
+    for c in classes:
+        children = subclasses.setdefault(c, [])
+        for d in classes:
+            if c != d and issubclass(d, c):
+                children.append(d)
+    in_order = []
+
+    def recurse(c):
+        if c in in_order:
+            return
+        for d in subclasses[c]:
+            recurse(d)
+        in_order.append(c)
+
+    while classes:
+        recurse(classes.pop())
+    return [
+        x
+        for c in in_order
+        for x in by_class[c]
+    ]
 
 
 def typekey(x):

tests/test_specmapper.py

@@ -1,8 +1,11 @@
 from hypothesis.internal.specmapper import (
     SpecificationMapper,
     MissingSpecification,
-    next_in_chain
+    next_in_chain,
+    sort_in_subclass_order,
 )
+from hypothesis import given
+from hypothesis.descriptors import sampled_from
 import pytest
 from collections import namedtuple
 import random
@@ -306,3 +309,104 @@ def trivial(x):
         s.define_specification_for_instances(c, trivial(i))
         assert s.specification_for(c()) == i
         values[c] = i
+
+
+class A(object):
+    pass
+
+
+class AA(A):
+    pass
+
+
+class AAA(AA):
+    pass
+
+
+class AAB(AA):
+    pass
+
+
+class AB(A):
+    pass
+
+
+class ABA(AB):
+    pass
+
+
+class ABB(AB):
+    pass
+
+
+class B(object):
+    pass
+
+
+class BA(B):
+    pass
+
+
+class BAA(BA):
+    pass
+
+
+class BAB(BA):
+    pass
+
+
+class BB(B):
+    pass
+
+
+class BBA(BB):
+    pass
+
+
+class BBB(AB):
+    pass
+
+
+all_test_classes = [
+    A, AA, AAA, AAB, AB, ABA, ABB,
+    B, BA, BAA, BAB, BB, BBA, BBB
+]
+
+
+def find_most_specific(x, classes):
+    best = None
+    for c in classes:
+        if isinstance(x, c):
+            if best is None or issubclass(c, best):
+                best = c
+    return best
+
+
+@given({sampled_from(all_test_classes)}, random.Random)
+def test_chooses_most_specific_subclass(classes, r):
+    classes = list(classes)
+    random.shuffle(classes)
+    classes = tuple(classes)
+    mapper = SpecificationMapper()
+    for i in xrange(len(classes)):
+        mapper.define_specification_for_instances(
+            classes[i],
+            const(i),
+        )
+    for c in all_test_classes:
+        if issubclass(c, classes):
+            x = c()
+            correct_choice = find_most_specific(x, classes)
+            i = mapper.specification_for(x)
+            assert i == classes.index(correct_choice)
+
+
+@given({sampled_from(all_test_classes)}, random.Random)
+def test_class_sorter_topologically_sorts_wrt_subclassing(classes, random):
+    classes = list(classes)
+    random.shuffle(classes)
+    in_order = sort_in_subclass_order(classes)
+    n = len(classes)
+    for i in xrange(n):
+        for j in xrange(i+1, n):
+            assert not issubclass(in_order[j], in_order[i])