Hi Andy, here are some suggestions and comments. The only major issue is the one to do with create_sorted_insts(), unless I misunderstood what it is doing. https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py File Lib/test/test_compare.py (right): https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:137: class ComparisonTest2(unittest.TestCase): Would be nice to have better names than test 1 and test 2. Maybe MiscTest and ComparisonResultTest, since the new class seems to be testing the result of comparing various objects? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:328: def assert_insts(self, i1, i2, equal, comp, i1_meth=(), i2_meth=()): Maybe assert_comparisons() is a better name? Or perhaps two separate driver methods: assert_equality_only(a, b, True/False) assert_total_order(a, b, ±1/0) https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:340: None means: Equality comparison not supported Are there any such cases? Otherwise, this is misleading. https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:347: None means: Order comparison not supported I guess this doesn’t account for partially ordered objects like sets that are neither supersets nor subsets? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:375: self.assertEqual(i1 == i2, id(i1) == id(i2)) Wouldn’t it be more obvious to do “i1 is i2”? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:480: self.assert_insts(i1, i2, False, id(i1)-id(i2)) What is the point of that last id() calculation? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:511: self.assert_insts(insts_a[0], insts_b[0], True, 0, I don’t get the sorted-by-identity business. The id() values are arbitrary, so how do we know that the first objects in each list are going to be equal? Surely we are just lucky that the id() values are memory addresses allocated in increasing order? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:550: class Class_str(str): Maybe StrSubclass would be more obvious? https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:554: def some_func(self): Does not appear to be used https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:569: self.assert_insts(c1, c1, True, 0, Class_str.meth, Class_str.meth) Can all these empty-tuple meth-s can be dropped? Also I suggest keywords for some of the other parameters, or maybe it would be clearer like: assert_total_order(c1, c1, 0) as I suggested above https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:591: i1 = int(10001) These kind of objects would probably read a whole lot better if they were named something like i1 → int_10001_a i4 → int_10001_b d5 → decimal_42p0 https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#new... Lib/test/test_compare.py:881: def test__internal_is_value_comparable(self): Why the double underscore? https://bugs.python.org/review/12067/