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 <ssbarnea@redhat.com>
ansible · Nov 3, 2019 · d658150 · d658150
1 parent 5453d07
commit d658150
Show file tree

Hide file tree

Showing 13 changed files with 76 additions and 36 deletions.
diff --git a/.pre-commit-config.yaml 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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