r47116 - review comments

Author: hawkowl
Date: Thu Mar 31 02:58:02 2016
New Revision: 47116

Modified:
   branches/oldstyle-decorator-8244/admin/run-python3-tests
   branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py
   branches/oldstyle-decorator-8244/twisted/python/test/test_util.py
   branches/oldstyle-decorator-8244/twisted/python/util.py
   branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py

Log:
review comments

Modified: branches/oldstyle-decorator-8244/admin/run-python3-tests
==============================================================================
--- branches/oldstyle-decorator-8244/admin/run-python3-tests	(original)
+++ branches/oldstyle-decorator-8244/admin/run-python3-tests	Thu Mar 31 02:58:02 2016
@@ -17,7 +17,9 @@
 testModules = []
 extraArguments = []
 installed = False
-twistedPath = os.path.abspath(os.path.dirname(os.path.dirname(sys.argv[0])))
+twistedPath = os.path.join(
+    os.path.abspath(os.path.dirname(os.path.dirname(sys.argv[0]))),
+    "src")
 
 
 for argument in sys.argv[1:]:

Modified: branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/_oldstyle.py	Thu Mar 31 02:58:02 2016
@@ -26,6 +26,24 @@
 
 
 
+def _shouldEnableNewStyle(environ=os.environ):
+    """
+    Returns whether or not we should enable the new-style conversion of
+    old-style classes. It inspects the environment for C{TWISTED_NEWSTYLE},
+    accepting an empty string, C{no}, C{false}, C{False}, and C{0} as falsey
+    values and everything else as a truthy value.
+
+    @rtype: L{bool}
+    """
+    value = environ.get('TWISTED_NEWSTYLE', '')
+
+    if value in ['', 'no', 'false', 'False', '0']:
+        return False
+    else:
+        return True
+
+
+
 def _ensureOldClass(cls):
     """
     Ensure that C{cls} is an old-style class.
@@ -49,15 +67,17 @@
 
 
 @_replaceIf(_PY3, passthru)
-@_replaceIf(int(os.environ.get('TWISTED_NEWSTYLE', 0)) == 0, _ensureOldClass)
+@_replaceIf(not _shouldEnableNewStyle(), _ensureOldClass)
 def _oldStyle(cls):
     """
     A decorator which conditionally converts old-style classes to new-style
-    classes. If it is Python 3, or if "TWISTED_NEWSTYLE" is not set or has a 0
-    value in the environment, this decorator is a no-op.
+    classes. If it is Python 3, or if the C{TWISTED_NEWSTYLE} environment
+    variable has a falsey (C{no}, C{false}, C{False}, or C{0}) value in the
+    environment, this decorator is a no-op.
 
     @param cls: An old-style class to convert to new-style.
     @type cls: L{types.ClassType}
+
     @return: A new-style version of C{cls}.
     """
     _ensureOldClass(cls)

Modified: branches/oldstyle-decorator-8244/twisted/python/test/test_util.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/test/test_util.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/test/test_util.py	Thu Mar 31 02:58:02 2016
@@ -1178,4 +1178,5 @@
                 """
 
         self.assertEqual(e.exception.args[0],
-                         "condition argument to _replaceIf requires a bool.")
+                         ("condition argument to _replaceIf requires a bool, "
+                          "not 'hi'"))

Modified: branches/oldstyle-decorator-8244/twisted/python/util.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/python/util.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/python/util.py	Thu Mar 31 02:58:02 2016
@@ -912,8 +912,10 @@
     If C{condition}, replace this function with C{alternative}.
 
     @param condition: A L{bool} which says whether this should be replaced.
+
     @param alternative: An alternative function that will be swapped in instead
