Python 3 - Fix some portability issues with class / instance attributes

[zturner]

Summary

TBH I'm honestly not sure what the problem was before, or why this fixes
it.  But what I can tell from debugging is that under Py3,
`sortMethodsUsing` is treated as a class attribute, but it was being
accessed as `self.sortMethodsUsing`.  What the full implications of this
are I don't quite know, but the symptom here was that the value we were
assigning to it -- the global lambda `cmp_` -- was being treated as a
bound method instead of a global method.  So it was expecting to be called
with a `self` argument, and I got an exception saying the method expected 3
arguments but received 2.

Why exactly this happens in Python 3, and why exactly this *doesn't*
happen in Python 2, even though the syntax was identical in both cases
are not clear to me.

[tfiala]

LGTM. We just weren't using the reversed_cmp() code it seems?

[zturner]

Derp, seems I uploaded this review twice. Now we have split threads. Anyway, there were a couple issues. The issue that was blocking me was related to the instance attributes. Specifically we were writing this:

testMethodPrefix = 'test'
suiteClass = suite.TestSuite
sortTestMethodsUsing = cmp_
_top_level_dir = None

instead of this:

def __init__(self):
    self.testMethodPrefix = 'test'
    self.sortTestMethodsUsing = cmp_
    self.testMethodPrefix = 'test'
    self._top_level_dir = None

But we were still accessing the variables as self.sortTestMethodsUsing. I'm not sure how this works in 2.7, but for whatever reason, it doesn't work in 3.x. sortTestMethodsUsing is treated as a bound method here (i.e. instance method), so it expects a self argument.

[zturner]

The other issues with reversed_cmp_ were still valid issues that I just fixed at the same time since they were related, but the main one was the instance / class attribute incompatibility.

third_party/Python/module/unittest2/unittest2/__init__.py

 else:
     cmp_ = cmp
 
