From 0ff2f1b137b9e7aa07bb4d85c191084fe3840186 Mon Sep 17 00:00:00 2001 From: oleks-dev <1228369+oleks-dev@users.noreply.github.com> Date: Sun, 14 Sep 2025 18:59:38 +0200 Subject: [PATCH] new validate command tests added; validate feature fixes and updates --- prich/cli/validate.py | 42 +++++++++---- prich/core/file_scope.py | 8 +-- prich/core/loaders.py | 2 +- prich/core/utils.py | 2 +- tests/resources/empty.yaml | 0 tests/resources/no_id.yaml | 11 ++++ tests/resources/no_name.yaml | 8 +++ tests/resources/no_schema_version.yaml | 2 + tests/resources/no_shared_venv.yaml | 11 ++++ tests/resources/no_steps.yaml | 6 ++ tests/resources/no_venv.yaml | 11 ++++ tests/resources/not_supported_schema.yaml | 3 + tests/resources/wrong_steps.yaml | 15 +++++ tests/resources/wrong_variables.yaml | 28 +++++++++ tests/resources/yaml_error.yaml | 10 +++ tests/test_template_steps.py | 3 +- tests/test_validate.py | 77 ++++++++++++++++++++++- 17 files changed, 215 insertions(+), 24 deletions(-) create mode 100644 tests/resources/empty.yaml create mode 100644 tests/resources/no_id.yaml create mode 100644 tests/resources/no_name.yaml create mode 100644 tests/resources/no_schema_version.yaml create mode 100644 tests/resources/no_shared_venv.yaml create mode 100644 tests/resources/no_steps.yaml create mode 100644 tests/resources/no_venv.yaml create mode 100644 tests/resources/not_supported_schema.yaml create mode 100644 tests/resources/wrong_steps.yaml create mode 100644 tests/resources/wrong_variables.yaml create mode 100644 tests/resources/yaml_error.yaml diff --git a/prich/cli/validate.py b/prich/cli/validate.py index 8ef0a1d..0d61643 100644 --- a/prich/cli/validate.py +++ b/prich/cli/validate.py @@ -6,7 +6,7 @@ from pydantic import ValidationError as PydanticValidationError from prich.constants import PRICH_DIR_NAME from prich.models.template import CommandStep, PythonStep -from prich.core.file_scope import classify_path +from prich.core.file_scope import classify_path, normalize_path from prich.core.loaders import find_template_files, load_template_model, get_env_vars, _load_yaml from prich.core.utils import console_print, shorten_path, get_prich_dir, is_just_filename, get_cwd_dir, get_home_dir @@ -21,7 +21,7 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat if template_yaml and err.get("loc"): # hide extra layering if err.get("loc")[0] == "steps": - if err.get("loc")[2] in ['llm', 'command', 'python', 'render']: + if len(err.get("loc")) >= 3 and err.get("loc")[2] in ['llm', 'command', 'python', 'render']: details = err.get("loc")[2] err['loc'] = tuple(err.get("loc")[:2] + err.get("loc")[3:]) @@ -29,6 +29,8 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat loc_list = err.get("loc")[:-1] if len(err.get("loc")) > 1 else err.get("loc") for x in loc_list: try: + if trace_dir[x] is None: + break trace_dir = trace_dir[x] except Exception: break @@ -45,8 +47,10 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat template_overview = re.sub("(\\.\\.\\.)", f"[yellow]+{err.get('loc')[-1]}: ...[/yellow]\n\\1", template_overview, count=1) else: + if 'Input tag ' in err.get('msg'): + err["msg"] = err['msg'].replace("Input tag ", "Field value ").replace(" any of the expected tags", " any of the expected values") if 'Input should be' in err.get('msg'): - err["msg"] = err['msg'].replace("Input should be", "Field value should be") + err["msg"] = err['msg'].replace("Input should be ", "Field value should be ") highligh_block = err.get('loc')[-1] if isinstance(err.get('loc')[-1], str) else err.get('loc')[-2] template_overview = re.sub(f"([\n]*(?:\\s+)|^)({highligh_block})(:)", "\\1[red]\\2[/red]\\3", template_overview, count=1) @@ -92,7 +96,7 @@ def template_model_doctor(template_yaml: dict, model_load_error: PydanticValidat else: doc = "See Template Content Documentation https://oleks-dev.github.io/prich/reference/template/content/" err_loc_string = re.sub(f"({err.get('loc')[-1]})$", "[red]\\1[/red]", err_loc_string) - found_issues_list.append(f"""{len(found_issues_list)+1}. [red]{err.get('msg')}[/red] '[white]{err_loc_string}[/white]':\n[white]{template_overview}[/white]{doc}""") + found_issues_list.append(f"""{len(found_issues_list)+1}. [red]{err.get('msg')}[/red] at '[white]{err_loc_string}[/white]':\n[white]{template_overview}[/white]{doc}""") return found_issues_list @@ -111,7 +115,10 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool, if validate_file and (global_only or local_only or template_id): raise click.ClickException(f"When YAML file is selected it doesn't combine with local, global, or id options, use: 'prich validate --file ./{PRICH_DIR_NAME}/templates/test-template/test-template.yaml'") - if validate_file and not validate_file.exists(): + if validate_file: + validate_file = normalize_path(validate_file, cwd=get_cwd_dir()) + + if validate_file and (not validate_file.exists() or not validate_file.is_file()): raise click.ClickException(f"Failed to find {validate_file} template file.") # Load Template Files @@ -154,10 +161,9 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool, model_failures_count = 0 output = [] try: - if template_file.is_file(): - template_yaml = _load_yaml(template_file) - template_id = template_yaml.get("id") if template_yaml else None - template_name = template_yaml.get("name") if template_yaml else None + template_yaml = _load_yaml(template_file) + template_id = template_yaml.get("id") if template_yaml else None + template_name = template_yaml.get("name") if template_yaml else None try: template = load_template_model(template_file) except PydanticValidationError as e: @@ -169,8 +175,18 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool, raise click.ClickException(f"1. [red]{str(e)}[/red]") if template.venv in ["isolated", "shared"]: venv_folder = (Path(template.folder) / "scripts") if template.venv == "isolated" else get_prich_dir() / "venv" + python_steps = [step for step in template.steps if step.type == 'python'] + if not python_steps: + extra_note = ". There are no steps with type 'python' found, if python is not used you can remove the 'venv' parameter from the template" + else: + extra_note = "" + if template.venv == 'isolated': + installation_note = f" Install it by running 'prich venv-install {template.id}'." + else: + # TODO: introduce help for shared venv installation + installation_note = "" if not venv_folder.exists(): - failures_list.append(f"{len(failures_list)+1}. [red]Failed to find {template.venv} venv at {shorten_path(str(venv_folder))}.[/red] Install it by running 'prich venv-install {template.id}'.") + failures_list.append(f"{len(failures_list)+1}. [red]Failed to find {template.venv} venv at {shorten_path(str(venv_folder))}{extra_note}.[/red]{installation_note}") idx = 0 for step in template.steps: idx += 1 @@ -196,7 +212,7 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool, output.append(f"- {template.id} [dim]({template.source.value}) {shorten_path(str(template_file))}[/dim]: ") if len(failures_list) > 0: failures_found = True - output[-1] += f"[red]is not valid[/red] ({len(failures_list)} issues)" + output[-1] += f"[red]is not valid[/red] ({len(failures_list)} issue{'s' if len(failures_list)>1 else ''})" failures = ' ' + '\n '.join(failures_list) output.append(failures) output.append("") @@ -206,11 +222,11 @@ def validate_templates(template_id: str, validate_file: Path, global_only: bool, failures_found = True template_source = classify_path(template_file) error_lines = ' ' + '\n '.join([x for x in e.message.split('\n')]) + '\n' - output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] {f'({model_failures_count} issues)' if model_failures_count > 0 else '(1 issue)'}\n [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n{error_lines}""") + output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] {f'({model_failures_count} issue{"s" if model_failures_count > 1 else ""})' if model_failures_count > 0 else '(1 issue)'}\n [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n{error_lines}""") except Exception as e: failures_found = True template_source = classify_path(template_file) - output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] (1 issue)\n [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n {str(e)}""") + output.append(f"""- {f"{template_id} " if template_id else ''}[dim]({template_source.value}) {shorten_path(str(template_file))}[/dim]: [red]is not valid[/red] (1 issue)\n [red]Failed to load template{f" {template_id}" if template_id else ""}{f" ({template_name})" if template_name else ""}[/red]:\n 1. {str(e)}""") if (invalid_only and failures_found) or not invalid_only: console_print('\n'.join(output)) if failures_found: diff --git a/prich/core/file_scope.py b/prich/core/file_scope.py index ed40da6..f969914 100644 --- a/prich/core/file_scope.py +++ b/prich/core/file_scope.py @@ -6,7 +6,7 @@ from prich.models.file_scope import FileScope -def _normalize(p: Path, *, cwd: Path) -> Path: +def normalize_path(p: Path, *, cwd: Path) -> Path: """ Expand ~, make absolute relative to cwd, and resolve as much as possible. Works for non-existent paths too (resolves the existing parent). @@ -66,9 +66,9 @@ def classify_path( global_root = home / PRICH_DIR_NAME if follow_symlinks: - p = _normalize(file, cwd=cwd) - lr = _normalize(local_root, cwd=cwd) - gr = _normalize(global_root, cwd=cwd) + p = normalize_path(file, cwd=cwd) + lr = normalize_path(local_root, cwd=cwd) + gr = normalize_path(global_root, cwd=cwd) else: # Don't resolve symlinks; still expand and absolutize p = (cwd / Path(file).expanduser()) if not Path(file).expanduser().is_absolute() else Path(file).expanduser() diff --git a/prich/core/loaders.py b/prich/core/loaders.py index 399e228..637216b 100644 --- a/prich/core/loaders.py +++ b/prich/core/loaders.py @@ -15,7 +15,7 @@ def _load_yaml(path: Path) -> Dict: import yaml - if not path.exists(): + if not path.exists() or not path.is_file(): return {} with path.open("r", encoding="utf-8") as f: return yaml.safe_load(f) or {} diff --git a/prich/core/utils.py b/prich/core/utils.py index 9280d2f..5b0f367 100644 --- a/prich/core/utils.py +++ b/prich/core/utils.py @@ -113,7 +113,7 @@ def get_prich_templates_dir(global_only: bool = None) -> Path: def shorten_path(path: str | Path) -> str: """ Return short path using ~/... or ./... instead of a full absolute path """ - home = str(Path.home()) + home = str(get_home_dir()) cwd = str(get_cwd_dir()) if isinstance(path, Path): path = str(path) diff --git a/tests/resources/empty.yaml b/tests/resources/empty.yaml new file mode 100644 index 0000000..e69de29 diff --git a/tests/resources/no_id.yaml b/tests/resources/no_id.yaml new file mode 100644 index 0000000..7b14842 --- /dev/null +++ b/tests/resources/no_id.yaml @@ -0,0 +1,11 @@ +name: Test Template +version: '1.0' +description: Example template +tags: +- example +- writer +steps: +- name: Ask to generate text + type: llm + input: Generate short phrase +schema_version: '1.0' diff --git a/tests/resources/no_name.yaml b/tests/resources/no_name.yaml new file mode 100644 index 0000000..ca6426f --- /dev/null +++ b/tests/resources/no_name.yaml @@ -0,0 +1,8 @@ +id: test-template +version: '1.0' +description: Example template +steps: +- name: Ask to generate text + type: llm + input: Generate short phrase +schema_version: '1.0' diff --git a/tests/resources/no_schema_version.yaml b/tests/resources/no_schema_version.yaml new file mode 100644 index 0000000..5a9c5d8 --- /dev/null +++ b/tests/resources/no_schema_version.yaml @@ -0,0 +1,2 @@ +id: "test" +name: "test" diff --git a/tests/resources/no_shared_venv.yaml b/tests/resources/no_shared_venv.yaml new file mode 100644 index 0000000..fab2df7 --- /dev/null +++ b/tests/resources/no_shared_venv.yaml @@ -0,0 +1,11 @@ +id: test-template +name: Test Template +version: '1.0' +description: Example template - Generate text about specified topic +venv: "shared" +steps: +- name: Ask to generate text + type: llm + instructions: You are {{ role }} + input: Generate text about {{ topic }} +schema_version: '1.0' diff --git a/tests/resources/no_steps.yaml b/tests/resources/no_steps.yaml new file mode 100644 index 0000000..40e62de --- /dev/null +++ b/tests/resources/no_steps.yaml @@ -0,0 +1,6 @@ +id: test-template +name: test template +version: '1.0' +description: Example template +steps: +schema_version: '1.0' diff --git a/tests/resources/no_venv.yaml b/tests/resources/no_venv.yaml new file mode 100644 index 0000000..09777d9 --- /dev/null +++ b/tests/resources/no_venv.yaml @@ -0,0 +1,11 @@ +id: test-template +name: Test Template +version: '1.0' +description: Example template - Generate text about specified topic +venv: "isolated" +steps: +- name: Ask to generate text + type: llm + instructions: You are {{ role }} + input: Generate text about {{ topic }} +schema_version: '1.0' diff --git a/tests/resources/not_supported_schema.yaml b/tests/resources/not_supported_schema.yaml new file mode 100644 index 0000000..f29791c --- /dev/null +++ b/tests/resources/not_supported_schema.yaml @@ -0,0 +1,3 @@ +id: "test" +name: "test" +schema_version: "0.1" diff --git a/tests/resources/wrong_steps.yaml b/tests/resources/wrong_steps.yaml new file mode 100644 index 0000000..430cda7 --- /dev/null +++ b/tests/resources/wrong_steps.yaml @@ -0,0 +1,15 @@ +id: test-template +name: test template +version: '1.0' +description: Example template +steps: +- name: Ask to generate 1st text + type: provider + input: Generate short phrase +- step_name: Ask to generate 2nd text + type: llm + input: Generate short phrase +- name: Ask to generate 3nd text + type: llm + input: Generate short phrase +schema_version: '1.0' diff --git a/tests/resources/wrong_variables.yaml b/tests/resources/wrong_variables.yaml new file mode 100644 index 0000000..8389703 --- /dev/null +++ b/tests/resources/wrong_variables.yaml @@ -0,0 +1,28 @@ +id: test-template +name: test template +version: '1.0' +description: Example template +steps: +- name: Ask to generate 3nd text + type: llm + input: You are an engineer +variables: + - var_name: "test" + - name: "test-variable" + - name: "test_variable" + type: "integer" + - name: "test_variable" + required: "hello" + - name: "test_variable" + default: null + - name: "test_variable" + cli_option: "test-var" + +# - name: role + # type: str + # description: Role of the Assistant + # default: article writer + # required: false + # cli_option: --role + +schema_version: '1.0' diff --git a/tests/resources/yaml_error.yaml b/tests/resources/yaml_error.yaml new file mode 100644 index 0000000..9c47198 --- /dev/null +++ b/tests/resources/yaml_error.yaml @@ -0,0 +1,10 @@ +id: test-template +name: Test Template +version: '1.0' +description_ Example template - Generate text about specified topic +steps: +- name: Ask to generate text + type: llm + instructions: You are {{ role }} + input: Generate text about {{ topic }} +schema_version: '1.0' diff --git a/tests/test_template_steps.py b/tests/test_template_steps.py index 0fa291c..b24c6cf 100644 --- a/tests/test_template_steps.py +++ b/tests/test_template_steps.py @@ -5,7 +5,6 @@ from prich.models.config import SettingsConfig, ProviderModeModel from prich.models.template import TemplateModel, LLMStep from prich.models.config import ConfigModel -from tests.fixtures.paths import mock_paths from tests.utils.utils import capture_stdout sample_variables = { @@ -321,7 +320,7 @@ def test_step_render_template(monkeypatch, template_string, expected, expected_e }, ] @pytest.mark.parametrize("case", get_step_send_to_llm_CASES, ids=[c["id"] for c in get_step_send_to_llm_CASES]) -def test_step_send_to_llm(tmp_path, mock_paths, case, monkeypatch): +def test_step_send_to_llm(tmp_path, case, monkeypatch): from prich.core.steps.step_send_to_llm import send_to_llm if case.get("expected_exception"): diff --git a/tests/test_validate.py b/tests/test_validate.py index 455777f..9225aef 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -1,3 +1,4 @@ +import os from pathlib import Path import pytest from click.testing import CliRunner @@ -19,7 +20,12 @@ {"id": "file_with_template_id_param", "args": ["--file", "test.yaml", "--id", "test-template"], "expected_output": "When YAML file is selected it doesn't combine with local, global, or id options"}, {"id": "file_not_present", "args": ["--file", "test.yaml"], - "expected_output": "Failed to find test.yaml template file."}, + "expected_output": ["Failed to find", "test.yaml template file."]}, + {"id": "path_as_file", "args": ["--file", "."], + "expected_output": ["Failed to find", " template file."]}, + {"id": "wrong_template_from_file", "add_wrong_template": True, + "args": ["--file", "./.prich/templates/tpl-local-wrong/tpl-local-wrong.yaml"], + "expected_output": "tpl-local-wrong.yaml: is not valid"}, {"id": "no_templates_found", "args": [], "expected_output": "No Templates found."}, {"id": "no_template_if_found", "args": ["--id", "test-template"], @@ -81,6 +87,66 @@ "Failed to find call python file ./.prich/templates/template-local/scripts/echo1.py", "Failed to find call python file ./.prich/templates/template-local/scripts/echo2.py", ]}, + + # wrong templates from resources + {"id": "file_empty", + "args": ["--file", "{resources}/empty.yaml"], + "expected_output": ["empty.yaml: is not valid", "check if file or contents are correct"]}, + {"id": "file_no_schema_version", + "args": ["--file", "{resources}/no_schema_version.yaml"], + "expected_output": ["no_schema_version.yaml", "is not valid", "Failed to load template", + "Unsupported template schema version NOT SET"]}, + {"id": "file_not_supported_schema", + "args": ["--file", "{resources}/not_supported_schema.yaml"], + "expected_output": ["not_supported_schema.yaml", "is not valid", "Failed to load template", + "Unsupported template schema version 0.1"]}, + {"id": "file_no_id", + "args": ["--file", "{resources}/no_id.yaml"], + "expected_output": ["no_id.yaml", "is not valid (1 issue)", "Failed to load template", + "Missing required field at 'id'", "+id: ..."]}, + {"id": "file_no_name", + "args": ["--file", "{resources}/no_name.yaml"], + "expected_output": ["no_name.yaml", "is not valid", "Failed to load template", + "Missing required field at 'name'", "+name: ..."]}, + {"id": "file_no_steps", + "args": ["--file", "{resources}/no_steps.yaml"], + "expected_output": ["no_steps.yaml", "is not valid", "Failed to load template", + "Field value should be a valid list at 'steps'", "steps: null", + "See Steps documentation:", "https://oleks-dev.github.io/prich/reference/template/steps/"]}, + {"id": "file_wrong_steps", + "args": ["--file", "{resources}/wrong_steps.yaml"], + "expected_output": ["wrong_steps.yaml", "is not valid (3 issues)", "Failed to load template", + "1. Field value 'provider' found using 'type' does not match any of the expected values: 'python', 'command', 'llm', 'render' at 'steps[1]': --- - name: Ask to generate 1st", + "2. Missing required field at 'steps[2].name': --- step_name: Ask to generate 2nd", + "3. Unrecognized field at 'steps[2].step_name': --- step_name: Ask to generate 2nd", + "See Steps documentation:", "https://oleks-dev.github.io/prich/reference/template/steps/"]}, + {"id": "file_wrong_variables", + "args": ["--file", "{resources}/wrong_variables.yaml"], + "expected_output": ["wrong_variables.yaml", "is not valid (4 issues)", "Failed to load template", + "1. Missing required field at 'variables[1].name': --- var_name: test +name: ...", + "2. Unrecognized field at 'variables[1].var_name': --- var_name: test ... ", + "3. Field value should be 'str',", + "4. Field value should be a valid boolean, unable to interpret input at 'variables[4].required'", + "See Variables documentation https://oleks-dev.github.io/prich/reference/template/variables/"]}, + {"id": "file_no_python_not_found_isolated_venv", + "args": ["--file", "{resources}/no_venv.yaml"], + "expected_output": ["no_venv.yaml", "is not valid (1 issue)", + "1. Failed to find isolated venv at", + "There are no steps with type 'python' found", + "Install it by running 'prich venv-install test-template'." + ]}, + {"id": "file_no_python_not_found_shared_venv", + "args": ["--file", "{resources}/no_shared_venv.yaml"], + "expected_output": ["no_shared_venv.yaml", "is not valid (1 issue)", + "1. Failed to find shared venv at", + "There are no steps with type 'python' found", + ]}, + {"id": "file_yaml_error", + "args": ["--file", "{resources}/yaml_error.yaml"], + "expected_output": ["yaml_error.yaml", "is not valid (1 issue)", + "Failed to load template:", "1. while scanning a simple key", + "could not find expected ':'", + ]}, ] @pytest.mark.parametrize("case", get_validate_template_CASES, ids=[c["id"] for c in get_validate_template_CASES]) def test_validate_template(mock_paths, monkeypatch, case, template, basic_config): @@ -125,9 +191,14 @@ def test_validate_template(mock_paths, monkeypatch, case, template, basic_config runner = CliRunner() with runner.isolated_filesystem(temp_dir=mock_paths.home_dir): - result = runner.invoke(validate_templates, case.get("args")) + resources_path = os.path.dirname(__file__) + args = [] + for arg in case.get("args"): + args.append(arg.replace("{resources}", str(Path(resources_path) / "resources"))) + + result = runner.invoke(validate_templates, args) if case.get("expected_output") is not None: if isinstance(case.get("expected_output"), str): case["expected_output"] = [case.get("expected_output")] for expected_output in case.get("expected_output"): - assert expected_output in result.output.replace("\n", "") + assert expected_output in result.output.replace("\n", " ").replace(" ", " ")