-    of the original, if C{condition} is truthy.
+        of the original, if C{condition} is truthy.
+
     @return: A decorator.
     """
     def decorator(func):
@@ -923,8 +925,8 @@
         elif condition is False:
             call = func
         else:
-            raise ValueError(
-                "condition argument to _replaceIf requires a bool.")
+            raise ValueError(("condition argument to _replaceIf requires a "
+                              "bool, not {}").format(repr(condition)))
 
         @wraps(func)
         def wrapped(*args, **kwargs):

Modified: branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py
==============================================================================
--- branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py	(original)
+++ branches/oldstyle-decorator-8244/twisted/test/test_nooldstyle.py	Thu Mar 31 02:58:02 2016
@@ -15,13 +15,13 @@
 from twisted.python.modules import getModule
 from twisted.python.compat import _PY3
 from twisted.trial import unittest
-from twisted.python._oldstyle import _oldStyle
+from twisted.python import _oldstyle
 
 _skip = None
 
 if _PY3:
     _skip = "Not relevant on Python 3."
-elif int(os.environ.get('TWISTED_NEWSTYLE', 0)) == 0:
+elif not _oldstyle._shouldEnableNewStyle():
     _skip = "Not running with TWISTED_NEWSTYLE=1"
 
 
@@ -60,7 +60,7 @@
         that has the same functions, attributes, etc.
         """
         self.assertEqual(type(SomeOldStyleClass), types.ClassType)
-        updatedClass = _oldStyle(SomeOldStyleClass)
+        updatedClass = _oldstyle._oldStyle(SomeOldStyleClass)
         self.assertEqual(type(updatedClass), type)
         self.assertEqual(updatedClass().func(), "hi")
         self.assertEqual(updatedClass().bar, "baz")
@@ -71,7 +71,7 @@
         The class returned by L{_oldStyle} has the same C{__name__},
         C{__module__}, and docstring (C{__doc__}) attributes as the original.
         """
-        updatedClass = _oldStyle(SomeOldStyleClass)
+        updatedClass = _oldstyle._oldStyle(SomeOldStyleClass)
 
         self.assertEqual(updatedClass.__name__, SomeOldStyleClass.__name__)
         self.assertEqual(updatedClass.__doc__, SomeOldStyleClass.__doc__)
@@ -85,7 +85,7 @@
         """
 
         with self.assertRaises(ValueError) as e:
-            _oldStyle(SomeNewStyleClass)
+            _oldstyle._oldStyle(SomeNewStyleClass)
 
         self.assertEqual(
             e.exception.args[0],
@@ -99,7 +99,9 @@
         On Python 3 or on Py2 when C{TWISTED_NEWSTYLE} is not set, L{_oldStyle}
         is a no-op.
         """
-        updatedClass = _oldStyle(SomeOldStyleClass)
+        print(_oldstyle._oldStyle)
+        updatedClass = _oldstyle._oldStyle(SomeOldStyleClass)
+        self.assertEqual(type(updatedClass), type(SomeOldStyleClass))
         self.assertIs(updatedClass, SomeOldStyleClass)
 
     if _PY3:
@@ -149,28 +151,34 @@
 
 
 
-for x in getModule("twisted").walkModules():
+def _buildTestClasses(_locals):
 
-    ignoredModules = [
-        "twisted.test.reflect_helper",
-        "twisted.internet.test.process_",
-        "twisted.test.process_"
-    ]
+    for x in getModule("twisted").walkModules():
 
-    is_ignored = [x.name.startswith(ignored) for ignored in ignoredModules]
+        ignoredModules = [
+            "twisted.test.reflect_helper",
+            "twisted.internet.test.process_",
+            "twisted.test.process_"
+        ]
 
-    if True in is_ignored:
-        continue
+        is_ignored = [x.name.startswith(ignored) for ignored in ignoredModules]
 
+        if True in is_ignored:
+            continue
+
+
+        class Test(NewStyleOnly, unittest.TestCase):
+            """
+            @see: L{NewStyleOnly}
+            """
+            module = x.name
+
+        acceptableName = x.name.replace(".", "_")
+        Test.__name__ = acceptableName
+        if hasattr(Test, "__qualname__"):
+            Test.__qualname__ = acceptableName
+        _locals.update({acceptableName: Test})
 
-    class Test(NewStyleOnly, unittest.TestCase):
-        """
-        @see: L{NewStyleOnly}
-        """
-        module = x.name
 
-    acceptableName = x.name.replace(".", "_")
-    Test.__name__ = acceptableName
-    locals().update({acceptableName: Test})
 
-    del Test
+_buildTestClasses(locals())