+reversed_cmp_ = lambda x, y: -cmp_(x,y)
+
 __all__ = ['TestResult', 'TestCase', 'TestSuite',
            'TextTestRunner', 'TestLoader', 'FunctionTestCase', 'main',
            'defaultTestLoader', 'SkipTest', 'skip', 'skipIf', 'skipUnless',

third_party/Python/module/unittest2/unittest2/loader.py

 """Loading unittests."""
 
+import functools
 import os
 import re
 import sys

 
 __unittest = True
 
-
-def _CmpToKey(mycmp):
-    'Convert a cmp= function into a key= function'
-    class K(object):
-        def __init__(self, obj):
-            self.obj = obj
-        def __lt__(self, other):
-            return mycmp(self.obj, other.obj) == -1
-    return K
-
-
 # what about .pyc or .pyo (etc)
 # we would need to avoid loading the same tests multiple times
 # from '.py', '.pyc' *and* '.pyo'

     This class is responsible for loading tests according to various criteria
     and returning them wrapped in a TestSuite
     """
-    testMethodPrefix = 'test'
-    sortTestMethodsUsing = cmp_
-    suiteClass = suite.TestSuite
-    _top_level_dir = None
+
+    def __init__(self):
+        self.testMethodPrefix = 'test'
+        self.sortTestMethodsUsing = cmp_
+        self.suiteClass = suite.TestSuite
+        self._top_level_dir = None
 
     def loadTestsFromTestCase(self, testCaseClass):
         """Return a suite of all tests cases contained in testCaseClass"""

                 hasattr(getattr(testCaseClass, attrname), '__call__')
         testFnNames = list(filter(isTestMethod, dir(testCaseClass)))
         if self.sortTestMethodsUsing:
-            testFnNames.sort(key=_CmpToKey(self.sortTestMethodsUsing))
+            testFnNames.sort(key=functools.cmp_to_key(self.sortTestMethodsUsing))
         return testFnNames
 
     def discover(self, start_dir, pattern='test*.py', top_level_dir=None):

third_party/Python/module/unittest2/unittest2/test/test_loader.py

     # "Function to be used to compare method names when sorting them in
     # getTestCaseNames() and all the loadTestsFromX() methods"
     def test_sortTestMethodsUsing__loadTestsFromTestCase(self):
-        def reversed_cmp(x, y):
-            return -cmp(x, y)
-
         class Foo(unittest2.TestCase):
             def test_1(self): pass
             def test_2(self): pass
 
         loader = unittest2.TestLoader()
-        loader.sortTestMethodsUsing = reversed_cmp
+        loader.sortTestMethodsUsing = unittest2.reversed_cmp_
 
         tests = loader.suiteClass([Foo('test_2'), Foo('test_1')])
         self.assertEqual(loader.loadTestsFromTestCase(Foo), tests)

     # "Function to be used to compare method names when sorting them in
     # getTestCaseNames() and all the loadTestsFromX() methods"
     def test_sortTestMethodsUsing__loadTestsFromModule(self):
-        def reversed_cmp(x, y):
-            return -cmp(x, y)
-
         m = types.ModuleType('m')
         class Foo(unittest2.TestCase):
             def test_1(self): pass

         m.Foo = Foo
 
         loader = unittest2.TestLoader()
-        loader.sortTestMethodsUsing = reversed_cmp
+        loader.sortTestMethodsUsing = unittest2.reversed_cmp_
 
         tests = [loader.suiteClass([Foo('test_2'), Foo('test_1')])]
         self.assertEqual(list(loader.loadTestsFromModule(m)), tests)

     # "Function to be used to compare method names when sorting them in
     # getTestCaseNames() and all the loadTestsFromX() methods"
     def test_sortTestMethodsUsing__loadTestsFromName(self):
-        def reversed_cmp(x, y):
-            return -cmp(x, y)
-
         m = types.ModuleType('m')
         class Foo(unittest2.TestCase):
             def test_1(self): pass

         m.Foo = Foo
 
         loader = unittest2.TestLoader()
-        loader.sortTestMethodsUsing = reversed_cmp
+        loader.sortTestMethodsUsing = unittest2.reversed_cmp_
 
         tests = loader.suiteClass([Foo('test_2'), Foo('test_1')])
         self.assertEqual(loader.loadTestsFromName('Foo', m), tests)

     # "Function to be used to compare method names when sorting them in
     # getTestCaseNames() and all the loadTestsFromX() methods"
     def test_sortTestMethodsUsing__loadTestsFromNames(self):
-        def reversed_cmp(x, y):
-            return -cmp(x, y)
-
         m = types.ModuleType('m')
         class Foo(unittest2.TestCase):
             def test_1(self): pass

         m.Foo = Foo
 
         loader = unittest2.TestLoader()
-        loader.sortTestMethodsUsing = reversed_cmp
+        loader.sortTestMethodsUsing = unittest2.reversed_cmp_
 
         tests = [loader.suiteClass([Foo('test_2'), Foo('test_1')])]
         self.assertEqual(list(loader.loadTestsFromNames(['Foo'], m)), tests)

     #
     # Does it actually affect getTestCaseNames()?
     def test_sortTestMethodsUsing__getTestCaseNames(self):
-        def reversed_cmp(x, y):
-            return -cmp(x, y)
-
         class Foo(unittest2.TestCase):
             def test_1(self): pass
             def test_2(self): pass
 
         loader = unittest2.TestLoader()
-        loader.sortTestMethodsUsing = reversed_cmp
+        loader.sortTestMethodsUsing = unittest2.reversed_cmp_
 
         test_names = ['test_2', 'test_1']
         self.assertEqual(loader.getTestCaseNames(Foo), test_names)

     # "The default value is the built-in cmp() function"
     def test_sortTestMethodsUsing__default_value(self):
         loader = unittest2.TestLoader()
-        self.assertTrue(loader.sortTestMethodsUsing is cmp)
+        self.assertTrue(loader.sortTestMethodsUsing is unittest2.cmp_)
 
     # "it can be set to None to disable the sort."
     #