From 90c5ce2e5ffac4f3192ffb97f617018b91d18e16 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 27 Oct 2014 16:52:11 -0700 Subject: [PATCH 1/3] Remove cyclic import from ignored for tests. --- pylintrc_reduced | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylintrc_reduced b/pylintrc_reduced index 3342430ab6f2..f8d3fbad384b 100644 --- a/pylintrc_reduced +++ b/pylintrc_reduced @@ -24,7 +24,7 @@ ignore = datastore_v1_pb2.py [MESSAGES CONTROL] disable = I, protected-access, maybe-no-member, no-member, redefined-builtin, star-args, missing-format-attribute, - similarities, cyclic-import, arguments-differ, + similarities, arguments-differ, invalid-name, missing-docstring, too-many-public-methods, too-few-public-methods, attribute-defined-outside-init, unbalanced-tuple-unpacking, too-many-locals, exec-used, From 8925aed7246e25aa71af57dc25d079ddf05f9b92 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 27 Oct 2014 23:37:15 -0700 Subject: [PATCH 2/3] Reducing repeated pylint configs by defining test configs as a delta. Uses ConfigParser to append modifications onto base file and writes a new config file (which is gitignore-ed) before running pylint. --- .gitignore | 3 +++ pylintrc_default | 6 +----- pylintrc_reduced | 37 ------------------------------------- pylintrc_test_modifications | 8 ++++++++ run_pylint.py | 36 ++++++++++++++++++++++++++++++++++++ 5 files changed, 48 insertions(+), 42 deletions(-) delete mode 100644 pylintrc_reduced create mode 100644 pylintrc_test_modifications diff --git a/.gitignore b/.gitignore index 0f046b6dbcd2..786d217d8120 100644 --- a/.gitignore +++ b/.gitignore @@ -45,3 +45,6 @@ coverage.xml # Regression test environment variables. regression/local_test_setup + +# Make sure a generated file isn't accidentally committed. +pylintrc_reduced diff --git a/pylintrc_default b/pylintrc_default index 38d7b97eb2de..84293654d3aa 100644 --- a/pylintrc_default +++ b/pylintrc_default @@ -24,11 +24,7 @@ ignore = datastore_v1_pb2.py [MESSAGES CONTROL] disable = I, protected-access, maybe-no-member, no-member, redefined-builtin, star-args, missing-format-attribute, - similarities, arguments-differ, - - - - + similarities, arguments-differ [REPORTS] reports = no diff --git a/pylintrc_reduced b/pylintrc_reduced deleted file mode 100644 index f8d3fbad384b..000000000000 --- a/pylintrc_reduced +++ /dev/null @@ -1,37 +0,0 @@ -# This config is intended to be used for test code -# and other non-production code, like demos. - -[BASIC] -good-names = i, j, k, ex, Run, _, pb, id, - _get_protobuf_attribute_and_value - -[DESIGN] -max-args = 10 -max-public-methods = 30 - -[FORMAT] -# NOTE: By default pylint ignores whitespace checks around the -# constructs "dict-separator" (cases like {1:2}) and "trailing-comma" -# (cases like {1: 2, }). By setting "no-space-check" to empty -# whitespace checks will be enforced around both constructs. -no-space-check = - -[MASTER] -# NOTE: This path must be relative due to the use of -# os.walk in astroid.modutils.get_module_files. -ignore = datastore_v1_pb2.py - -[MESSAGES CONTROL] -disable = I, protected-access, maybe-no-member, no-member, - redefined-builtin, star-args, missing-format-attribute, - similarities, arguments-differ, - invalid-name, missing-docstring, too-many-public-methods, - too-few-public-methods, attribute-defined-outside-init, - unbalanced-tuple-unpacking, too-many-locals, exec-used, - no-init, no-self-use - -[REPORTS] -reports = no - -[VARIABLES] -dummy-variables-rgx = _$|dummy|^unused_ diff --git a/pylintrc_test_modifications b/pylintrc_test_modifications new file mode 100644 index 000000000000..3eacb65f7c8f --- /dev/null +++ b/pylintrc_test_modifications @@ -0,0 +1,8 @@ +# This config is intended to be used for test code +# and other non-production code, like demos. + +[MESSAGES CONTROL] +disable = invalid-name, missing-docstring, too-many-public-methods, + too-few-public-methods, attribute-defined-outside-init, + unbalanced-tuple-unpacking, too-many-locals, exec-used, + no-init, no-self-use diff --git a/run_pylint.py b/run_pylint.py index 21560f318112..65df7529eeb2 100644 --- a/run_pylint.py +++ b/run_pylint.py @@ -7,6 +7,8 @@ violations (hence it has a reduced number of style checks). """ +import ConfigParser +import copy import subprocess import sys @@ -18,6 +20,39 @@ ] PRODUCTION_RC = 'pylintrc_default' TEST_RC = 'pylintrc_reduced' +TEST_RC_MODS = 'pylintrc_test_modifications' + + +def read_config(filename): + """Reads pylintrc config onto native ConfigParser object.""" + config = ConfigParser.ConfigParser() + with open(filename, 'r') as file_obj: + config.readfp(file_obj) + return config + + +def make_test_rc(base_rc_filename, modifications_rc_filename, + target_filename): + """Combines a base rc and modifications into single file.""" + main_cfg = read_config(base_rc_filename) + modification_cfg = read_config(modifications_rc_filename) + + # Create fresh config for test, which must extend production. + test_cfg = ConfigParser.ConfigParser() + + test_cfg._sections = copy.deepcopy(main_cfg._sections) + for section, opts in modification_cfg._sections.items(): + curr_section = test_cfg._sections.setdefault( + section, modification_cfg._dict()) + for opt, opt_val in opts.items(): + if opt in curr_section: + if curr_section[opt] != opt_val: + curr_section[opt] = ', '.join([curr_section[opt], opt_val]) + else: + curr_section[opt] = opt_val + + with open(target_filename, 'w') as file_obj: + test_cfg.write(file_obj) def valid_filename(filename): @@ -74,6 +109,7 @@ def lint_fileset(filenames, rcfile, description): def main(): """Script entry point. Lints both sets of files.""" + make_test_rc(PRODUCTION_RC, TEST_RC_MODS, TEST_RC) library_files, non_library_files = get_python_files() lint_fileset(library_files, PRODUCTION_RC, 'library code') lint_fileset(non_library_files, TEST_RC, 'test and demo code') From d75091a70d2083a71352c8069e03bfb750ecc4d0 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 28 Oct 2014 13:13:00 -0700 Subject: [PATCH 3/3] Putting pylint additions for tests into run_pylint. --- pylintrc_test_modifications | 8 -------- run_pylint.py | 41 ++++++++++++++++++++++++------------- 2 files changed, 27 insertions(+), 22 deletions(-) delete mode 100644 pylintrc_test_modifications diff --git a/pylintrc_test_modifications b/pylintrc_test_modifications deleted file mode 100644 index 3eacb65f7c8f..000000000000 --- a/pylintrc_test_modifications +++ /dev/null @@ -1,8 +0,0 @@ -# This config is intended to be used for test code -# and other non-production code, like demos. - -[MESSAGES CONTROL] -disable = invalid-name, missing-docstring, too-many-public-methods, - too-few-public-methods, attribute-defined-outside-init, - unbalanced-tuple-unpacking, too-many-locals, exec-used, - no-init, no-self-use diff --git a/run_pylint.py b/run_pylint.py index 65df7529eeb2..14299046f86a 100644 --- a/run_pylint.py +++ b/run_pylint.py @@ -20,7 +20,23 @@ ] PRODUCTION_RC = 'pylintrc_default' TEST_RC = 'pylintrc_reduced' -TEST_RC_MODS = 'pylintrc_test_modifications' +TEST_DISABLED_MESSAGES = [ + 'invalid-name', + 'missing-docstring', + 'too-many-public-methods', + 'too-few-public-methods', + 'attribute-defined-outside-init', + 'unbalanced-tuple-unpacking', + 'too-many-locals', + 'exec-used', + 'no-init', + 'no-self-use', +] +TEST_RC_ADDITIONS = { + 'MESSAGES CONTROL': { + 'disable': ', '.join(TEST_DISABLED_MESSAGES), + }, +} def read_config(filename): @@ -31,25 +47,22 @@ def read_config(filename): return config -def make_test_rc(base_rc_filename, modifications_rc_filename, - target_filename): - """Combines a base rc and modifications into single file.""" +def make_test_rc(base_rc_filename, additions_dict, target_filename): + """Combines a base rc and test additions into single file.""" main_cfg = read_config(base_rc_filename) - modification_cfg = read_config(modifications_rc_filename) # Create fresh config for test, which must extend production. test_cfg = ConfigParser.ConfigParser() - test_cfg._sections = copy.deepcopy(main_cfg._sections) - for section, opts in modification_cfg._sections.items(): + + for section, opts in additions_dict.items(): curr_section = test_cfg._sections.setdefault( - section, modification_cfg._dict()) + section, test_cfg._dict()) for opt, opt_val in opts.items(): - if opt in curr_section: - if curr_section[opt] != opt_val: - curr_section[opt] = ', '.join([curr_section[opt], opt_val]) - else: - curr_section[opt] = opt_val + curr_val = curr_section.get(opt) + if curr_val is None: + raise KeyError('Expected to be adding to existing option.') + curr_section[opt] = '%s, %s' % (curr_val, opt_val) with open(target_filename, 'w') as file_obj: test_cfg.write(file_obj) @@ -109,7 +122,7 @@ def lint_fileset(filenames, rcfile, description): def main(): """Script entry point. Lints both sets of files.""" - make_test_rc(PRODUCTION_RC, TEST_RC_MODS, TEST_RC) + make_test_rc(PRODUCTION_RC, TEST_RC_ADDITIONS, TEST_RC) library_files, non_library_files = get_python_files() lint_fileset(library_files, PRODUCTION_RC, 'library code') lint_fileset(non_library_files, TEST_RC, 'test and demo code')