From dc3f0ac968f117dfa21e367870a38aafb5c5742d Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Mon, 11 Nov 2024 17:32:02 -0800 Subject: [PATCH] Test the option value derivation details. (#21628) Previously we only checked that the detail text was as expected for cli flags, now we check for env vars and config too. Also, fix the implementation so it actually passes the test as expected. --- src/python/pants/help/help_info_extracter.py | 5 +- .../pants/help/help_info_extracter_test.py | 47 ++++++++++++------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/python/pants/help/help_info_extracter.py b/src/python/pants/help/help_info_extracter.py index 18fff9a7233..dc087531d5e 100644 --- a/src/python/pants/help/help_info_extracter.py +++ b/src/python/pants/help/help_info_extracter.py @@ -1040,10 +1040,11 @@ def get_option_scope_help_info( # Again, distinguishing between '' vs None in the details field # does not matter, but we want to be consistent with the idiosyncratic # behavior of the legacy parser, until we get rid of it. - details = rank.description() or empty_details + details = ( + ", ".join(filter(None, [r.description() for r in ranks])) or empty_details + ) ranked_values.append(RankedValue(rank, value, details)) history = OptionValueHistory(tuple(ranked_values)) - # history = parser.history(kwargs["dest"]) ohi = self.get_option_help_info(args, kwargs) ohi = dataclasses.replace(ohi, value_history=history) if ohi.deprecation_active: diff --git a/src/python/pants/help/help_info_extracter_test.py b/src/python/pants/help/help_info_extracter_test.py index d8105bb3e88..a88e02d0ce1 100644 --- a/src/python/pants/help/help_info_extracter_test.py +++ b/src/python/pants/help/help_info_extracter_test.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from enum import Enum +from types import SimpleNamespace from typing import Any, Iterable, List, Optional, Tuple, Union from pants.base.build_environment import get_buildroot @@ -15,7 +16,7 @@ from pants.option.config import Config from pants.option.global_options import GlobalOptions, LogLevelOption from pants.option.native_options import NativeOptionParser -from pants.option.option_types import BoolOption, IntOption, StrListOption +from pants.option.option_types import BoolOption, IntListOption, StrListOption from pants.option.options import Options from pants.option.parser import Parser from pants.option.ranked_value import Rank @@ -217,12 +218,12 @@ def exp_to_len(exp): do_test({"advanced": True}, expected_advanced=True) -def test_get_all_help_info(): +def test_get_all_help_info(tmp_path) -> None: class Global(Subsystem): options_scope = GLOBAL_SCOPE help = help_text("Global options.") - opt1 = IntOption(default=42, help="Option 1") + opt1 = IntListOption(default=[42], help="Option 1") # This is special in having a short option `-l`. Make sure it works. level = LogLevelOption() @@ -256,11 +257,13 @@ class BazLibrary(Target): alias = "baz_library" help = "A library of baz-es.\n\nUse it however you like." - core_fields = [QuxField, QuuxField] + core_fields = (QuxField, QuuxField) + config_path = "pants.test.toml" + config_source = SimpleNamespace(path=config_path, content=b"[GLOBAL]\nopt1 = '+[99]'") options = Options.create( - env={}, - config=Config.load([]), + env={"PANTS_OPT1": "88"}, + config=Config.load([config_source]), native_options_config_discovery=False, known_scope_infos=[Global.get_scope_info(), Foo.get_scope_info(), Bar.get_scope_info()], args=["./pants", "--backend-packages=['internal_plugins.releases']"], @@ -309,20 +312,24 @@ def fake_consumed_scopes_mapper(scope: str) -> Tuple[str, ...]: "deprecated_scope": None, "basic": ( { - "display_args": ("--opt1=",), - "comma_separated_display_args": "--opt1=", + "display_args": ('--opt1="[, , ...]"',), + "comma_separated_display_args": '--opt1="[, , ...]"', "scoped_cmd_line_args": ("--opt1",), "unscoped_cmd_line_args": ("--opt1",), "config_key": "opt1", "env_var": "PANTS_OPT1", "value_history": { "ranked_values": ( - {"rank": Rank.NONE, "value": None, "details": None}, - {"rank": Rank.HARDCODED, "value": 42, "details": None}, + {"rank": Rank.NONE, "value": [], "details": ""}, + { + "rank": Rank.ENVIRONMENT, + "value": [42, 99, 88], + "details": "from config, from an env var", + }, ), }, - "typ": int, - "default": 42, + "typ": list, + "default": [42], "fromfile": False, "help": "Option 1", "deprecation_active": False, @@ -628,20 +635,24 @@ def fake_consumed_scopes_mapper(scope: str) -> Tuple[str, ...]: }, "env_var_to_help_info": { "PANTS_OPT1": { - "display_args": ("--opt1=",), - "comma_separated_display_args": "--opt1=", + "display_args": ('--opt1="[, , ...]"',), + "comma_separated_display_args": '--opt1="[, , ...]"', "scoped_cmd_line_args": ("--opt1",), "unscoped_cmd_line_args": ("--opt1",), "config_key": "opt1", "env_var": "PANTS_OPT1", "value_history": { "ranked_values": ( - {"rank": Rank.NONE, "value": None, "details": None}, - {"rank": Rank.HARDCODED, "value": 42, "details": None}, + {"rank": Rank.NONE, "value": [], "details": ""}, + { + "rank": Rank.ENVIRONMENT, + "value": [42, 99, 88], + "details": "from config, from an env var", + }, ), }, - "typ": int, - "default": 42, + "typ": list, + "default": [42], "fromfile": False, "help": "Option 1", "deprecation_active": False,