zturner created this revision.
zturner added a reviewer: tfiala.
zturner added a subscriber: lldb-commits.

    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.

    Why exactly this happens in Python 3, and why exactly this *doesn't*
    happen in Python 2 are not clear to me.

It's worth mentioning that I tried to switch unittest2 over to unittest -- the 
builtin implementation -- as a means of solving the Py2 / Py3 portability 
problems, and it was non-trivial at best, at least with my limited Python 
knowledge.  The reason is that we have many custom decorators that rely on 
internals of unittest2 that are even documented in the source code as 
implementation details.  But we use them, and they don't exist in the Python 3 
version of unittest2, and as a result I don't know how to do the port smoothly.

Honestly, I couldn't even get it working under Python 2.7's built-in unittest 
implementation for similar reasons (relying on implementation details).  It 
would be nice to do this someday, but our decorators have so many layers of 
wrapping and indirection that it makes my head explode trying to figure out 
what's going on :-/

http://reviews.llvm.org/D14395

Files:
  third_party/Python/module/unittest2/unittest2/__init__.py
  third_party/Python/module/unittest2/unittest2/loader.py
  third_party/Python/module/unittest2/unittest2/test/test_loader.py

Index: third_party/Python/module/unittest2/unittest2/test/test_loader.py
===================================================================
--- third_party/Python/module/unittest2/unittest2/test/test_loader.py
+++ third_party/Python/module/unittest2/unittest2/test/test_loader.py
@@ -1104,15 +1104,12 @@
     # "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)
@@ -1120,9 +1117,6 @@
     # "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
@@ -1130,7 +1124,7 @@
         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)
@@ -1138,9 +1132,6 @@
     # "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
@@ -1148,7 +1139,7 @@
         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)
@@ -1156,9 +1147,6 @@
     # "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
@@ -1166,7 +1154,7 @@
         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)
@@ -1176,15 +1164,12 @@
     #
     # 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)
@@ -1192,7 +1177,7 @@
     # "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."
     #
Index: third_party/Python/module/unittest2/unittest2/loader.py
===================================================================
--- third_party/Python/module/unittest2/unittest2/loader.py
+++ third_party/Python/module/unittest2/unittest2/loader.py
@@ -1,5 +1,6 @@
 """Loading unittests."""
 
+import functools
 import os
 import re
 import sys
@@ -18,17 +19,6 @@
 
 __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'
@@ -60,10 +50,12 @@
     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"""
@@ -157,7 +149,7 @@
                 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):
Index: third_party/Python/module/unittest2/unittest2/__init__.py
===================================================================
--- third_party/Python/module/unittest2/unittest2/__init__.py
+++ third_party/Python/module/unittest2/unittest2/__init__.py
@@ -34,6 +34,8 @@
 else:
     cmp_ = cmp
 
+reversed_cmp_ = lambda x, y: -cmp_(x,y)
+
 __all__ = ['TestResult', 'TestCase', 'TestSuite',
            'TextTestRunner', 'TestLoader', 'FunctionTestCase', 'main',
            'defaultTestLoader', 'SkipTest', 'skip', 'skipIf', 'skipUnless',
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to