THRIFT-3691 Run flake8 Python style check on Travis-CI

Client: Build (Python)
Patch: Nobuaki Sukegawa

This closes apache#907

Author: nsuke

.travis.yml

@@ -20,6 +20,7 @@
 # build Apache Thrift on Travis CI - https://travis-ci.org/
 
 sudo: required
+dist: trusty
 
 services:
   - docker
@@ -148,12 +149,18 @@ matrix:
   include:
     # QA jobs for code analytics and metrics
     #
-    # static code analysis with cppcheck
+    # C/C++ static code analysis with cppcheck
     # add --error-exitcode=1 to --enable=all as soon as everything is fixed
-    - env: TEST_NAME="cppcheck"
+    #
+    # Python code style check with flake8
+    #
+    # search for TODO etc within source tree
+    # some statistics about the code base
+    # some info about the build machine
+    - env: TEST_NAME="cppcheck, flake8, TODO FIXME HACK, LoC and system info"
       install:
         - sudo apt-get update
-        - sudo apt-get install cppcheck
+        - sudo apt-get install -y cppcheck sloccount python-flake8
       script:
         # Compiler cppcheck (All)
         - cppcheck --force --quiet --inline-suppr --enable=all -j2 compiler/cpp/src
@@ -165,15 +172,16 @@ matrix:
         - cppcheck --force --quiet --inline-suppr --error-exitcode=1 -j2 compiler/cpp/src
         - cppcheck --force --quiet --inline-suppr --error-exitcode=1 -j2 lib/cpp/src lib/cpp/test test/cpp tutorial/cpp
         - cppcheck --force --quiet --inline-suppr --error-exitcode=1 -j2 lib/c_glib/src lib/c_glib/test test/c_glib/src tutorial/c_glib
-
-    # search for TODO etc within source tree
-    # some statistics about the code base
-    # some info about the build machine
-    - env:    TEST_NAME="TODO FIXME HACK, LoC and system info"
-      install:
-        - sudo apt-get update
-        - sudo apt-get install sloccount
-      script:
+        # Python code style
+        - flake8 --ignore=E501 lib/py
+        - flake8 tutorial/py
+        - flake8 --ignore=E501 test/py
+        - flake8 test/py.twisted
+        - flake8 test/py.tornado
+        - flake8 --ignore=E501 test/test.py
+        - flake8 --ignore=E501 test/crossrunner
+        - flake8 test/features
+        # TODO etc
         - grep -r TODO *
         - grep -r FIXME *
         - grep -r HACK *

compiler/cpp/src/main.cc

