aggregator: Forward --config and --isolated options during aggregation

This fixes a regression introduced in !346 to ensure that `--config` and
`--isolated` are recognized in `aggregate_options()`.

The regression manifested because `aggregate_options()` was relying on
re-parsing `argv` to obtain the option values.  However, !346 changed
the preliminary parsing logic to only parse and "eat" what is necessary
and forward along the options needed before all the configuration was
loaded.  This code path was overlooked because the tests in
`test_aggregator()` were passing but the call from the `Application`
object would never have these options in the remaining `argv` list to be
passed long.
This commit is contained in:
Eric N. Vander Weele 2019-12-29 17:22:26 -05:00
parent bb61b3df82
commit 7f9f70064c
5 changed files with 46 additions and 19 deletions

View file

@ -39,7 +39,12 @@ def get_style_guide(**kwargs):
config_finder, prelim_opts.config, prelim_opts.isolated
)
application.register_plugin_options()
application.parse_configuration_and_cli(config_finder, remaining_args)
application.parse_configuration_and_cli(
config_finder,
prelim_opts.config,
prelim_opts.isolated,
remaining_args,
)
# We basically want application.initialize to be called but with these
# options set instead before we make our formatter, notifier, internal
# style guide and file checker manager.

View file

@ -169,17 +169,33 @@ class Application(object):
self.check_plugins.register_plugin_versions(self.option_manager)
self.formatting_plugins.register_options(self.option_manager)
def parse_configuration_and_cli(self, config_finder, argv):
# type: (config.ConfigFileFinder, List[str]) -> None
def parse_configuration_and_cli(
self,
config_finder, # type: config.ConfigFileFinder
config_file, # type: Optional[str]
ignore_config_files, # type: bool
argv, # type: List[str]
):
# type: (...) -> None
"""Parse configuration files and the CLI options.
:param config.ConfigFileFinder config_finder:
The finder for finding and reading configuration files.
:param str config_file:
The optional configuraiton file to override all other configuration
files (i.e., the --config option).
:param bool ignore_config_files:
Determine whether to parse configuration files or not. (i.e., the
--isolated option).
:param list argv:
Command-line arguments passed in directly.
"""
self.options, self.args = aggregator.aggregate_options(
self.option_manager, config_finder, argv
self.option_manager,
config_finder,
config_file,
ignore_config_files,
argv,
)
self.running_against_diff = self.options.diff
@ -325,7 +341,12 @@ class Application(object):
config_finder, prelim_opts.config, prelim_opts.isolated
)
self.register_plugin_options()
self.parse_configuration_and_cli(config_finder, remaining_args)
self.parse_configuration_and_cli(
config_finder,
prelim_opts.config,
prelim_opts.isolated,
remaining_args,
)
self.make_formatter()
self.make_guide()
self.make_file_checker_manager()

View file

@ -5,7 +5,7 @@ applies the user-specified command-line configuration on top of it.
"""
import argparse
import logging
from typing import List, Tuple
from typing import List, Optional, Tuple
from flake8.options import config
from flake8.options.manager import OptionManager
@ -16,6 +16,8 @@ LOG = logging.getLogger(__name__)
def aggregate_options(
manager, # type: OptionManager
config_finder, # type: config.ConfigFileFinder
cli_config, # type: Optional[str]
isolated, # type: bool
argv, # type: List[str]
): # type: (...) -> Tuple[argparse.Namespace, List[str]]
"""Aggregate and merge CLI and config file options.
@ -24,6 +26,12 @@ def aggregate_options(
The instance of the OptionManager that we're presently using.
:param flake8.options.config.ConfigFileFinder config_finder:
The config file finder to use.
:param str cli_config:
Value of --config when specified at the command-line. Overrides
all other config files.
:param bool isolated:
Determines if we should parse configuration files at all or not.
If running in isolated mode, we ignore all configuration files
:param list argv:
The list of remaining command-line argumentsthat were unknown during
preliminary option parsing to pass to ``manager.parse_args``.
@ -35,9 +43,6 @@ def aggregate_options(
"""
# Get defaults from the option parser
default_values, _ = manager.parse_args([])
# Get original CLI values so we can find additional config file paths and
# see if --config was specified.
original_values, _ = manager.parse_args(argv)
# Make our new configuration file mergerator
config_parser = config.MergedConfigParser(
@ -45,9 +50,7 @@ def aggregate_options(
)
# Get the parsed config
parsed_config = config_parser.parse(
original_values.config, original_values.isolated
)
parsed_config = config_parser.parse(cli_config, isolated)
# Extend the default ignore value with the extended default ignore list,
# registered by plugins.

View file

@ -28,13 +28,12 @@ def optmanager():
def test_aggregate_options_with_config(optmanager):
"""Verify we aggregate options and config values appropriately."""
arguments = ['flake8', '--config', CLI_SPECIFIED_CONFIG, '--select',
arguments = ['flake8', '--select',
'E11,E34,E402,W,F', '--exclude', 'tests/*']
config_finder = config.ConfigFileFinder('flake8', [])
options, args = aggregator.aggregate_options(
optmanager, config_finder, arguments)
optmanager, config_finder, CLI_SPECIFIED_CONFIG, False, arguments)
assert options.config == CLI_SPECIFIED_CONFIG
assert options.select == ['E11', 'E34', 'E402', 'W', 'F']
assert options.ignore == ['E123', 'W234', 'E111']
assert options.exclude == [os.path.abspath('tests/*')]
@ -42,14 +41,13 @@ def test_aggregate_options_with_config(optmanager):
def test_aggregate_options_when_isolated(optmanager):
"""Verify we aggregate options and config values appropriately."""
arguments = ['flake8', '--isolated', '--select', 'E11,E34,E402,W,F',
arguments = ['flake8', '--select', 'E11,E34,E402,W,F',
'--exclude', 'tests/*']
config_finder = config.ConfigFileFinder('flake8', [])
optmanager.extend_default_ignore(['E8'])
options, args = aggregator.aggregate_options(
optmanager, config_finder, arguments)
optmanager, config_finder, None, True, arguments)
assert options.isolated is True
assert options.select == ['E11', 'E34', 'E402', 'W', 'F']
assert sorted(options.ignore) == [
'E121', 'E123', 'E126', 'E226', 'E24', 'E704', 'E8', 'W503', 'W504',

View file

@ -34,7 +34,7 @@ def test_get_style_guide():
mockedapp.find_plugins.assert_called_once_with(config_finder, None, False)
mockedapp.register_plugin_options.assert_called_once_with()
mockedapp.parse_configuration_and_cli.assert_called_once_with(
config_finder, [])
config_finder, None, False, [])
mockedapp.make_formatter.assert_called_once_with()
mockedapp.make_guide.assert_called_once_with()
mockedapp.make_file_checker_manager.assert_called_once_with()