From 3f434f7d1cf3f7a8e48456410db94376bae4bc24 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Thu, 16 Jun 2016 16:19:09 -0500 Subject: [PATCH] Add broken config file to test error handling ConfigFileFinder should absolutely handle broken/invalid config files by refusing to try to parse them. Here we catch the ParsingError, log the exception, and then return normally. The RawConfigParser instance is perfectly valid still and will behave as if nothing had been read and we just need to indicate that we didn't find any files worthy of reading. Related to: https://github.com/PyCQA/pycodestyle/issues/506 --- flake8/options/config.py | 8 +++++++- tests/fixtures/config_files/broken.ini | 9 +++++++++ tests/unit/test_config_file_finder.py | 10 ++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/config_files/broken.ini diff --git a/flake8/options/config.py b/flake8/options/config.py index 9bd7e19..48719a8 100644 --- a/flake8/options/config.py +++ b/flake8/options/config.py @@ -52,7 +52,13 @@ class ConfigFileFinder(object): @staticmethod def _read_config(files): config = configparser.RawConfigParser() - found_files = config.read(files) + try: + found_files = config.read(files) + except configparser.ParsingError: + LOG.exception("There was an error trying to parse a config " + "file. The files we were attempting to parse " + "were: %r", files) + found_files = [] return (config, found_files) def cli_config(self, files): diff --git a/tests/fixtures/config_files/broken.ini b/tests/fixtures/config_files/broken.ini new file mode 100644 index 0000000..33986ae --- /dev/null +++ b/tests/fixtures/config_files/broken.ini @@ -0,0 +1,9 @@ +[flake8] +exclude = +<<<<<<< 642f88cb1b6027e184d9a662b255f7fea4d9eacc + tests/fixtures/, +======= + tests/, +>>>>>>> HEAD + docs/ +ignore = D203 diff --git a/tests/unit/test_config_file_finder.py b/tests/unit/test_config_file_finder.py index 8d7e920..2c56685 100644 --- a/tests/unit/test_config_file_finder.py +++ b/tests/unit/test_config_file_finder.py @@ -10,6 +10,7 @@ import mock import pytest CLI_SPECIFIED_FILEPATH = 'tests/fixtures/config_files/cli-specified.ini' +BROKEN_CONFIG_PATH = 'tests/fixtures/config_files/broken.ini' def test_uses_default_args(): @@ -115,3 +116,12 @@ def test_local_configs(): finder = config.ConfigFileFinder('flake8', None, []) assert isinstance(finder.local_configs(), configparser.RawConfigParser) + + +@pytest.mark.parametrize('files', [ + [BROKEN_CONFIG_PATH], + [CLI_SPECIFIED_FILEPATH, BROKEN_CONFIG_PATH], +]) +def test_read_config_catches_broken_config_files(files): + """Verify that we do not allow the exception to bubble up.""" + assert config.ConfigFileFinder._read_config(files)[1] == []