From d658150fcb5ac1abb8c45553e837b6fa1a0284b8 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Sun, 3 Nov 2019 12:57:55 +0000 Subject: [PATCH] Adopt use of pre-commit for calling flake8 This enables us to easily add other linters and to better control the linter versions, without riskign of getting different results based on their versions (flake8 version was not pinned before). Removes flake8 excludes, so we cover our entire codebase. Fixes few rules that were not covered yet by our codebase: E201 whitespace after '[' E202 whitespace before ']' E231 missing whitespace after ',' E251 unexpected spaces around keyword / parameter equals E302 expected 2 blank lines, found 1 E501 line too long (108 > 100 characters) F841 local variable 'result' is assigned to but never used Note: .github actions still uses flake8 and must be updated via github GUI because github refuses pushes touching ~/.github folder. Signed-off-by: Sorin Sbarnea --- .pre-commit-config.yaml | 7 +++ .travis.yml | 4 +- docs/docsite/rst/conf.py | 3 +- setup.cfg | 2 +- test/TestCommandLineInvocationSameAsConfig.py | 4 +- test/TestFormatter.py | 7 ++- test/TestLineTooLong.py | 2 +- test/TestRoleHandlers.py | 4 +- test/TestRulesCollection.py | 2 +- test/TestTaskIncludes.py | 9 ++-- test/TestUseHandlerRatherThanWhenChanged.py | 3 +- test/TestUtils.py | 48 ++++++++++++------- tox.ini | 17 +++++-- 13 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000000..e07216b95b --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,7 @@ +--- +repos: +- repo: https://gitlab.com/pycqa/flake8.git + rev: 3.7.9 + hooks: + - id: flake8 + language_version: python3 diff --git a/.travis.yml b/.travis.yml index 88d32131ff..ff6f2ba90c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,9 +26,9 @@ jobs: include: - python: "3.8" <<: *reset-prerequisites - name: Running flake8 linting checks + name: Running linters env: - TOXENV: flake8 + TOXENV: lint - python: "3.8" <<: *reset-prerequisites name: Running docs building checks diff --git a/docs/docsite/rst/conf.py b/docs/docsite/rst/conf.py index 9f2e5b4349..a7b95e6e77 100644 --- a/docs/docsite/rst/conf.py +++ b/docs/docsite/rst/conf.py @@ -105,7 +105,8 @@ highlight_language = 'YAML+Jinja' # Substitutions, variables, entities, & shortcuts for text which do not need to link to anything. -# For titles which should be a link, use the intersphinx anchors set at the index, chapter, and section levels, such as qi_start_: +# For titles which should be a link, use the intersphinx anchors set at the index, chapter, and +# section levels, such as qi_start_: rst_epilog = """ .. |acapi| replace:: *Ansible Core API Guide* .. |acrn| replace:: *Ansible Core Release Notes* diff --git a/setup.cfg b/setup.cfg index 5b5dbfdf72..ed85fccaff 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,7 +6,7 @@ universal = 1 [flake8] max-line-length = 100 -exclude = .git,.hg,.svn,test,setup.py,__pycache__,docs,.tox,.eggs,env +exclude = .git,.hg,.svn,__pycache__,.tox,.eggs,env [metadata] name = ansible-lint diff --git a/test/TestCommandLineInvocationSameAsConfig.py b/test/TestCommandLineInvocationSameAsConfig.py index 9327eb5b5b..d0857d9f38 100644 --- a/test/TestCommandLineInvocationSameAsConfig.py +++ b/test/TestCommandLineInvocationSameAsConfig.py @@ -66,7 +66,9 @@ def test_exclude(self): def test_config_can_be_overridden(self): no_override = self.run_ansible_lint(args="-t bad_tag") - overridden = self.run_ansible_lint(args="-t bad_tag", config=dict(tags=["skip_ansible_lint"])) + overridden = self.run_ansible_lint( + args="-t bad_tag", + config=dict(tags=["skip_ansible_lint"])) self.assertEqual(no_override, overridden) diff --git a/test/TestFormatter.py b/test/TestFormatter.py index b31cf8feaf..e9fee13a8d 100644 --- a/test/TestFormatter.py +++ b/test/TestFormatter.py @@ -18,7 +18,6 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. -import os import unittest from ansiblelint import Match, AnsibleLintRule @@ -34,12 +33,12 @@ def setUp(self): def test_format_coloured_string(self): match = Match(1, "hello", "filename.yml", self.rule, "message") - result = self.formatter.format(match, True) + self.formatter.format(match, True) def test_unicode_format_string(self): match = Match(1, "hello", "filename.yml", self.rule, u'\U0001f427') - result = self.formatter.format(match, False) + self.formatter.format(match, False) def test_dict_format_line(self): match = Match(1, {'hello': 'world'}, "filename.yml", self.rule, "xyz") - result = self.formatter.format(match, True) + self.formatter.format(match, True) diff --git a/test/TestLineTooLong.py b/test/TestLineTooLong.py index dd014c0ada..94190390c8 100644 --- a/test/TestLineTooLong.py +++ b/test/TestLineTooLong.py @@ -8,7 +8,7 @@ - name: task example debug: msg: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua tempor incididunt ut labore et dolore' -''' +''' # noqa 501 class TestLineTooLongRule(unittest.TestCase): diff --git a/test/TestRoleHandlers.py b/test/TestRoleHandlers.py index 6a41a3dc5e..a20108a41b 100644 --- a/test/TestRoleHandlers.py +++ b/test/TestRoleHandlers.py @@ -1,6 +1,8 @@ import unittest from ansiblelint import Runner, RulesCollection -from ansiblelint.rules.UseHandlerRatherThanWhenChangedRule import UseHandlerRatherThanWhenChangedRule +from ansiblelint.rules.UseHandlerRatherThanWhenChangedRule import ( + UseHandlerRatherThanWhenChangedRule) + class TestRoleHandlers(unittest.TestCase): collection = RulesCollection() diff --git a/test/TestRulesCollection.py b/test/TestRulesCollection.py index 0c717a6ac6..74f679df36 100644 --- a/test/TestRulesCollection.py +++ b/test/TestRulesCollection.py @@ -76,5 +76,5 @@ def test_skip_non_existent_id(self): def test_no_duplicate_rule_ids(self): real_rules = RulesCollection.create_from_directory('./lib/ansiblelint/rules') - rule_ids = [ rule.id for rule in real_rules ] + rule_ids = [rule.id for rule in real_rules] self.assertEqual([x for x, y in collections.Counter(rule_ids).items() if y > 1], []) diff --git a/test/TestTaskIncludes.py b/test/TestTaskIncludes.py index d87d8d79ed..f91c004317 100644 --- a/test/TestTaskIncludes.py +++ b/test/TestTaskIncludes.py @@ -30,14 +30,16 @@ def test_included_tasks(self): runner.run() self.assertEqual(len(runner.playbooks), 4) - @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), "not supported with ansible < 2.4") + @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), + "not supported with ansible < 2.4") def test_include_tasks_2_4_style(self): filename = 'test/taskincludes_2_4_style.yml' runner = Runner(self.rules, filename, [], [], []) runner.run() self.assertEqual(len(runner.playbooks), 4) - @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), "not supported with ansible < 2.4") + @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), + "not supported with ansible < 2.4") def test_import_tasks_2_4_style(self): filename = 'test/taskimports.yml' runner = Runner(self.rules, filename, [], [], []) @@ -50,7 +52,8 @@ def test_include_tasks_with_block_include(self): runner.run() self.assertEqual(len(runner.playbooks), 3) - @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), "not supported with ansible < 2.4") + @unittest.skipIf(parse_version(ansible.__version__) < parse_version('2.4'), + "not supported with ansible < 2.4") def test_include_tasks_in_role(self): filename = 'test/include-import-tasks-in-role.yml' runner = Runner(self.rules, filename, [], [], []) diff --git a/test/TestUseHandlerRatherThanWhenChanged.py b/test/TestUseHandlerRatherThanWhenChanged.py index c5e2988075..82c2adf34a 100644 --- a/test/TestUseHandlerRatherThanWhenChanged.py +++ b/test/TestUseHandlerRatherThanWhenChanged.py @@ -1,7 +1,8 @@ import unittest from ansiblelint import RulesCollection -from ansiblelint.rules.UseHandlerRatherThanWhenChangedRule import UseHandlerRatherThanWhenChangedRule +from ansiblelint.rules.UseHandlerRatherThanWhenChangedRule import ( + UseHandlerRatherThanWhenChangedRule) from test import RunFromText diff --git a/test/TestUtils.py b/test/TestUtils.py index 88d80fc8ef..b8a4ed1686 100644 --- a/test/TestUtils.py +++ b/test/TestUtils.py @@ -67,7 +67,9 @@ def test_tokenize_command_with_args(self): def test_normalize_simple_command(self): task1 = dict(name="hello", action="command chdir=abc echo hello world") task2 = dict(name="hello", command="chdir=abc echo hello world") - self.assertEqual(utils.normalize_task(task1, 'tasks.yml'), utils.normalize_task(task2, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task1, 'tasks.yml'), + utils.normalize_task(task2, 'tasks.yml')) def test_normalize_complex_command(self): task1 = dict(name="hello", action={'module': 'ec2', @@ -77,26 +79,38 @@ def test_normalize_complex_command(self): 'etc': 'whatever'}) task3 = dict(name="hello", ec2="region=us-east1 etc=whatever") task4 = dict(name="hello", action="ec2 region=us-east1 etc=whatever") - self.assertEqual(utils.normalize_task(task1, 'tasks.yml'), utils.normalize_task(task2, 'tasks.yml')) - self.assertEqual(utils.normalize_task(task2, 'tasks.yml'), utils.normalize_task(task3, 'tasks.yml')) - self.assertEqual(utils.normalize_task(task3, 'tasks.yml'), utils.normalize_task(task4, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task1, 'tasks.yml'), + utils.normalize_task(task2, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task2, 'tasks.yml'), + utils.normalize_task(task3, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task3, 'tasks.yml'), + utils.normalize_task(task4, 'tasks.yml')) def test_normalize_args(self): - task1 = dict(git={'version': 'abc'}, args={'repo': 'blah', 'dest': 'xyz'}) - task2 = dict(git={'version': 'abc', 'repo': 'blah', 'dest': 'xyz'}) - - task3 = dict(git='version=abc repo=blah dest=xyz') - task4 = dict(git=None, args={'repo': 'blah', 'dest': 'xyz', 'version': 'abc'}) - self.assertEqual(utils.normalize_task(task1, 'tasks.yml'), utils.normalize_task(task2, 'tasks.yml')) - self.assertEqual(utils.normalize_task(task1, 'tasks.yml'), utils.normalize_task(task3, 'tasks.yml')) - self.assertEqual(utils.normalize_task(task1, 'tasks.yml'), utils.normalize_task(task4, 'tasks.yml')) + task1 = {'git': {'version': 'abc'}, 'args': {'repo': 'blah', 'dest': 'xyz'}} + task2 = {'git': {'version': 'abc', 'repo': 'blah', 'dest': 'xyz'}} + + task3 = {"git": 'version=abc repo=blah dest=xyz'} + task4 = {"git": None, "args": {'repo': 'blah', 'dest': 'xyz', 'version': 'abc'}} + self.assertEqual( + utils.normalize_task(task1, 'tasks.yml'), + utils.normalize_task(task2, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task1, 'tasks.yml'), + utils.normalize_task(task3, 'tasks.yml')) + self.assertEqual( + utils.normalize_task(task1, 'tasks.yml'), + utils.normalize_task(task4, 'tasks.yml')) def test_extract_from_list(self): - block = dict( - block = [dict(tasks=[dict(name="hello",command="whoami")])], - test_none = None, - test_string = 'foo' - ) + block = { + 'block': [{'tasks': {'name': 'hello', 'command': 'whoami'}}], + 'test_none': None, + 'test_string': 'foo' + } blocks = [block] test_list = utils.extract_from_list(blocks, ['block']) diff --git a/tox.ini b/tox.ini index 15707f6f78..f794aa99ec 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] minversion = 3.5.3 -envlist = flake8,py{38,37,36,35,27}-ansible{29,28,27,devel} +envlist = lint,py{38,37,36,35,27}-ansible{29,28,27,devel} isolated_build = true requires = setuptools >= 41.4.0 @@ -53,9 +53,20 @@ commands = --out-dir {toxinidir}/dist/ \ {toxinidir} +# deprecated: use more generic 'lint' instead [testenv:flake8] -deps = flake8 -commands = flake8 +deps = {[testenv:lint]deps} +envdir = {toxworkdir}/lint +skip_install = true +commands = + python -m pre_commit run --all-files flake8 + +[testenv:lint] +deps = + pre-commit>=1.20.0 +skip_install = true +commands = + python -m pre_commit run {posargs:--all-files} [testenv:docs] whitelist_externals = make