From f7bdc54c697ff24bec1bd0781666ac05401bafb2 Mon Sep 17 00:00:00 2001
From: skullydazed <skullydazed@users.noreply.github.com>
Date: Wed, 20 Nov 2019 14:54:18 -0800
Subject: [PATCH] Add flake8 to our test suite and fix all errors (#7379)

* Add flake8 to our test suite and fix all errors

* Add some documentation
---
 bin/qmk                              |  4 +-
 docs/cli_development.md              | 32 ++++++++++
 lib/python/kle2xy.py                 |  2 +-
 lib/python/qmk/cli/compile.py        |  3 -
 lib/python/qmk/cli/config.py         | 94 +++++++++++++++++-----------
 lib/python/qmk/cli/doctor.py         |  2 -
 lib/python/qmk/cli/flash.py          | 10 +--
 lib/python/qmk/cli/json/keymap.py    |  1 -
 lib/python/qmk/cli/kle2json.py       |  4 +-
 lib/python/qmk/cli/list/keyboards.py | 26 ++++----
 lib/python/qmk/cli/pytest.py         | 16 ++---
 lib/python/qmk/keymap.py             |  4 --
 lib/python/qmk/path.py               |  1 -
 requirements.txt                     |  2 +
 setup.cfg                            |  9 +++
 15 files changed, 125 insertions(+), 85 deletions(-)

diff --git a/bin/qmk b/bin/qmk
index 5da8673ba0fe9b53f271ba60e264e54fe06de969..4d5b3d884f66aefff83c0d0cebb59a9a5a1be3b7 100755
--- a/bin/qmk
+++ b/bin/qmk
@@ -41,7 +41,7 @@ else:
     os.environ['QMK_VERSION'] = 'nogit-' + strftime('%Y-%m-%d-%H:%M:%S') + '-dirty'
 
 # Setup the CLI
-import milc
+import milc  # noqa
 
 milc.EMOJI_LOGLEVELS['INFO'] = '{fg_blue}Ψ{style_reset_all}'
 
@@ -61,7 +61,7 @@ def main():
     os.chdir(qmk_dir)
 
     # Import the subcommands
-    import qmk.cli
+    import qmk.cli  # noqa
 
     # Execute
     return_code = milc.cli()
diff --git a/docs/cli_development.md b/docs/cli_development.md
index f5c7ad139a42e61e1e708419b21db51640b521f0..cc8c59d067eebde0f5a4d58f1f9927f43a4a8c70 100644
--- a/docs/cli_development.md
+++ b/docs/cli_development.md
@@ -173,3 +173,35 @@ You will only be able to access these arguments using `cli.args`. For example:
 ```
 cli.log.info('Reading from %s and writing to %s', cli.args.filename, cli.args.output)
 ```
+
+# Testing, and Linting, and Formatting (oh my!)
+
+We use nose2, flake8, and yapf to test, lint, and format code. You can use the `pytest` and `pyformat` subcommands to run these tests:
+
+### Testing and Linting
+
+    qmk pytest
+
+### Formatting
+
+    qmk pyformat
+
+## Formatting Details
+
+We use [yapf](https://github.com/google/yapf) to automatically format code. Our configuration is in the `[yapf]` section of `setup.cfg`.
+
+?> Tip- Many editors can use yapf as a plugin to automatically format code as you type.
+
+## Testing Details
+
+Our tests can be found in `lib/python/qmk/tests/`. You will find both unit and integration tests in this directory. We hope you will write both unit and integration tests for your code, but if you do not please favor integration tests. 
+
+If your PR does not include a comprehensive set of tests please add comments like this to your code so that other people know where they can help:
+
+    # TODO(unassigned/<yourGithubUsername>): Write <unit|integration> tests
+
+We use [nose2](https://nose2.readthedocs.io/en/latest/getting_started.html) to run our tests. You can refer to the nose2 documentation for more details on what you can do in your test functions.
+
+## Linting Details
+
+We use flake8 to lint our code. Your code should pass flake8 before you open a PR. This will be checked when you run `qmk pytest` and by CI when you submit a PR.
diff --git a/lib/python/kle2xy.py b/lib/python/kle2xy.py
index 92914431904f0eebbab7d788a9f840047bd4c729..bff1d025b76878cc9a9bbac513baf48ff2b5e66f 100644
--- a/lib/python/kle2xy.py
+++ b/lib/python/kle2xy.py
@@ -46,7 +46,7 @@ class KLE2xy(list):
         if 'name' in properties:
             self.name = properties['name']
 
-    def parse_layout(self, layout):
+    def parse_layout(self, layout):  # noqa  FIXME(skullydazed): flake8 says this has a complexity of 25, it should be refactored.
         # Wrap this in a dictionary so hjson will parse KLE raw data
         layout = '{"layout": [' + layout + ']}'
         layout = hjson.loads(layout)['layout']
diff --git a/lib/python/qmk/cli/compile.py b/lib/python/qmk/cli/compile.py
index 5c29800096e61feed102b3db4db7755b9e94184e..234ffb12ca9a2bcd04a6da0eccd2a296b9124908 100755
--- a/lib/python/qmk/cli/compile.py
+++ b/lib/python/qmk/cli/compile.py
@@ -2,9 +2,6 @@
 
 You can compile a keymap already in the repo or using a QMK Configurator export.
 """
-import json
-import os
-import sys
 import subprocess
 from argparse import FileType
 
diff --git a/lib/python/qmk/cli/config.py b/lib/python/qmk/cli/config.py
index c4ee20cba5bd26e3cb2e4adeb50314a08ac0cb28..e17d8bb9ba0e05afd1747d650e97d4d2806b707a 100644
--- a/lib/python/qmk/cli/config.py
+++ b/lib/python/qmk/cli/config.py
@@ -1,8 +1,5 @@
 """Read and write configuration settings
 """
-import os
-import subprocess
-
 from milc import cli
 
 
@@ -12,6 +9,54 @@ def print_config(section, key):
     cli.echo('%s.%s{fg_cyan}={fg_reset}%s', section, key, cli.config[section][key])
 
 
+def show_config():
+    """Print the current configuration to stdout.
+    """
+    for section in cli.config:
+        for key in cli.config[section]:
+            print_config(section, key)
+
+
+def parse_config_token(config_token):
+    """Split a user-supplied configuration-token into its components.
+    """
+    section = option = value = None
+
+    if '=' in config_token and '.' not in config_token:
+        cli.log.error('Invalid configuration token, the key must be of the form <section>.<option>: %s', config_token)
+        return section, option, value
+
+    # Separate the key (<section>.<option>) from the value
+    if '=' in config_token:
+        key, value = config_token.split('=')
+    else:
+        key = config_token
+
+    # Extract the section and option from the key
+    if '.' in key:
+        section, option = key.split('.', 1)
+    else:
+        section = key
+
+    return section, option, value
+
+
+def set_config(section, option, value):
+    """Set a config key in the running config.
+    """
+    log_string = '%s.%s{fg_cyan}:{fg_reset} %s {fg_cyan}->{fg_reset} %s'
+    if cli.args.read_only:
+        log_string += ' {fg_red}(change not written)'
+
+    cli.echo(log_string, section, option, cli.config[section][option], value)
+
+    if not cli.args.read_only:
+        if value == 'None':
+            del cli.config[section][option]
+        else:
+            cli.config[section][option] = value
+
+
 @cli.argument('-ro', '--read-only', arg_only=True, action='store_true', help='Operate in read-only mode.')
 @cli.argument('configs', nargs='*', arg_only=True, help='Configuration options to read or write.')
 @cli.subcommand("Read and write configuration settings.")
@@ -33,12 +78,7 @@ def config(cli):
     No validation is done to ensure that the supplied section.key is actually used by qmk scripts.
     """
     if not cli.args.configs:
-        # Walk the config tree
-        for section in cli.config:
-            for key in cli.config[section]:
-                print_config(section, key)
-
-        return True
+        return show_config()
 
     # Process config_tokens
     save_config = False
@@ -46,43 +86,23 @@ def config(cli):
     for argument in cli.args.configs:
         # Split on space in case they quoted multiple config tokens
         for config_token in argument.split(' '):
-            # Extract the section, config_key, and value to write from the supplied config_token.
-            if '=' in config_token:
-                key, value = config_token.split('=')
-            else:
-                key = config_token
-                value = None
-
-            if '.' in key:
-                section, config_key = key.split('.', 1)
-            else:
-                section = key
-                config_key = None
+            section, option, value = parse_config_token(config_token)
 
             # Validation
-            if config_key and '.' in config_key:
-                cli.log.error('Config keys may not have more than one period! "%s" is not valid.', key)
+            if option and '.' in option:
+                cli.log.error('Config keys may not have more than one period! "%s" is not valid.', config_token)
                 return False
 
             # Do what the user wants
-            if section and config_key and value:
-                # Write a config key
-                log_string = '%s.%s{fg_cyan}:{fg_reset} %s {fg_cyan}->{fg_reset} %s'
-                if cli.args.read_only:
-                    log_string += ' {fg_red}(change not written)'
-
-                cli.echo(log_string, section, config_key, cli.config[section][config_key], value)
-
+            if section and option and value:
+                # Write a configuration option
+                set_config(section, option, value)
                 if not cli.args.read_only:
-                    if value == 'None':
-                        del cli.config[section][config_key]
-                    else:
-                        cli.config[section][config_key] = value
                     save_config = True
 
-            elif section and config_key:
+            elif section and option:
                 # Display a single key
-                print_config(section, config_key)
+                print_config(section, option)
 
             elif section:
                 # Display an entire section
diff --git a/lib/python/qmk/cli/doctor.py b/lib/python/qmk/cli/doctor.py
index c2723bfcbbdc0697dab4feb8aa821ec2971efba7..1010eafb33e7c10052c791f2b423f2298d2b50b9 100755
--- a/lib/python/qmk/cli/doctor.py
+++ b/lib/python/qmk/cli/doctor.py
@@ -2,11 +2,9 @@
 
 Check up for QMK environment.
 """
-import os
 import platform
 import shutil
 import subprocess
-from glob import glob
 
 from milc import cli
 
diff --git a/lib/python/qmk/cli/flash.py b/lib/python/qmk/cli/flash.py
index 37556aaafd38dcceeb5bde94545bb615561e72a9..031cb9496755e48a7b45868e91b29e8621c67fbc 100644
--- a/lib/python/qmk/cli/flash.py
+++ b/lib/python/qmk/cli/flash.py
@@ -3,17 +3,11 @@
 You can compile a keymap already in the repo or using a QMK Configurator export.
 A bootloader must be specified.
 """
-import os
-import sys
 import subprocess
-from argparse import FileType
-
-from milc import cli
-from qmk.commands import create_make_command
-from qmk.commands import parse_configurator_json
-from qmk.commands import compile_configurator_json
 
 import qmk.path
+from milc import cli
+from qmk.commands import compile_configurator_json, create_make_command, parse_configurator_json
 
 
 def print_bootloader_help():
diff --git a/lib/python/qmk/cli/json/keymap.py b/lib/python/qmk/cli/json/keymap.py
index 7b7553104f02b3d1b00382ea7c1afb488d3c7cce..a030ab53d6ab3aeed52cfd9dd9278529304d3c8c 100755
--- a/lib/python/qmk/cli/json/keymap.py
+++ b/lib/python/qmk/cli/json/keymap.py
@@ -2,7 +2,6 @@
 """
 import json
 import os
-import sys
 
 from milc import cli
 
diff --git a/lib/python/qmk/cli/kle2json.py b/lib/python/qmk/cli/kle2json.py
index 5a4e97e3abea0ab591ebfbd66aa8e9985d6b7df2..00f63d3622312650db12f1b3d733497dbaf290fe 100755
--- a/lib/python/qmk/cli/kle2json.py
+++ b/lib/python/qmk/cli/kle2json.py
@@ -1,10 +1,8 @@
 """Convert raw KLE to JSON
-
 """
 import json
 import os
 from pathlib import Path
-from argparse import FileType
 from decimal import Decimal
 from collections import OrderedDict
 
@@ -23,7 +21,7 @@ class CustomJSONEncoder(json.JSONEncoder):
                 return float(obj)
         except TypeError:
             pass
-        return JSONEncoder.default(self, obj)
+        return json.JSONEncoder.default(self, obj)
 
 
 @cli.argument('filename', help='The KLE raw txt to convert')
diff --git a/lib/python/qmk/cli/list/keyboards.py b/lib/python/qmk/cli/list/keyboards.py
index 2a29ccb146a798053d3c43fc1bf349a870eba62f..76e7760e8405125f4cd1ffb8b179ddf04f8f5652 100644
--- a/lib/python/qmk/cli/list/keyboards.py
+++ b/lib/python/qmk/cli/list/keyboards.py
@@ -1,27 +1,27 @@
 """List the keyboards currently defined within QMK
 """
 import os
-import re
 import glob
 
 from milc import cli
 
+BASE_PATH = os.path.join(os.getcwd(), "keyboards") + os.path.sep
+KB_WILDCARD = os.path.join(BASE_PATH, "**", "rules.mk")
+
+
+def find_name(path):
+    """Determine the keyboard name by stripping off the base_path and rules.mk.
+    """
+    return path.replace(BASE_PATH, "").replace(os.path.sep + "rules.mk", "")
+
 
 @cli.subcommand("List the keyboards currently defined within QMK")
 def list_keyboards(cli):
     """List the keyboards currently defined within QMK
     """
-
-    base_path = os.path.join(os.getcwd(), "keyboards") + os.path.sep
-    kb_path_wildcard = os.path.join(base_path, "**", "rules.mk")
-
     # find everywhere we have rules.mk where keymaps isn't in the path
-    paths = [path for path in glob.iglob(kb_path_wildcard, recursive=True) if 'keymaps' not in path]
-
-    # strip the keyboard directory path prefix and rules.mk suffix and alphabetize
-    find_name = lambda path: path.replace(base_path, "").replace(os.path.sep + "rules.mk", "")
-    names = sorted(map(find_name, paths))
+    paths = [path for path in glob.iglob(KB_WILDCARD, recursive=True) if 'keymaps' not in path]
 
-    for name in names:
-        # We echo instead of cli.log.info to allow easier piping of this output
-        cli.echo(name)
+    # Extract the keyboard name from the path and print it
+    for keyboard_name in sorted(map(find_name, paths)):
+        print(keyboard_name)
diff --git a/lib/python/qmk/cli/pytest.py b/lib/python/qmk/cli/pytest.py
index 14613e1d962eee48ae1d8cb3e9687f107fbc5ceb..09611d750fc215f878fa8fb70d9c4ddedadad09f 100644
--- a/lib/python/qmk/cli/pytest.py
+++ b/lib/python/qmk/cli/pytest.py
@@ -2,19 +2,15 @@
 
 QMK script to run unit and integration tests against our python code.
 """
-import sys
+import subprocess
+
 from milc import cli
 
 
 @cli.subcommand('QMK Python Unit Tests')
 def pytest(cli):
-    """Use nose2 to run unittests
+    """Run several linting/testing commands.
     """
-    try:
-        import nose2
-
-    except ImportError:
-        cli.log.error('Could not import nose2! Please install it with {fg_cyan}pip3 install nose2')
-        return False
-
-    nose2.discover(argv=['nose2', '-v'])
+    flake8 = subprocess.run(['flake8', 'lib/python', 'bin/qmk'])
+    nose2 = subprocess.run(['nose2', '-v'])
+    return flake8.returncode | nose2.returncode
diff --git a/lib/python/qmk/keymap.py b/lib/python/qmk/keymap.py
index 396b53a6f5cde91913fc15db0c49d9eb5130da32..f4a2893f385556b94e0663b1aaa16fb98f1c2f23 100644
--- a/lib/python/qmk/keymap.py
+++ b/lib/python/qmk/keymap.py
@@ -1,12 +1,8 @@
 """Functions that help you work with QMK keymaps.
 """
-import json
-import logging
 import os
-from traceback import format_exc
 
 import qmk.path
-from qmk.errors import NoSuchKeyboardError
 
 # The `keymap.c` template to use when a keyboard doesn't have its own
 DEFAULT_KEYMAP_C = """#include QMK_KEYBOARD_H
diff --git a/lib/python/qmk/path.py b/lib/python/qmk/path.py
index 2149625cc6f83981f6d36df1b5849ceb2fa85867..cf087265fbf87426eedce35eacf3a521d2e5dde5 100644
--- a/lib/python/qmk/path.py
+++ b/lib/python/qmk/path.py
@@ -2,7 +2,6 @@
 """
 import logging
 import os
-from pkgutil import walk_packages
 
 from qmk.errors import NoSuchKeyboardError
 
diff --git a/requirements.txt b/requirements.txt
index aa6ee1ba329d6a5aff4a3c4954f6ea77d78a29ba..033b688fc6aa96bba447514309f4ba7ec532f197 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,3 +4,5 @@ appdirs
 argcomplete
 colorama
 hjson
+nose2
+flake8
diff --git a/setup.cfg b/setup.cfg
index 528512ac6f47527b325c03e3cad261c214e2a3ee..9e178ad6596bb327f25dba0dbe22da458d6d1dac 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,4 +1,13 @@
 # Python settings for QMK
+[flake8]
+ignore =
+    # QMK is ok with long lines.
+    E501
+per_file_ignores =
+    **/__init__.py:F401
+
+# Let's slowly crank this down
+max_complexity=16
 
 [yapf]
 # Align closing bracket with visual indentation.