[9954] Fade to black #1381

twm · 2020-09-13T05:21:38Z

Following up on #1380.

@rodrigc I consolidated steps 1 and 2 from your comment to avoid creating separate tickets and newsfiles for them.

Contributor Checklist:

The associated ticket in Trac is here: https://twistedmatrix.com/trac/ticket/9954
The changes pass minimal style checks (see: https://twistedmatrix.com/trac/wiki/TwistedDevelopment#GettingYourPatchAccepted )
I have created a newsfragment in src/twisted/newsfragments/ (see: https://twistedmatrix.com/trac/wiki/ReviewProcess#Newsfiles )
I have updated the automated tests.
I have submitted the associated Trac ticket for review by adding the word review to the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.

Is this commit spoiling git blame for you? Check out .git-blame-ignore-revs in the root of the repository.

…backHandlesUTF8DecodeFailure Resolve this failure: Traceback (most recent call last): File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/logger/test/test_format.py", line 531, in test_formatTracebackHandlesUTF8DecodeFailure self.assertIn(r"CapturedError(b'\xff\xfet\x00e\x00s\x00t\x00')", eventText) File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_synctest.py", line 489, in assertIn raise self.failureException(msg or "%r not in %r" % (containee, container)) twisted.trial.unittest.FailTest: "CapturedError(b'\\xff\\xfet\\x00e\\x00s\\x00t\\x00')" not in '- This is a test log message\nTraceback (most recent call last):\n File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_asynctest.py", line 148, in deferTestMethod\n d = self._run(self._testMethodName, result)\n File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_asynctest.py", line 119, in _run\n utils.runWithWarningsSuppressed, self._getSuppress(), method\n File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/internet/defer.py", line 162, in maybeDeferred\n result = f(*args, **kw)\n File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/internet/utils.py", line 205, in runWithWarningsSuppressed\n result = f(*a, **kw)\n--- <exception caught here> ---\n File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/logger/test/test_format.py", line 523, in test_formatTracebackHandlesUTF8DecodeFailure\n raise CapturedError(b"\\xff\\xfet\\x00e\\x00s\\x00t\\x00")\ntwisted.logger.test.test_format.CapturedError: b\'\\xff\\xfet\\x00e\\x00s\\x00t\\x00\'\n' The solution is to update the expectation to use the double quotes Black has changed the code to use.

…cks failure Resolve this test failure: Traceback (most recent call last): File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/test/test_defer.py", line 1424, in test_inlineCallbacksTracebacks self.assertEqual("1/0", tb[2][3]) File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_synctest.py", line 424, in assertEqual super(_Assertions, self).assertEqual(first, second, msg) File "/usr/lib/python3.6/unittest/case.py", line 829, in assertEqual assertion_func(first, second, msg=msg) File "/usr/lib/python3.6/unittest/case.py", line 1203, in assertMultiLineEqual self.fail(self._formatMessage(msg, standardMsg)) twisted.trial.unittest.FailTest: '1/0' != '1 / 0' - 1/0 + 1 / 0 ? + + Black added whitespace around the division operator. Update the assertion to match.

Resolve these two test failures: [FAIL] Traceback (most recent call last): File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/test/test_failure.py", line 859, in test_throwExceptionIntoGenerator self.assertEqual(traceback.extract_tb(stuff[0][2])[-1][-1], "1/0") File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_synctest.py", line 424, in assertEqual super(_Assertions, self).assertEqual(first, second, msg) File "/usr/lib/python3.6/unittest/case.py", line 829, in assertEqual assertion_func(first, second, msg=msg) File "/usr/lib/python3.6/unittest/case.py", line 1203, in assertMultiLineEqual self.fail(self._formatMessage(msg, standardMsg)) twisted.trial.unittest.FailTest: '1 / 0' != '1/0' - 1 / 0 ? - - + 1/0 twisted.test.test_failure.ExtendedGeneratorTests.test_throwExceptionIntoGenerator =============================================================================== [FAIL] Traceback (most recent call last): File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/test/test_failure.py", line 404, in test_RaiseExceptionWithTB self.assertEqual(innerline, "1/0") File "/home/twm/dev/twisted/build/nocov/lib/python3.6/site-packages/twisted/trial/_synctest.py", line 424, in assertEqual super(_Assertions, self).assertEqual(first, second, msg) File "/usr/lib/python3.6/unittest/case.py", line 829, in assertEqual assertion_func(first, second, msg=msg) File "/usr/lib/python3.6/unittest/case.py", line 1203, in assertMultiLineEqual self.fail(self._formatMessage(msg, standardMsg)) twisted.trial.unittest.FailTest: '1 / 0' != '1/0' - 1 / 0 ? - - + 1/0 twisted.test.test_failure.FailureTests.test_RaiseExceptionWithTB These failures were caused by the addition of whitespace around the division operator by Black. I updated the test expectations.

rodrigc · 2020-09-13T06:43:51Z

A whole bunch of CI is failing with this change. For example:

mypy failure due to reformatting /home/runner/work/twisted/twisted/src/twisted/words/test/test_irc.py: https://github.com/twisted/twisted/pull/1381/checks?check_run_id=1107647491#step:5:21
twisted.web.test.test_util.FailureElementTests.test_sourceFragmentElement failing: https://dev.azure.com/twistedmatrix/twisted/_build/results?buildId=2445&view=logs&j=786d9c93-0d4c-552d-b083-0885e74fd630&t=99d69555-4eea-5a30-cc7d-84ec61f55af8&l=15910

Can those be fixed in this commit somehow?

rodrigc · 2020-09-13T09:15:27Z

Can you add this patch to your branch:

diff --git a/src/twisted/web/test/test_util.py b/src/twisted/web/test/test_util.py
index e15413656..593ee583f 100644
--- a/src/twisted/web/test/test_util.py
+++ b/src/twisted/web/test/test_util.py
@@ -180,23 +180,27 @@ class FailureElementTests(TestCase):
         source = [
             " \N{NO-BREAK SPACE} \N{NO-BREAK SPACE}message = " '"This is a problem"',
             " \N{NO-BREAK SPACE} \N{NO-BREAK SPACE}raise Exception(message)",
-            "# Figure out the line number from which the exception will be " "raised.",
+            "",
         ]
         d = flattenString(None, element)
-        stringToCheckFor = "".join(
-            [
-                '<div class="snippet%sLine"><span>%d</span><span>%s</span>'
-                "</div>"
-                % (
+
+        stringToCheckFor = ""
+        for (lineNumber, sourceLine) in enumerate(source):
+            template = '<div class="snippet{}Line"><span>{}</span><span>{}</span></div>'
+            if lineNumber <= 1:
+                stringToCheckFor += template.format(
                     ["", "Highlight"][lineNumber == 1],
                     self.base + lineNumber,
                     (" \N{NO-BREAK SPACE}" * 4 + sourceLine),
                 )
-                for (lineNumber, sourceLine) in enumerate(source)
-            ]
-        ).encode("utf8")
+            else:
+                stringToCheckFor += template.format(
+                    "", self.base + lineNumber, ("" + sourceLine)
+                )
+
+        bytesToCheckFor = stringToCheckFor.encode("utf8")
 
-        d.addCallback(self.assertEqual, stringToCheckFor)
+        d.addCallback(self.assertEqual, bytesToCheckFor)
         return d
 
     def test_frameElementFilename(self):

test_sourceFragmentElement actually parses twisted/web/test/test_util.py, so when that file gets reformatted, this particular test breaks.

rodrigc · 2020-09-13T09:41:41Z

Can you also add this patch to your branch? This should fix the mypy errors:

diff --git a/src/twisted/internet/endpoints.py b/src/twisted/internet/endpoints.py
index ad6bcf3f2..e0da0f200 100644
--- a/src/twisted/internet/endpoints.py
+++ b/src/twisted/internet/endpoints.py
@@ -317,8 +317,8 @@ class _IProcessTransportWithConsumerAndProducer(
 
 
 class _ProcessEndpointTransport(
-    proxyForInterface(
-        _IProcessTransportWithConsumerAndProducer,  # type: ignore[misc]  # noqa
+    proxyForInterface(  # type: ignore[misc]
+        _IProcessTransportWithConsumerAndProducer,
         "_process",
     )
 ):
diff --git a/src/twisted/internet/test/process_cli.py b/src/twisted/internet/test/process_cli.py
index 7e29cdc1e..481c7be1c 100644
--- a/src/twisted/internet/test/process_cli.py
+++ b/src/twisted/internet/test/process_cli.py
@@ -7,9 +7,9 @@ try:
     # the tests.
     import msvcrt
 
-    msvcrt.setmode(
+    msvcrt.setmode(  # type:ignore[attr-defined]
         sys.stdout.fileno(), os.O_BINARY
-    )  # type:ignore[attr-defined]  # noqa
+    )
 except ImportError:
     pass
 
diff --git a/src/twisted/mail/test/test_pop3.py b/src/twisted/mail/test/test_pop3.py
index 529a2cd02..ba9bb3d8e 100644
--- a/src/twisted/mail/test/test_pop3.py
+++ b/src/twisted/mail/test/test_pop3.py
@@ -369,10 +369,8 @@ Someone set up us the bomb!\015
         d = loopback.loopbackAsync(protocol, clientProtocol)
         return d.addCallback(check)
 
-    test_loopback.suppress = [
-        util.suppress(  # type: ignore[attr-defined]
-            message="twisted.mail.pop3.POP3Client is deprecated"
-        )
+    test_loopback.suppress = [  # type: ignore[attr-defined]
+        util.suppress(message="twisted.mail.pop3.POP3Client is deprecated")
     ]
 
     def test_incorrectDomain(self):
diff --git a/src/twisted/python/test/test_runtime.py b/src/twisted/python/test/test_runtime.py
index 4cebdff2a..2d93d4527 100644
--- a/src/twisted/python/test/test_runtime.py
+++ b/src/twisted/python/test/test_runtime.py
@@ -87,9 +87,9 @@ class PlatformTests(SynchronousTestCase):
         if platform.getType() != "win32":
             self.assertFalse(isWinNT)
 
-    test_isWinNT.suppress = [
+    test_isWinNT.suppress = [  # type: ignore[attr-defined]
         SUPRESS(
-            category=DeprecationWarning,  # type: ignore[attr-defined]  # noqa
+            category=DeprecationWarning,
             message=isWinNTDeprecationMessage,
         )
     ]
diff --git a/src/twisted/trial/_asyncrunner.py b/src/twisted/trial/_asyncrunner.py
index bce4869c2..51715ee24 100644
--- a/src/twisted/trial/_asyncrunner.py
+++ b/src/twisted/trial/_asyncrunner.py
@@ -38,8 +38,8 @@ class TestSuite(pyunit.TestSuite):
 
 @implementer(itrial.ITestCase)
 class TestDecorator(
-    components.proxyForInterface(
-        itrial.ITestCase, "_originalTest"  # type: ignore[misc]  # noqa
+    components.proxyForInterface(  # type: ignore[misc]
+        itrial.ITestCase, "_originalTest"
     )
 ):
     """
diff --git a/src/twisted/trial/reporter.py b/src/twisted/trial/reporter.py
index d6ad9d576..03a9c86e1 100644
--- a/src/twisted/trial/reporter.py
+++ b/src/twisted/trial/reporter.py
@@ -223,9 +223,7 @@ class TestResult(pyunit.TestResult):
 
 @implementer(itrial.IReporter)
 class TestResultDecorator(
-    proxyForInterface(
-        itrial.IReporter, "_originalReporter"  # type: ignore[misc]  # noqa
-    )
+    proxyForInterface(itrial.IReporter, "_originalReporter")  # type: ignore[misc]
 ):
     """
     Base class for TestResult decorators.
diff --git a/src/twisted/trial/test/erroneous.py b/src/twisted/trial/test/erroneous.py
index e259e6fb1..2d527bfb8 100644
--- a/src/twisted/trial/test/erroneous.py
+++ b/src/twisted/trial/test/erroneous.py
@@ -162,8 +162,8 @@ class DelayedCall(unittest.TestCase):
         reactor.iterate(0.01)
         self.fail("Deliberate failure to mask the hidden exception")
 
-    testHiddenException.suppress = [
-        util.suppress(  # type: ignore[attr-defined]  # noqa
+    testHiddenException.suppress = [  # type: ignore[attr-defined]
+        util.suppress(
             message=r"reactor\.iterate cannot be used.*", category=DeprecationWarning
         )
     ]
diff --git a/src/twisted/trial/test/test_deferred.py b/src/twisted/trial/test/test_deferred.py
index b4690ff58..5927f7c6f 100644
--- a/src/twisted/trial/test/test_deferred.py
+++ b/src/twisted/trial/test/test_deferred.py
@@ -120,10 +120,8 @@ class DeferredTests(TestTester):
         self.assertEqual(result.testsRun, 1)
         self.assertTrue(detests.DeferredTests.touched)
 
-    test_passGenerated.supress = [
-        util.suppress(  # type: ignore[attr-defined]
-            message="twisted.internet.defer.deferredGenerator is deprecated"
-        )
+    test_passGenerated.supress = [  # type: ignore[attr-defined]
+        util.suppress(message="twisted.internet.defer.deferredGenerator is deprecated")
     ]
 
     def test_passInlineCallbacks(self):
diff --git a/src/twisted/web/test/test_xmlrpc.py b/src/twisted/web/test/test_xmlrpc.py
index 2be3079eb..a452014a8 100644
--- a/src/twisted/web/test/test_xmlrpc.py
+++ b/src/twisted/web/test/test_xmlrpc.py
@@ -114,8 +114,8 @@ class Test(XMLRPC):
         """
         return a + b
 
-    xmlrpc_add.signature = [
-        ["int", "int", "int"],  # type: ignore[attr-defined]
+    xmlrpc_add.signature = [  # type: ignore[attr-defined]
+        ["int", "int", "int"],
         ["double", "double", "double"],
     ]
 
diff --git a/src/twisted/web/xmlrpc.py b/src/twisted/web/xmlrpc.py
index 689314c64..a5f210792 100644
--- a/src/twisted/web/xmlrpc.py
+++ b/src/twisted/web/xmlrpc.py
@@ -307,8 +307,8 @@ class XMLRPCIntrospection(XMLRPC):
         method = self._xmlrpc_parent.lookupProcedure(method)
         return getattr(method, "signature", None) or ""
 
-    xmlrpc_methodSignature.signature = [
-        ["array", "string"],  # type: ignore[attr-defined] # noqa
+    xmlrpc_methodSignature.signature = [  # type: ignore[attr-defined]
+        ["array", "string"],
         ["string", "string"],
     ]

twm · 2020-09-13T18:30:47Z

@rodrigc I am working through the failures.

I haven't looked at the mypy failure yet, but I'd guess it's because I ran Black with Python 3.6. Perhaps it moved the comments around because they aren't part of the AST until Python 3.8. I'll test that theory tomorrow; I'm headed for bed now.

rodrigc · 2020-09-13T18:51:26Z

Yes the mypy issues are because comments got shifted to a different line. Python 3.6 vs 3.8 is irrelevant. See this patch: #1381 (comment)

twm · 2020-09-13T18:52:08Z

Oh sorry, I didn't see your new comments! I'll apply those patches. Thanks for digging in!

@rodrigc

…tElement Patch from @rodrigc.

@rodrigc

Move back the type annotation comments Black moved. Patch from @rodrigc. Thanks!

rodrigc

All the CI now passes.
Before merging, you may want to consider squashing some of the commits in this PR to clean up the history

twm · 2020-09-13T21:39:54Z

Thank you very much @rodrigc! I will merge this and begin the follow-up communications work.

rodrigc · 2020-09-14T01:18:12Z

@twm Nice job!

twm added 7 commits September 12, 2020 22:10

Fade to black

bc96c77

Is this commit spoiling git blame for you? Check out .git-blame-ignore-revs in the root of the repository.

Ignore the previous in git blame

88255bf

Add newsfile

91a7b77

Enable black linting

3d60a85

twm added 2 commits September 13, 2020 11:56

Fix twisted.web.test.test_util.FailureElementTests.test_sourceFragmen…

9f6f8dd

…tElement Patch from @rodrigc.

Fix for MyPy

8e6e62d

Move back the type annotation comments Black moved. Patch from @rodrigc. Thanks!

rodrigc approved these changes Sep 13, 2020

View reviewed changes

Work around psf/black#1629

1d0734b

twm force-pushed the 9954-fade-to-black branch from e539cff to 1d0734b Compare September 13, 2020 21:13

twm merged commit 7e3ce79 into trunk Sep 13, 2020

twm deleted the 9954-fade-to-black branch September 13, 2020 21:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[9954] Fade to black #1381

[9954] Fade to black #1381

twm commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading

twm commented Sep 13, 2020

rodrigc commented Sep 13, 2020

twm commented Sep 13, 2020

rodrigc left a comment

twm commented Sep 13, 2020

rodrigc commented Sep 14, 2020

[9954] Fade to black #1381

[9954] Fade to black #1381

Conversation

twm commented Sep 13, 2020 • edited Loading

Contributor Checklist:

rodrigc commented Sep 13, 2020 • edited Loading

rodrigc commented Sep 13, 2020 • edited Loading

rodrigc commented Sep 13, 2020 • edited Loading

twm commented Sep 13, 2020

rodrigc commented Sep 13, 2020

twm commented Sep 13, 2020

rodrigc left a comment

Choose a reason for hiding this comment

twm commented Sep 13, 2020

rodrigc commented Sep 14, 2020

twm commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading

rodrigc commented Sep 13, 2020 •

edited

Loading