From 466ef2e5966d2b36a11dbf205a7bf3835f427d70 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Sat, 28 May 2016 12:00:47 -0500 Subject: [PATCH] Add example configuration sections to the docs Add tests to verify our examples do not regress --- docs/source/user/configuration.rst | 125 +++++++++++++++++- .../cli-specified-with-inline-comments.ini | 16 +++ .../cli-specified-without-inline-comments.ini | 16 +++ tests/unit/test_merged_config_parser.py | 33 +++++ 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/config_files/cli-specified-with-inline-comments.ini create mode 100644 tests/fixtures/config_files/cli-specified-without-inline-comments.ini diff --git a/docs/source/user/configuration.rst b/docs/source/user/configuration.rst index 1e51e0a..ff671a6 100644 --- a/docs/source/user/configuration.rst +++ b/docs/source/user/configuration.rst @@ -75,4 +75,127 @@ sub-directories. Let's take for example Flake8's own project structure └── unit In the top-level ``flake8`` directory (which contains ``docs``, ``flake8``, -and ``tests``) there's also ``tox.ini`` and ``setup.cfg`` files. +and ``tests``) there's also ``tox.ini`` and ``setup.cfg`` files. In our case, +we keep our Flake8 configuration in ``tox.ini``. Regardless of whether you +keep your config in ``.flake8``, ``setup.cfg``, or ``tox.ini`` we expect you +to use INI to configure Flake8 (since each of these files already uses INI +as a format). This means that any Flake8 configuration you wish to set needs +to be in the ``flake8`` section, which means it needs to start like so: + +.. code-block:: ini + + [flake8] + +Each command-line option that you want to specify in your config file can +be named in either of two ways: + +#. Using underscores (``_``) instead of hyphens (``-``) + +#. Simply using hyphens (without the leading hyphens) + +So let's actually look at Flake8's own configuration section: + +.. code-block:: ini + + [flake8] + ignore = D203 + exclude = .git,__pycache__,docs/source/conf.py,old,build,dist + max-complexity = 10 + +This is equivalent to: + +.. prompt:: bash + + flake8 --ignore D203 \ + --exclude .git,__pycache__,docs/source/conf.py,old,build,dist \ + --max-complexity 10 + +In our case, if we wanted to, we could also do + +.. code-block:: ini + + [flake8] + ignore = D203 + exclude = + .git, + __pycache__, + docs/source/conf.py, + old, + build, + dist + max-complexity = 10 + +This would allow us to add comments for why we're excluding items, e.g., + +.. code-block:: ini + + [flake8] + ignore = D203 + exclude = + # No need to traverse our git directory + .git, + # There's no value in checking cache directories + __pycache__, + # The conf file is mostly autogenerated, ignore it + docs/source/conf.py, + # The old directory contains Flake8 2.0 + old, + # This contains our built documentation + build, + # This contains builds of flake8 that we don't want to check + dist + max-complexity = 10 + +This is also useful if you have a long list of error codes to ignore. Let's +look at a portion of OpenStack's Swift project configuration: + +.. code-block:: ini + + [flake8] + # it's not a bug that we aren't using all of hacking, ignore: + # F812: list comprehension redefines ... + # H101: Use TODO(NAME) + # H202: assertRaises Exception too broad + # H233: Python 3.x incompatible use of print operator + # H301: one import per line + # H306: imports not in alphabetical order (time, os) + # H401: docstring should not start with a space + # H403: multi line docstrings should end on a new line + # H404: multi line docstring should start without a leading new line + # H405: multi line docstring summary not separated with an empty line + # H501: Do not use self.__dict__ for string formatting + ignore = F812,H101,H202,H233,H301,H306,H401,H403,H404,H405,H501 + +They use the comments to describe the check but they could also write this as: + +.. code-block:: ini + + [flake8] + # it's not a bug that we aren't using all of hacking + ignore = + # F812: list comprehension redefines ... + F812, + # H101: Use TODO(NAME) + H101, + # H202: assertRaises Exception too broad + H202, + # H233: Python 3.x incompatible use of print operator + H233, + # H301: one import per line + H301, + # H306: imports not in alphabetical order (time, os) + H306, + # H401: docstring should not start with a space + H401, + # H403: multi line docstrings should end on a new line + H403, + # H404: multi line docstring should start without a leading new line + H404, + # H405: multi line docstring summary not separated with an empty line + H405, + # H501: Do not use self.__dict__ for string formatting + H501 + +Or they could use each comment to describe **why** they've ignored the check. +:program:`Flake8` knows how to parse these lists and will appropriatey handle +these situations. diff --git a/tests/fixtures/config_files/cli-specified-with-inline-comments.ini b/tests/fixtures/config_files/cli-specified-with-inline-comments.ini new file mode 100644 index 0000000..4d57e85 --- /dev/null +++ b/tests/fixtures/config_files/cli-specified-with-inline-comments.ini @@ -0,0 +1,16 @@ +[flake8] +# This is a flake8 config, there are many like it, but this is mine +ignore = + # Disable E123 + E123, + # Disable W234 + W234, + # Also disable E111 + E111 +exclude = + # Exclude foo/ + foo/, + # Exclude bar/ while we're at it + bar/, + # Exclude bogus/ + bogus/ diff --git a/tests/fixtures/config_files/cli-specified-without-inline-comments.ini b/tests/fixtures/config_files/cli-specified-without-inline-comments.ini new file mode 100644 index 0000000..f50ba75 --- /dev/null +++ b/tests/fixtures/config_files/cli-specified-without-inline-comments.ini @@ -0,0 +1,16 @@ +[flake8] +# This is a flake8 config, there are many like it, but this is mine +# Disable E123 +# Disable W234 +# Also disable E111 +ignore = + E123, + W234, + E111 +# Exclude foo/ +# Exclude bar/ while we're at it +# Exclude bogus/ +exclude = + foo/, + bar/, + bogus/ diff --git a/tests/unit/test_merged_config_parser.py b/tests/unit/test_merged_config_parser.py index c64cae6..baaa57a 100644 --- a/tests/unit/test_merged_config_parser.py +++ b/tests/unit/test_merged_config_parser.py @@ -172,3 +172,36 @@ def test_parse_uses_cli_config(ConfigFileManager, optmanager): parser.parse(cli_config='foo.ini') parser.config_finder.cli_config.assert_called_once_with('foo.ini') + + +@pytest.mark.parametrize('config_fixture_path', [ + 'tests/fixtures/config_files/cli-specified.ini', + 'tests/fixtures/config_files/cli-specified-with-inline-comments.ini', + 'tests/fixtures/config_files/cli-specified-without-inline-comments.ini', +]) +def test_parsed_configs_are_equivalent(optmanager, config_fixture_path): + """Verify the each file matches the expected parsed output. + + This is used to ensure our documented behaviour does not regress. + """ + optmanager.add_option('--exclude', parse_from_config=True, + comma_separated_list=True, + normalize_paths=True) + optmanager.add_option('--ignore', parse_from_config=True, + comma_separated_list=True) + parser = config.MergedConfigParser(optmanager) + config_finder = parser.config_finder + + with mock.patch.object(config_finder, 'local_config_files') as localcfs: + localcfs.return_value = [config_fixture_path] + with mock.patch.object(config_finder, + 'user_config_file') as usercf: + usercf.return_value = [] + parsed_config = parser.merge_user_and_local_config() + + assert parsed_config['ignore'] == ['E123', 'W234', 'E111'] + assert parsed_config['exclude'] == [ + os.path.abspath('foo/'), + os.path.abspath('bar/'), + os.path.abspath('bogus/'), + ]