From 618ab4bea95dba4232d51aa1a93071402934c88f Mon Sep 17 00:00:00 2001 From: wu0607 <33343044+wu0607@users.noreply.github.com> Date: Wed, 5 Feb 2025 12:35:29 -0800 Subject: [PATCH] [Hydra] Improve error message in parse_overrides (#3022) * [Hydra] Improve error message in parse_overrides Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test * Fix test --- .../core/override_parser/overrides_parser.py | 5 +- tests/test_hydra_cli_errors.py | 66 +++++++++++++------ 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/hydra/core/override_parser/overrides_parser.py b/hydra/core/override_parser/overrides_parser.py index 860d3458ad..371963fedd 100644 --- a/hydra/core/override_parser/overrides_parser.py +++ b/hydra/core/override_parser/overrides_parser.py @@ -80,7 +80,7 @@ def parse_override(self, s: str) -> Override: def parse_overrides(self, overrides: List[str]) -> List[Override]: ret: List[Override] = [] - for override in overrides: + for idx, override in enumerate(overrides): try: parsed = self.parse_rule(override, "override") except HydraException as e: @@ -98,7 +98,8 @@ def parse_overrides(self, overrides: List[str]) -> List[Override]: msg = f"Error parsing override '{override}'" f"\n{e}" raise OverrideParseException( override=override, - message=f"{msg}" + message=f"Error when parsing index: {idx}, string: {override} out of {overrides}." + f"\n{msg}" f"\nSee https://hydra.cc/docs/1.2/advanced/override_grammar/basic for details", ) from e.__cause__ assert isinstance(parsed, Override) diff --git a/tests/test_hydra_cli_errors.py b/tests/test_hydra_cli_errors.py index 871b313660..a43bc31e14 100644 --- a/tests/test_hydra_cli_errors.py +++ b/tests/test_hydra_cli_errors.py @@ -1,7 +1,6 @@ # Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved -import re from pathlib import Path -from typing import Any +from typing import Any, List from pytest import mark, param @@ -15,78 +14,103 @@ @mark.parametrize( - "override,expected", + "override,expected_substrings", [ param( "+key=int(", - "no viable alternative at input 'int('", + [ + "Error when parsing index: 1, string: +key=int( out of [", + "no viable alternative at input 'int('", + ], id="parse_error_in_function", ), param( "+key=sort()", - """Error parsing override '+key=sort()' + [ + """Error when parsing index: 1, string: +key=sort() out of [""", + """Error parsing override '+key=sort()' ValueError while evaluating 'sort()': empty sort input""", + ], id="empty_sort", ), param( "key=sort(interval(1,10))", - """Error parsing override 'key=sort(interval(1,10))' + [ + """Error when parsing index: 1, string: key=sort(interval(1,10)) out of [""", + """Error parsing override 'key=sort(interval(1,10))' TypeError while evaluating 'sort(interval(1,10))': mismatch type argument args[0]""", + ], id="sort_interval", ), param( "+key=choice()", - """Error parsing override '+key=choice()' + [ + """Error when parsing index: 1, string: +key=choice() out of [""", + """Error parsing override '+key=choice()' ValueError while evaluating 'choice()': empty choice is not legal""", + ], id="empty choice", ), param( "+key=extend_list(1, 2, 3)", - """Error parsing override '+key=extend_list(1, 2, 3)' + [ + """Error when parsing index: 1, string: +key=extend_list(1, 2, 3) out of [""", + """Error parsing override '+key=extend_list(1, 2, 3)' Trying to use override symbols when extending a list""", + ], id="plus key extend_list", ), param( "key={inner_key=extend_list(1, 2, 3)}", - "no viable alternative at input '{inner_key='", + [ + """Error when parsing index: 1, string: key={inner_key=extend_list(1, 2, 3)} out of [""", + "no viable alternative at input '{inner_key='", + ], id="embedded extend_list", ), param( ["+key=choice(choice(a,b))", "-m"], - """Error parsing override '+key=choice(choice(a,b))' + [ + """Error when parsing index: 1, string: +key=choice(choice(a,b)) out of [""", + """Error parsing override '+key=choice(choice(a,b))' ValueError while evaluating 'choice(choice(a,b))': nesting choices is not supported See https://hydra.cc/docs/1.2/advanced/override_grammar/basic for details Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace. """, + ], id="empty choice", ), param( "--config-dir=/dir/not/found", - f"""Additional config directory '{Path('/dir/not/found').absolute()}' not found + [ + f"""Additional config directory '{Path('/dir/not/found').absolute()}' not found Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace. -""", +""" + ], id="config_dir_not_found", ), ], ) -def test_cli_error(tmpdir: Any, monkeypatch: Any, override: Any, expected: str) -> None: +def test_cli_error( + tmpdir: Any, + monkeypatch: Any, + override: Any, + expected_substrings: List[str], +) -> None: monkeypatch.chdir("tests/test_apps/app_without_config/") if isinstance(override, str): override = [override] cmd = ["my_app.py", "hydra.sweep.dir=" + str(tmpdir)] + override ret = normalize_newlines(run_with_error(cmd)) - assert ( - re.search("^" + re.escape(normalize_newlines(expected.strip())), ret) - is not None - ), ( + missing_substrings = [s for s in expected_substrings if s.strip() not in ret] + assert not missing_substrings, ( f"Result:" f"\n---" f"\n{ret}" f"\n---" - f"\nDid not match expected:" - f"\n---" - f"\n{expected}" - f"\n---" + f"\nDoes not contain the following expected substrings:" + + "\n---\n".join(f"{s}" for s in missing_substrings) + + "\n---" )