From 5ff1d768261c759bbde01d402ada12c79758e7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Tue, 29 Mar 2022 09:16:07 +0545 Subject: [PATCH] ui: add revision info on dvc.yaml/dvc.lock/.dvc syntax and schema errors --- dvc/utils/strictyaml.py | 24 +++++++-- tests/func/utils/test_strict_yaml.py | 75 +++++++++++++++++++++------- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/dvc/utils/strictyaml.py b/dvc/utils/strictyaml.py index ee4ba95a4..0b8cfda00 100644 --- a/dvc/utils/strictyaml.py +++ b/dvc/utils/strictyaml.py @@ -71,13 +71,20 @@ def _prepare_code_snippets( class YAMLSyntaxError(PrettyDvcException, YAMLFileCorruptedError): - def __init__(self, path: str, yaml_text: str, exc: Exception) -> None: + def __init__( + self, + path: str, + yaml_text: str, + exc: Exception, + rev: Optional[str] = None, + ) -> None: self.path: str = path self.yaml_text: str = yaml_text self.exc: Exception = exc merge_conflicts = merge_conflict_marker.search(self.yaml_text) self.hint = " (possible merge conflicts)" if merge_conflicts else "" + self.rev: Optional[str] = rev super().__init__(self.path) def __pretty_exc__(self, **kwargs: Any) -> None: @@ -133,7 +140,9 @@ class YAMLSyntaxError(PrettyDvcException, YAMLFileCorruptedError): lines.insert(0, "") rel = make_relpath(self.path) - lines.insert(0, _prepare_message(f"'{rel}' is invalid{self.hint}.")) + rev_msg = f" in revision '{self.rev[:7]}'" if self.rev else "" + msg_fmt = f"'{rel}' is invalid{self.hint}{rev_msg}." + lines.insert(0, _prepare_message(msg_fmt)) for line in lines: ui.error_write(line, styled=True) @@ -189,6 +198,7 @@ class YAMLValidationError(PrettyDvcException): exc: "MultipleInvalid", path: str = None, text: str = None, + rev: str = None, ) -> None: self.text = text or "" self.exc = exc @@ -197,6 +207,7 @@ class YAMLValidationError(PrettyDvcException): self.path = path or "" message = f"'{rel}' validation failed" + message += " in revision '{}'".format(rev[:7]) if rev else "" if len(self.exc.errors) > 1: message += f": {len(self.exc.errors)} errors" super().__init__(f"{message}") @@ -254,13 +265,14 @@ def validate( schema: Callable[[_T], _T], text: str = None, path: str = None, + rev: str = None, ) -> _T: from voluptuous import MultipleInvalid try: return schema(data) except MultipleInvalid as exc: - raise YAMLValidationError(exc, path, text) from exc + raise YAMLValidationError(exc, path, text, rev=rev) from exc def load( @@ -271,6 +283,8 @@ def load( round_trip: bool = False, ) -> Any: open_fn = fs.open if fs else open + rev = getattr(fs, "rev", None) + try: with open_fn(path, encoding=encoding) as fd: # type: ignore text = fd.read() @@ -279,10 +293,10 @@ def load( raise EncodingError(path, encoding) from exc except YAMLFileCorruptedError as exc: cause = exc.__cause__ - raise YAMLSyntaxError(path, text, exc) from cause + raise YAMLSyntaxError(path, text, exc, rev=rev) from cause if schema: # not returning validated data, as it may remove # details from CommentedMap that we get from roundtrip parser - validate(data, schema, text=text, path=path) + validate(data, schema, text=text, path=path, rev=rev) return data, text diff --git a/tests/func/utils/test_strict_yaml.py b/tests/func/utils/test_strict_yaml.py index 6c4d325b5..c18b0375c 100644 --- a/tests/func/utils/test_strict_yaml.py +++ b/tests/func/utils/test_strict_yaml.py @@ -4,7 +4,6 @@ import pytest from ruamel.yaml import __with_libyaml__ as ruamel_clib from dvc.cli import main -from dvc.ui import ui DUPLICATE_KEYS = """\ stages: @@ -325,25 +324,37 @@ examples = { } +@pytest.fixture +def force_posixpath(mocker): + # make it always return posix path, easier for validating error messages + mocker.patch( + "dvc.utils.strictyaml.make_relpath", + return_value="./dvc.yaml", + ) + + +@pytest.fixture +def fixed_width_term(mocker): + """Fixed width console.""" + from rich.console import Console + + mocker.patch.object( + Console, "width", new_callable=mocker.PropertyMock(return_value=80) + ) + + @pytest.mark.parametrize( "text, expected", examples.values(), ids=examples.keys() ) -def test_exceptions(tmp_dir, dvc, capsys, text, expected, mocker): - # make it always return posix path, easier for validating error messages - mocker.patch( - "dvc.utils.strictyaml.make_relpath", return_value="./dvc.yaml" - ) - - console = ui.error_console - original_printer = console.print - - def print_with_fixed_width(*args, **kwargs): - console.options.min_width = console.options.max_width = 80 - console.width = kwargs["width"] = 80 - return original_printer(*args, **kwargs) - - mocker.patch.object(console, "print", print_with_fixed_width) - +def test_exceptions( + tmp_dir, + dvc, + capsys, + force_posixpath, + fixed_width_term, + text, + expected, +): tmp_dir.gen("dvc.yaml", text) capsys.readouterr() # clear outputs @@ -360,6 +371,36 @@ def test_exceptions(tmp_dir, dvc, capsys, text, expected, mocker): assert expected_line == err_line.rstrip(" ") +@pytest.mark.parametrize( + "text, expected", + [ + (DUPLICATE_KEYS, "'./dvc.yaml' is invalid in revision '{short_rev}'."), + ( + MISSING_CMD, + "'./dvc.yaml' validation failed in revision '{short_rev}'.", + ), + ], +) +def test_on_revision( + tmp_dir, + scm, + dvc, + force_posixpath, + fixed_width_term, + capsys, + text, + expected, +): + tmp_dir.scm_gen("dvc.yaml", text, commit="add dvc.yaml") + capsys.readouterr() # clear outputs + + assert main(["ls", f"file://{tmp_dir}", "--rev", "HEAD"]) != 0 + + out, err = capsys.readouterr() + assert not out + assert expected.format(short_rev=scm.get_rev()[:7]) in err + + def test_make_relpath(tmp_dir, monkeypatch): from dvc.utils.strictyaml import make_relpath