@@ -356,6 +356,7 @@ string include_file(string filename) {
   if (filename[0] == '/') {
     // Realpath!
     char rp[THRIFT_PATH_MAX];
+    // cppcheck-suppress uninitvar
     if (saferealpath(filename.c_str(), rp) == NULL) {
       pwarning(0, "Cannot open include file %s\n", filename.c_str());
       return std::string();
@@ -378,6 +379,7 @@ string include_file(string filename) {
 
       // Realpath!
       char rp[THRIFT_PATH_MAX];
+      // cppcheck-suppress uninitvar
       if (saferealpath(sfilename.c_str(), rp) == NULL) {
         continue;
       }
@@ -1163,6 +1165,7 @@ int main(int argc, char** argv) {
         }
         char old_thrift_file_rp[THRIFT_PATH_MAX];
 
+        // cppcheck-suppress uninitvar
         if (saferealpath(arg, old_thrift_file_rp) == NULL) {
           failure("Could not open input file with realpath: %s", arg);
         }
@@ -1231,6 +1234,7 @@ int main(int argc, char** argv) {
       fprintf(stderr, "Missing file name of new thrift file for audit\n");
       usage();
     }
+    // cppcheck-suppress uninitvar
     if (saferealpath(argv[i], new_thrift_file_rp) == NULL) {
       failure("Could not open input file with realpath: %s", argv[i]);
     }
@@ -1256,6 +1260,7 @@ int main(int argc, char** argv) {
       fprintf(stderr, "Missing file name\n");
       usage();
     }
+    // cppcheck-suppress uninitvar
     if (saferealpath(argv[i], rp) == NULL) {
       failure("Could not open input file with realpath: %s", argv[i]);
     }

lib/py/CMakeLists.txt

@@ -2,3 +2,5 @@
 optimize = 1
 [metadata]
 description-file = README.md
+[flake8]
+max-line-length = 100

lib/py/Makefile.am

@@ -31,7 +31,7 @@ def str_to_binary(str_val):
 
 else:
 
-    from io import BytesIO as BufferIO
+    from io import BytesIO as BufferIO  # noqa
 
     def binary_to_str(bin_val):
         return bin_val.decode('utf8')

lib/py/setup.cfg

@@ -20,7 +20,7 @@
 import sys
 import unittest
 
-import _import_local_thrift
+import _import_local_thrift  # noqa
 from thrift.protocol.TJSONProtocol import TJSONProtocol
 from thrift.transport import TTransport
 

lib/py/src/compat.py

@@ -17,7 +17,7 @@
 # under the License.
 #
 
-from .test import test_name
-from .collect import collect_cross_tests, collect_feature_tests
-from .run import TestDispatcher
-from .report import generate_known_failures, load_known_failures
+from .test import test_name  # noqa
+from .collect import collect_cross_tests, collect_feature_tests  # noqa
+from .run import TestDispatcher  # noqa
+from .report import generate_known_failures, load_known_failures  # noqa

lib/py/test/thrift_json.py

@@ -37,6 +37,10 @@
 RESULT_TIMEOUT = 128
 RESULT_ERROR = 64
 
+# globals
+ports = None
+stop = None
+
 
 class ExecutionContext(object):
     def __init__(self, cmd, cwd, env, report):

test/crossrunner/__init__.py

@@ -0,0 +1,2 @@
+[flake8]
+max-line-length = 100

test/crossrunner/run.py

@@ -4,7 +4,7 @@
 import sys
 
 from util import add_common_args, init_protocol
-from local_thrift import thrift
+from local_thrift import thrift  # noqa
 from thrift.Thrift import TMessageType, TType
 
 

test/crossrunner/setup.cfg

@@ -0,0 +1,2 @@
+[flake8]
+max-line-length = 100

test/features/container_limit.py

@@ -4,7 +4,7 @@
 import sys
 
 from util import add_common_args, init_protocol
-from local_thrift import thrift
+from local_thrift import thrift  # noqa
 from thrift.Thrift import TMessageType, TType
 
 

test/features/setup.cfg

@@ -5,7 +5,7 @@
 import sys
 
 from util import add_common_args
-from local_thrift import thrift
+from local_thrift import thrift  # noqa
 from thrift.Thrift import TMessageType, TType
 from thrift.transport.TSocket import TSocket
 from thrift.transport.TTransport import TBufferedTransport, TFramedTransport

test/features/string_limit.py

@@ -1,7 +1,7 @@
 import argparse
 import socket
 
-from local_thrift import thrift
+from local_thrift import thrift  # noqa
 from thrift.transport.TSocket import TSocket
 from thrift.transport.TTransport import TBufferedTransport, TFramedTransport
 from thrift.transport.THttpClient import THttpClient

test/features/theader_binary.py

@@ -0,0 +1,3 @@
+[flake8]
+ignore = E402
+max-line-length = 100

test/features/util.py

@@ -44,7 +44,7 @@
 from thrift.transport.TTransport import TTransportException
 
 from ThriftTest import ThriftTest
-from ThriftTest.ttypes import *
+from ThriftTest.ttypes import Xception, Xtruct
 
 
 class TestHandler(object):

test/py.tornado/setup.cfg

@@ -0,0 +1,3 @@
+[flake8]
+ignore = E402
+max-line-length = 100

test/py.tornado/test_suite.py

@@ -113,9 +113,8 @@ def setUp(self):
         self.processor = ThriftTest.Processor(self.handler)
         self.pfactory = TBinaryProtocol.TBinaryProtocolFactory()
 
-        self.server = reactor.listenTCP(0,
-                                        TTwisted.ThriftServerFactory(self.processor,
-                                                                     self.pfactory), interface="127.0.0.1")
+        self.server = reactor.listenTCP(
+            0, TTwisted.ThriftServerFactory(self.processor, self.pfactory), interface="127.0.0.1")
 
         self.portNo = self.server.getHost().port
 

test/py.twisted/setup.cfg

@@ -80,7 +80,7 @@ def setUp(self):
             vertwo2000=VersioningTestV2(newstruct=Bonk(message='World!', type=314)),
             a_set2500=set(['lazy', 'brown', 'cow']),
             vertwo3000=VersioningTestV2(newset=set([2, 3, 5, 7, 11])),
-            big_numbers=[2**8, 2**16, 2**31 - 1, -(2**31 - 1)]
+            big_numbers=[2 ** 8, 2 ** 16, 2 ** 31 - 1, -(2 ** 31 - 1)]
         )
 
         self.compact_struct = CompactProtoTestStruct(
@@ -160,10 +160,9 @@ def setUp(self):
             # note, the sets below are sets of chars, since the strings are iterated
             map_int_strset={10: set('abc'), 20: set('def'), 30: set('GHI')},
             map_int_strset_list=[
-            {10: set('abc'), 20: set('def'), 30: set('GHI')},
-            {100: set('lmn'), 200: set('opq'), 300: set('RST')},
-            {1000: set('uvw'), 2000: set('wxy'), 3000: set('XYZ')}
-        ]
+                {10: set('abc'), 20: set('def'), 30: set('GHI')},
+                {100: set('lmn'), 200: set('opq'), 300: set('RST')},
+                {1000: set('uvw'), 2000: set('wxy'), 3000: set('XYZ')}]
         )
 
         self.nested_lists_bonk = NestedListsBonk(

test/py.twisted/test_suite.py

@@ -19,7 +19,6 @@
 # under the License.
 #
 
-import glob
 import os
 import sys
 import time

test/py/SerializationTest.py

@@ -20,5 +20,5 @@
 #
 
 # Just import these generated files to make sure they are syntactically valid
-from DebugProtoTest import EmptyService
-from DebugProtoTest import Inherited
+from DebugProtoTest import EmptyService  # noqa
+from DebugProtoTest import Inherited  # noqa

test/py/TestClient.py

@@ -0,0 +1,2 @@
+[flake8]
+max-line-length = 100

test/py/TestSyntax.py

@@ -52,8 +52,8 @@ def main():
     client.ping()
     print('ping()')
 
-    sum = client.add(1, 1)
-    print(('1+1=%d' % (sum)))
+    sum_ = client.add(1, 1)
+    print('1+1=%d' % sum_)
 
     work = Work()
 
@@ -66,17 +66,17 @@ def main():
         print('Whoa? You know how to divide by zero?')
         print('FYI the answer is %d' % quotient)
     except InvalidOperation as e:
-        print(('InvalidOperation: %r' % e))
+        print('InvalidOperation: %r' % e)
 
     work.op = Operation.SUBTRACT
     work.num1 = 15
     work.num2 = 10
 
     diff = client.calculate(1, work)
-    print(('15-10=%d' % (diff)))
+    print('15-10=%d' % diff)
 
     log = client.getStruct(1)
-    print(('Check log: %s' % (log.value)))
+    print('Check log: %s' % log.value)
 
     # Close!
     transport.close()
@@ -85,4 +85,4 @@ def main():
     try:
         main()
     except Thrift.TException as tx:
-        print(('%s' % (tx.message)))
+        print('%s' % tx.message)

test/py/setup.cfg

@@ -92,8 +92,10 @@ def zip(self):
     server = TServer.TSimpleServer(processor, transport, tfactory, pfactory)
 
     # You could do one of these for a multithreaded server
-    # server = TServer.TThreadedServer(processor, transport, tfactory, pfactory)
-    # server = TServer.TThreadPoolServer(processor, transport, tfactory, pfactory)
+    # server = TServer.TThreadedServer(
+    #     processor, transport, tfactory, pfactory)
+    # server = TServer.TThreadPoolServer(
+    #     processor, transport, tfactory, pfactory)
 
     print('Starting the server...')
     server.serve()

tutorial/py/PythonClient.py

@@ -0,0 +1,2 @@
+[flake8]
+ignore = E402