Skip to content

Commit 4006012

Browse files
authored
Tighten up options tests (#20660)
This fixes a few loose behaviors in the tests that happen to work, but that we have never documented or claimed to support. Notably, we test an option type of "list of dicts", but we have no actual options of that type, have never documented support for it, and it is not supported by our new-style declarative registration classes. We also tighten up a couple of places where we implicitly relied on the assumption that option values (including list members) were roundtripped via a `str`. But of course this is not guaranteed, and we have never claimed that this should work. These were noticed while manually testing the Rust options parser against these Python tests. This PR is groundwork for switching Python code to use the rust parser.
1 parent b8b3c72 commit 4006012

File tree

1 file changed

+5
-52
lines changed

1 file changed

+5
-52
lines changed

src/python/pants/option/options_test.py

+5-52
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ def no_exception():
248248
),
249249
),
250250
(
251-
dict(type=bool, default="True"),
251+
dict(type=bool, default=True),
252252
no_exception(),
253253
),
254254
(
@@ -597,9 +597,6 @@ def register_global(*args, **kwargs):
597597
# Custom types.
598598
register_global("--listy", type=list, member_type=int, default="[1, 2, 3]")
599599
register_global("--dicty", type=dict, default='{"a": "b"}')
600-
register_global(
601-
"--dict-listy", type=list, member_type=dict, default='[{"a": 1, "b": 2}, {"c": 3}]'
602-
)
603600
register_global("--targety", type=target_option, default="//:a")
604601
register_global(
605602
"--target-listy", type=list, member_type=target_option, default=["//:a", "//:b"]
@@ -748,10 +745,6 @@ def test_arg_scoping() -> None:
748745
global_options = _parse(flags='--dicty=\'{"c": "d"}\'').for_global_scope()
749746
assert {"c": "d"} == global_options.dicty
750747

751-
# Test list-of-dict-typed option.
752-
global_options = _parse(flags='--dict-listy=\'[{"c": "d"}, {"e": "f"}]\'').for_global_scope()
753-
assert [{"c": "d"}, {"e": "f"}] == global_options.dict_listy
754-
755748
# Test target-typed option.
756749
global_options = _parse().for_global_scope()
757750
assert "//:a" == global_options.targety
@@ -896,46 +889,6 @@ def check(
896889
check(env_val="[4]", config_val="-[4]", expected=[4])
897890

898891

899-
def test_dict_list_option() -> None:
900-
def check(
901-
*,
902-
expected: list[dict[str, int]],
903-
flags: str = "",
904-
env_val: str | None = None,
905-
config_val: str | None = None,
906-
) -> None:
907-
env = {"PANTS_GLOBAL_DICT_LISTY": env_val} if env_val else None
908-
config = {"GLOBAL": {"dict_listy": config_val}} if config_val else None
909-
global_options = _parse(flags=flags, env=env, config=config).for_global_scope()
910-
assert global_options.dict_listy == expected
911-
912-
default = [{"a": 1, "b": 2}, {"c": 3}]
913-
one_element_appended = [*default, {"d": 4, "e": 5}]
914-
two_elements_appended = [*one_element_appended, {"f": 6}]
915-
replaced = [{"d": 4, "e": 5}, {"f": 6}]
916-
917-
check(expected=default)
918-
919-
check(flags='--dict-listy=\'{"d": 4, "e": 5}\'', expected=one_element_appended)
920-
check(
921-
flags='--dict-listy=\'{"d": 4, "e": 5}\' --dict-listy=\'{"f": 6}\'',
922-
expected=two_elements_appended,
923-
)
924-
check(
925-
flags='--dict-listy=\'+[{"d": 4, "e": 5}, {"f": 6}]\'',
926-
expected=two_elements_appended,
927-
)
928-
check(flags='--dict-listy=\'[{"d": 4, "e": 5}, {"f": 6}]\'', expected=replaced)
929-
930-
check(env_val='{"d": 4, "e": 5}', expected=one_element_appended)
931-
check(env_val='+[{"d": 4, "e": 5}, {"f": 6}]', expected=two_elements_appended)
932-
check(env_val='[{"d": 4, "e": 5}, {"f": 6}]', expected=replaced)
933-
934-
check(config_val='{"d": 4, "e": 5}', expected=one_element_appended)
935-
check(config_val='+[{"d": 4, "e": 5}, {"f": 6}]', expected=two_elements_appended)
936-
check(config_val='[{"d": 4, "e": 5}, {"f": 6}]', expected=replaced)
937-
938-
939892
def test_target_list_option() -> None:
940893
def check(
941894
*,
@@ -1436,8 +1389,8 @@ def _do_assert_fromfile(dest, expected, contents, passthru_flags=""):
14361389
contents=dedent(
14371390
"""
14381391
['a',
1439-
1,
1440-
2]
1392+
'1',
1393+
'2']
14411394
"""
14421395
),
14431396
)
@@ -1447,8 +1400,8 @@ def _do_assert_fromfile(dest, expected, contents, passthru_flags=""):
14471400
contents=dedent(
14481401
"""
14491402
['a',
1450-
1,
1451-
2]
1403+
'1',
1404+
'2']
14521405
"""
14531406
),
14541407
passthru_flags="bob @jake",

0 commit comments

Comments
 (0)