From bcd72b00561dacb0e0da1d002b38251b9215845c Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Fri, 20 Mar 2026 17:20:42 +0000 Subject: [PATCH 1/3] refactor: move lint flow to linting module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move lint flow from flow/lint.py to linting/flow.py to consolidate lint-related code in the linting module. This is part of separating flows into standalone components. Changes: - Move flow/lint.py → linting/flow.py - Rename linting/parser.py → linting/output_parser.py to make room for new config parser - Update imports in factory.py, cdc.py, rdc.py Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: James McCorrie --- src/dvsim/flow/cdc.py | 2 +- src/dvsim/flow/factory.py | 2 +- src/dvsim/flow/rdc.py | 2 +- src/dvsim/{flow/lint.py => linting/flow.py} | 2 ++ src/dvsim/linting/{parser.py => output_parser.py} | 0 5 files changed, 5 insertions(+), 3 deletions(-) rename src/dvsim/{flow/lint.py => linting/flow.py} (98%) rename src/dvsim/linting/{parser.py => output_parser.py} (100%) diff --git a/src/dvsim/flow/cdc.py b/src/dvsim/flow/cdc.py index 2a3bea7c..5556be35 100644 --- a/src/dvsim/flow/cdc.py +++ b/src/dvsim/flow/cdc.py @@ -4,7 +4,7 @@ """Class describing lint configuration object.""" -from dvsim.flow.lint import LintCfg +from dvsim.linting.flow import LintCfg class CdcCfg(LintCfg): diff --git a/src/dvsim/flow/factory.py b/src/dvsim/flow/factory.py index 166db158..517296c6 100644 --- a/src/dvsim/flow/factory.py +++ b/src/dvsim/flow/factory.py @@ -9,9 +9,9 @@ from dvsim.flow.cdc import CdcCfg from dvsim.flow.formal import FormalCfg from dvsim.flow.hjson import load_hjson -from dvsim.flow.lint import LintCfg from dvsim.flow.rdc import RdcCfg from dvsim.flow.syn import SynCfg +from dvsim.linting.flow import LintCfg from dvsim.logging import log from dvsim.sim.flow import SimCfg diff --git a/src/dvsim/flow/rdc.py b/src/dvsim/flow/rdc.py index fcb50ab3..d1f56add 100644 --- a/src/dvsim/flow/rdc.py +++ b/src/dvsim/flow/rdc.py @@ -4,7 +4,7 @@ """RDC Configuration Class.""" -from dvsim.flow.lint import LintCfg +from dvsim.linting.flow import LintCfg class RdcCfg(LintCfg): diff --git a/src/dvsim/flow/lint.py b/src/dvsim/linting/flow.py similarity index 98% rename from src/dvsim/flow/lint.py rename to src/dvsim/linting/flow.py index cdf434c3..f989f243 100644 --- a/src/dvsim/flow/lint.py +++ b/src/dvsim/linting/flow.py @@ -10,6 +10,8 @@ from tabulate import tabulate from dvsim.flow.one_shot import OneShotCfg + +# TODO: Update to use dvsim.linting.config when integrating new config parser from dvsim.job.data import CompletedJobStatus from dvsim.logging import log from dvsim.msg_buckets import MsgBuckets diff --git a/src/dvsim/linting/parser.py b/src/dvsim/linting/output_parser.py similarity index 100% rename from src/dvsim/linting/parser.py rename to src/dvsim/linting/output_parser.py From 38c5fcf7d3bde2d4ac0234db39813bf5cf9e7955 Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Fri, 20 Mar 2026 17:24:33 +0000 Subject: [PATCH 2/3] feat: add pydantic model and parser for lint flow config Add Pydantic-based config parser separate from existing flow config infrastructure to enable gradual migration to standalone components. Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: James McCorrie --- src/dvsim/linting/config.py | 79 +++++++++ src/dvsim/linting/parser.py | 97 +++++++++++ tests/flow/__init__.py | 5 + tests/flow/fixtures/example_lint.hjson | 50 ++++++ tests/flow/test_lint_parser.py | 223 +++++++++++++++++++++++++ 5 files changed, 454 insertions(+) create mode 100644 src/dvsim/linting/config.py create mode 100644 src/dvsim/linting/parser.py create mode 100644 tests/flow/__init__.py create mode 100644 tests/flow/fixtures/example_lint.hjson create mode 100644 tests/flow/test_lint_parser.py diff --git a/src/dvsim/linting/config.py b/src/dvsim/linting/config.py new file mode 100644 index 00000000..60a27918 --- /dev/null +++ b/src/dvsim/linting/config.py @@ -0,0 +1,79 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Pydantic models for lint flow configuration.""" + + +from pydantic import BaseModel, ConfigDict, Field + + +class MessageBucket(BaseModel): + """Message bucket configuration for categorizing lint messages.""" + + model_config = ConfigDict(frozen=True) + + category: str = Field(..., description="Message category (e.g., 'flow', 'lint')") + severity: str = Field(..., description="Message severity (e.g., 'info', 'warning', 'error')") + label: str = Field(default="", description="Optional label for the bucket") + + +class LintFlowConfig(BaseModel): + """Configuration for the lint flow. + + This model represents the core lint flow configuration that can be parsed + from hjson files. It focuses on lint-specific fields and does not include + all the base flow configuration fields that are handled by the existing + FlowCfg class hierarchy. + """ + + # Flow identification + flow: str = Field(default="lint", description="Flow type identifier") + name: str = Field(..., description="Name of the lint configuration") + + # Build configuration + build_dir: str = Field(default="", description="Build directory path") + build_log: str = Field(default="", description="Build log file path") + build_cmd: str = Field(default="", description="Build command") + build_opts: list[str] = Field(default_factory=list, description="Build options") + + # FuseSoC configuration + fusesoc_core: str = Field(default="", description="FuseSoC core to use") + additional_fusesoc_argument: str = Field( + default="", description="Additional FuseSoC argument (e.g., mapping)" + ) + + # Lint-specific configuration + is_style_lint: bool = Field(default=False, description="Whether this is a style lint run") + report_severities: list[str] = Field( + default_factory=lambda: ["info", "warning", "error"], + description="Message severities to include in reports", + ) + fail_severities: list[str] = Field( + default_factory=lambda: ["warning", "error"], + description="Message severities that cause the flow to fail", + ) + message_buckets: list[MessageBucket] = Field( + default_factory=list, description="Message bucket configuration" + ) + + # Tool configuration + tool: str | None = Field(default=None, description="Lint tool to use") + dut: str = Field(default="", description="Design under test name") + + # Directory paths + scratch_path: str = Field(default="", description="Scratch directory path") + rel_path: str = Field(default="", description="Relative path for results") + + # Import and use configurations + import_cfgs: list[str] = Field( + default_factory=list, description="Configuration files to import" + ) + use_cfgs: list[dict | str] = Field( + default_factory=list, description="Sub-configurations to use (for primary configs)" + ) + + model_config = ConfigDict( + frozen=True, # Make the model immutable + extra="allow", # Allow extra fields for forwards compatibility + ) diff --git a/src/dvsim/linting/parser.py b/src/dvsim/linting/parser.py new file mode 100644 index 00000000..266e3d40 --- /dev/null +++ b/src/dvsim/linting/parser.py @@ -0,0 +1,97 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Parser for lint flow hjson configuration files.""" + +from pathlib import Path + +import hjson +from pydantic import ValidationError + +from dvsim.linting.config import LintFlowConfig, MessageBucket + + +def parse_lint_flow_config(hjson_path: str | Path) -> LintFlowConfig: + """Parse a lint flow hjson configuration file. + + This function loads an hjson file and validates it against the LintFlowConfig + pydantic model. This is a new parser specifically for lint flows and does not + modify the existing flow config parsing infrastructure. + + Args: + hjson_path: Path to the hjson configuration file + + Returns: + LintFlowConfig: Validated lint flow configuration + + Raises: + FileNotFoundError: If the hjson file doesn't exist + ValidationError: If the hjson data doesn't match the expected schema + RuntimeError: If there are other errors parsing the hjson + + Example: + >>> config = parse_lint_flow_config("path/to/lint_cfg.hjson") + >>> print(config.name) + >>> print(config.report_severities) + + """ + hjson_path = Path(hjson_path) + + if not hjson_path.exists(): + msg = f"Configuration file not found: {hjson_path}" + raise FileNotFoundError(msg) + + # Parse the hjson file using hjson library directly + try: + with hjson_path.open() as f: + hjson_data = hjson.load(f, use_decimal=True) + except hjson.HjsonDecodeError as e: + msg = f"Failed to parse hjson file {hjson_path}: {e}" + raise RuntimeError(msg) from e + except OSError as e: + msg = f"Failed to read hjson file {hjson_path}: {e}" + raise RuntimeError(msg) from e + + # Validate against the pydantic model + try: + # Convert message_buckets from list of dicts to list of MessageBucket objects + if "message_buckets" in hjson_data: + hjson_data["message_buckets"] = [ + MessageBucket(**bucket) if isinstance(bucket, dict) else bucket + for bucket in hjson_data["message_buckets"] + ] + + return LintFlowConfig(**hjson_data) + except ValidationError as e: + msg = f"Configuration validation failed for {hjson_path}:\n{e}" + raise RuntimeError(msg) from e + + +def load_lint_config_from_dict(config_dict: dict) -> LintFlowConfig: + """Load a lint flow configuration from a dictionary. + + This is useful for loading inline configurations or for testing. + + Args: + config_dict: Dictionary containing lint flow configuration + + Returns: + LintFlowConfig: Validated lint flow configuration + + Raises: + ValidationError: If the dictionary doesn't match the expected schema + + Example: + >>> config_dict = {"name": "test_lint", "flow": "lint"} + >>> config = load_lint_config_from_dict(config_dict) + + """ + # Convert message_buckets if present + if "message_buckets" in config_dict: + config_dict["message_buckets"] = [ + MessageBucket(**bucket) if isinstance(bucket, dict) else bucket + for bucket in config_dict["message_buckets"] + ] + + return LintFlowConfig(**config_dict) diff --git a/tests/flow/__init__.py b/tests/flow/__init__.py new file mode 100644 index 00000000..2524972f --- /dev/null +++ b/tests/flow/__init__.py @@ -0,0 +1,5 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Flow tests.""" diff --git a/tests/flow/fixtures/example_lint.hjson b/tests/flow/fixtures/example_lint.hjson new file mode 100644 index 00000000..dd213a0f --- /dev/null +++ b/tests/flow/fixtures/example_lint.hjson @@ -0,0 +1,50 @@ +// Copyright lowRISC contributors (OpenTitan project). +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +// Example lint flow configuration for integration testing +{ + name: aes_dv_lint + flow: lint + + // Design under test + dut: aes + + // FuseSoC configuration + fusesoc_core: lowrisc:dv:aes_sim + additional_fusesoc_argument: --mapping=lowrisc:systems:top_earlgrey:0.1 + + // Build configuration + build_dir: "{scratch_path}/{build_mode}" + build_log: "{build_dir}/{tool}.log" + build_cmd: fusesoc + build_opts: [ + "--cores-root {proj_root}/hw" + "run" + "--target={flow}" + "--tool={tool}" + "--work-root={build_dir}/fusesoc-work" + ] + + // Lint configuration + is_style_lint: false + report_severities: ["info", "warning", "error"] + fail_severities: ["warning", "error"] + + // Message bucket configuration + message_buckets: [ + {category: "flow", severity: "info", label: "Flow Info"} + {category: "flow", severity: "warning", label: "Flow Warnings"} + {category: "flow", severity: "error", label: "Flow Errors"} + {category: "lint", severity: "info", label: "Lint Info"} + {category: "lint", severity: "warning", label: "Lint Warnings"} + {category: "lint", severity: "error", label: "Lint Errors"} + ] + + // Tool configuration + tool: ascentlint + + // Paths + scratch_path: /tmp/dvsim/scratch + rel_path: hw/ip/aes/dv/lint/{tool} +} diff --git a/tests/flow/test_lint_parser.py b/tests/flow/test_lint_parser.py new file mode 100644 index 00000000..94c80f37 --- /dev/null +++ b/tests/flow/test_lint_parser.py @@ -0,0 +1,223 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Test lint flow configuration parser.""" + +from pathlib import Path + +import hjson +import pytest +from hamcrest import assert_that, calling, equal_to, has_length, raises +from pydantic import ValidationError + +from dvsim.linting.config import LintFlowConfig, MessageBucket +from dvsim.linting.parser import load_lint_config_from_dict, parse_lint_flow_config + + +def test_parse_minimal_lint_config(tmp_path: Path) -> None: + """Test parsing a minimal lint configuration.""" + config_data = { + "name": "test_lint", + "flow": "lint", + } + + config_file = tmp_path / "test_lint.hjson" + config_file.write_text(hjson.dumps(config_data)) + + config = parse_lint_flow_config(config_file) + + assert_that(config.name, equal_to("test_lint")) + assert_that(config.flow, equal_to("lint")) + assert_that(config.is_style_lint, equal_to(False)) + assert_that(config.report_severities, equal_to(["info", "warning", "error"])) + assert_that(config.fail_severities, equal_to(["warning", "error"])) + + +def test_parse_full_lint_config(tmp_path: Path) -> None: + """Test parsing a full lint configuration with all fields.""" + config_data = { + "name": "aes_lint", + "flow": "lint", + "dut": "aes", + "fusesoc_core": "lowrisc:dv:aes_sim", + "additional_fusesoc_argument": "--mapping=lowrisc:systems:top_earlgrey:0.1", + "build_dir": "{scratch_path}/{build_mode}", + "build_log": "{build_dir}/{tool}.log", + "build_cmd": "fusesoc", + "build_opts": ["--cores-root {proj_root}/hw", "run"], + "is_style_lint": False, + "report_severities": ["info", "warning", "error"], + "fail_severities": ["warning", "error"], + "message_buckets": [ + {"category": "flow", "severity": "info", "label": ""}, + {"category": "flow", "severity": "warning", "label": ""}, + {"category": "flow", "severity": "error", "label": ""}, + {"category": "lint", "severity": "info", "label": ""}, + {"category": "lint", "severity": "warning", "label": ""}, + {"category": "lint", "severity": "error", "label": ""}, + ], + } + + config_file = tmp_path / "aes_lint.hjson" + config_file.write_text(hjson.dumps(config_data)) + + config = parse_lint_flow_config(config_file) + + assert_that(config.name, equal_to("aes_lint")) + assert_that(config.dut, equal_to("aes")) + assert_that(config.fusesoc_core, equal_to("lowrisc:dv:aes_sim")) + assert_that(config.build_opts, has_length(2)) + assert_that(config.message_buckets, has_length(6)) + assert_that(config.message_buckets[0].category, equal_to("flow")) + assert_that(config.message_buckets[0].severity, equal_to("info")) + + +def test_parse_style_lint_config(tmp_path: Path) -> None: + """Test parsing a style lint configuration.""" + config_data = { + "name": "verible_style_lint", + "flow": "lint", + "is_style_lint": True, + "tool": "veriblelint", + } + + config_file = tmp_path / "style_lint.hjson" + config_file.write_text(hjson.dumps(config_data)) + + config = parse_lint_flow_config(config_file) + + assert_that(config.name, equal_to("verible_style_lint")) + assert_that(config.is_style_lint, equal_to(True)) + assert_that(config.tool, equal_to("veriblelint")) + + +def test_load_lint_config_from_dict() -> None: + """Test loading configuration from a dictionary.""" + config_dict = { + "name": "dict_test", + "flow": "lint", + "message_buckets": [ + {"category": "lint", "severity": "error", "label": "Errors"}, + ], + } + + config = load_lint_config_from_dict(config_dict) + + assert_that(config.name, equal_to("dict_test")) + assert_that(config.message_buckets, has_length(1)) + assert_that(config.message_buckets[0].category, equal_to("lint")) + + +def test_message_bucket_model() -> None: + """Test the MessageBucket pydantic model.""" + bucket = MessageBucket(category="flow", severity="warning", label="Flow Warnings") + + assert_that(bucket.category, equal_to("flow")) + assert_that(bucket.severity, equal_to("warning")) + assert_that(bucket.label, equal_to("Flow Warnings")) + + +def test_message_bucket_default_label() -> None: + """Test MessageBucket with default label.""" + bucket = MessageBucket(category="lint", severity="info") + + assert_that(bucket.category, equal_to("lint")) + assert_that(bucket.severity, equal_to("info")) + assert_that(bucket.label, equal_to("")) + + +def test_parse_missing_file_raises() -> None: + """Test that parsing a non-existent file raises FileNotFoundError.""" + assert_that( + calling(parse_lint_flow_config).with_args("/nonexistent/file.hjson"), + raises(FileNotFoundError), + ) + + +def test_parse_invalid_hjson_raises(tmp_path: Path) -> None: + """Test that parsing invalid hjson raises RuntimeError.""" + config_file = tmp_path / "invalid.hjson" + config_file.write_text("{invalid hjson content") + + # Should raise RuntimeError for invalid hjson syntax + with pytest.raises(RuntimeError) as exc_info: + parse_lint_flow_config(config_file) + + # Verify the error message mentions the file and parsing failure + assert str(config_file) in str(exc_info.value) + assert "parse" in str(exc_info.value).lower() + + +def test_missing_required_field_raises() -> None: + """Test that missing required fields raise ValidationError.""" + config_dict = { + "flow": "lint", + # Missing required 'name' field + } + + # ValidationError should be raised directly by Pydantic + with pytest.raises(ValidationError) as exc_info: + load_lint_config_from_dict(config_dict) + + # Verify the error mentions the missing 'name' field + assert "name" in str(exc_info.value) + + +def test_extra_fields_allowed() -> None: + """Test that extra fields are allowed in the configuration.""" + config_dict = { + "name": "test", + "flow": "lint", + "custom_field": "custom_value", + "another_field": 123, + } + + config = load_lint_config_from_dict(config_dict) + + assert_that(config.name, equal_to("test")) + # Extra fields should be accessible via model_extra or __dict__ + assert hasattr(config, "custom_field") or "custom_field" in config.model_extra + + +def test_integration_with_example_hjson() -> None: + """Integration test with a realistic lint flow hjson file.""" + fixtures_dir = Path(__file__).parent / "fixtures" + config_file = fixtures_dir / "example_lint.hjson" + + config = parse_lint_flow_config(config_file) + + # Build expected config + expected = LintFlowConfig( + name="aes_dv_lint", + flow="lint", + dut="aes", + fusesoc_core="lowrisc:dv:aes_sim", + additional_fusesoc_argument="--mapping=lowrisc:systems:top_earlgrey:0.1", + build_dir="{scratch_path}/{build_mode}", + build_log="{build_dir}/{tool}.log", + build_cmd="fusesoc", + build_opts=[ + "--cores-root {proj_root}/hw", + "run", + "--target={flow}", + "--tool={tool}", + "--work-root={build_dir}/fusesoc-work", + ], + is_style_lint=False, + report_severities=["info", "warning", "error"], + fail_severities=["warning", "error"], + message_buckets=[ + MessageBucket(category="flow", severity="info", label="Flow Info"), + MessageBucket(category="flow", severity="warning", label="Flow Warnings"), + MessageBucket(category="flow", severity="error", label="Flow Errors"), + MessageBucket(category="lint", severity="info", label="Lint Info"), + MessageBucket(category="lint", severity="warning", label="Lint Warnings"), + MessageBucket(category="lint", severity="error", label="Lint Errors"), + ], + tool="ascentlint", + scratch_path="/tmp/dvsim/scratch", # noqa: S108 + rel_path="hw/ip/aes/dv/lint/{tool}", + ) + + assert_that(config.model_dump(), equal_to(expected.model_dump())) From 4c811138d0beaea26d4b904291ba085d58de1c3d Mon Sep 17 00:00:00 2001 From: James McCorrie Date: Fri, 20 Mar 2026 17:47:34 +0000 Subject: [PATCH 3/3] feat: add dvsim-admin check command for flow configs Add 'dvsim-admin check' subcommand to validate flow configuration files. The check subsystem detects the flow type and validates the config using the appropriate parser. Currently supports lint flows with extensibility for other flow types. Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: James McCorrie --- src/dvsim/check/__init__.py | 5 ++ src/dvsim/check/flow.py | 81 +++++++++++++++++++++++ src/dvsim/cli/admin.py | 22 +++++++ src/dvsim/linting/config.py | 3 +- tests/check/__init__.py | 5 ++ tests/check/test_flow.py | 113 +++++++++++++++++++++++++++++++++ tests/flow/test_lint_parser.py | 14 ++-- 7 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 src/dvsim/check/__init__.py create mode 100644 src/dvsim/check/flow.py create mode 100644 tests/check/__init__.py create mode 100644 tests/check/test_flow.py diff --git a/src/dvsim/check/__init__.py b/src/dvsim/check/__init__.py new file mode 100644 index 00000000..5aae22fe --- /dev/null +++ b/src/dvsim/check/__init__.py @@ -0,0 +1,5 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Flow configuration checking.""" diff --git a/src/dvsim/check/flow.py b/src/dvsim/check/flow.py new file mode 100644 index 00000000..27881a09 --- /dev/null +++ b/src/dvsim/check/flow.py @@ -0,0 +1,81 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Check flow configuration files for validity.""" + +from pathlib import Path + +import hjson + +from dvsim.linting.parser import parse_lint_flow_config + + +class FlowCheckError(Exception): + """Error checking flow configuration.""" + + +def detect_flow_type(hjson_path: Path) -> str | None: + """Detect the flow type from an hjson file. + + Args: + hjson_path: Path to the hjson configuration file + + Returns: + Flow type string (e.g., "lint", "sim") or None if not detected + + Raises: + FlowCheckError: If the file cannot be read or parsed + + """ + try: + with hjson_path.open() as f: + data = hjson.load(f, use_decimal=True) + except hjson.HjsonDecodeError as e: + msg = f"Failed to parse hjson: {e}" + raise FlowCheckError(msg) from e + except OSError as e: + msg = f"Failed to read file: {e}" + raise FlowCheckError(msg) from e + + return data.get("flow") + + +def check_flow_config(hjson_path: Path) -> tuple[bool, str, str | None]: + """Check a flow configuration file for validity. + + Args: + hjson_path: Path to the hjson configuration file + + Returns: + Tuple of (success, message, flow_type): + - success: True if config is valid, False otherwise + - message: Human-readable message describing the result + - flow_type: The detected flow type or None + + """ + hjson_path = Path(hjson_path) + + if not hjson_path.exists(): + return False, f"File not found: {hjson_path}", None + + # Detect flow type + try: + flow_type = detect_flow_type(hjson_path) + except FlowCheckError as e: + return False, str(e), None + + if flow_type is None: + return False, "No 'flow' field found in configuration", None + + # Check based on flow type + if flow_type == "lint": + try: + config = parse_lint_flow_config(hjson_path) + except (FileNotFoundError, RuntimeError) as e: + return False, f"Invalid lint flow config: {e}", flow_type + else: + return True, f"Valid lint flow config: {config.name}", flow_type + + # Other flow types not yet supported + return False, f"Flow type '{flow_type}' checking not yet implemented", flow_type diff --git a/src/dvsim/cli/admin.py b/src/dvsim/cli/admin.py index 754588fe..301807ff 100644 --- a/src/dvsim/cli/admin.py +++ b/src/dvsim/cli/admin.py @@ -60,6 +60,28 @@ def dashboard_gen(json_path: Path, output_dir: Path, base_url: str | None) -> No ) +@cli.command() +@click.argument( + "hjson_file", + type=click.Path(exists=True, file_okay=True, dir_okay=False, path_type=Path), +) +def check(hjson_file: Path) -> None: + """Check a flow configuration file for validity.""" + from dvsim.check.flow import check_flow_config # noqa: PLC0415 + + success, message, flow_type = check_flow_config(hjson_file) + + if flow_type: + click.echo(f"Flow type: {flow_type}") + + if success: + click.secho(f"✓ {message}", fg="green") + sys.exit(0) + else: + click.secho(f"✗ {message}", fg="red") + sys.exit(1) + + @cli.group() def report() -> None: """Reporting helper commands.""" diff --git a/src/dvsim/linting/config.py b/src/dvsim/linting/config.py index 60a27918..3004d28c 100644 --- a/src/dvsim/linting/config.py +++ b/src/dvsim/linting/config.py @@ -4,7 +4,6 @@ """Pydantic models for lint flow configuration.""" - from pydantic import BaseModel, ConfigDict, Field @@ -75,5 +74,5 @@ class LintFlowConfig(BaseModel): model_config = ConfigDict( frozen=True, # Make the model immutable - extra="allow", # Allow extra fields for forwards compatibility + extra="forbid", # Forbid extra fields to catch configuration errors ) diff --git a/tests/check/__init__.py b/tests/check/__init__.py new file mode 100644 index 00000000..a459a447 --- /dev/null +++ b/tests/check/__init__.py @@ -0,0 +1,5 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Check tests.""" diff --git a/tests/check/test_flow.py b/tests/check/test_flow.py new file mode 100644 index 00000000..da0fda57 --- /dev/null +++ b/tests/check/test_flow.py @@ -0,0 +1,113 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Test flow configuration checking.""" + +from pathlib import Path + +import hjson +from hamcrest import assert_that, equal_to + +from dvsim.check.flow import check_flow_config, detect_flow_type + + +def test_check_valid_lint_config() -> None: + """Test checking a valid lint flow configuration.""" + fixtures_dir = Path(__file__).parent.parent / "flow" / "fixtures" + config_file = fixtures_dir / "example_lint.hjson" + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(True)) + assert_that(flow_type, equal_to("lint")) + assert_that("aes_dv_lint" in message, equal_to(True)) + + +def test_check_invalid_lint_config(tmp_path: Path) -> None: + """Test checking an invalid lint flow configuration.""" + config_file = tmp_path / "invalid_lint.hjson" + config_data = { + "flow": "lint", + # Missing required 'name' field + } + config_file.write_text(hjson.dumps(config_data)) + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(False)) + assert_that(flow_type, equal_to("lint")) + assert_that("Invalid" in message, equal_to(True)) + + +def test_check_missing_flow_field(tmp_path: Path) -> None: + """Test checking a config without a flow field.""" + config_file = tmp_path / "no_flow.hjson" + config_data = {"name": "test"} + config_file.write_text(hjson.dumps(config_data)) + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(False)) + assert_that(flow_type, equal_to(None)) + assert_that("No 'flow' field" in message, equal_to(True)) + + +def test_check_nonexistent_file(tmp_path: Path) -> None: + """Test checking a file that doesn't exist.""" + config_file = tmp_path / "nonexistent.hjson" + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(False)) + assert_that(flow_type, equal_to(None)) + assert_that("not found" in message.lower(), equal_to(True)) + + +def test_check_invalid_hjson(tmp_path: Path) -> None: + """Test checking a file with invalid hjson syntax.""" + config_file = tmp_path / "invalid.hjson" + config_file.write_text("{invalid hjson content") + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(False)) + assert_that(flow_type, equal_to(None)) + assert_that("parse" in message.lower(), equal_to(True)) + + +def test_check_unsupported_flow_type(tmp_path: Path) -> None: + """Test checking a config with an unsupported flow type.""" + config_file = tmp_path / "sim.hjson" + config_data = { + "name": "test_sim", + "flow": "sim", + } + config_file.write_text(hjson.dumps(config_data)) + + success, message, flow_type = check_flow_config(config_file) + + assert_that(success, equal_to(False)) + assert_that(flow_type, equal_to("sim")) + assert_that("not yet implemented" in message, equal_to(True)) + + +def test_detect_flow_type() -> None: + """Test detecting flow type from config file.""" + fixtures_dir = Path(__file__).parent.parent / "flow" / "fixtures" + config_file = fixtures_dir / "example_lint.hjson" + + flow_type = detect_flow_type(config_file) + + assert_that(flow_type, equal_to("lint")) + + +def test_detect_flow_type_missing(tmp_path: Path) -> None: + """Test detecting flow type when field is missing.""" + config_file = tmp_path / "no_flow.hjson" + config_data = {"name": "test"} + config_file.write_text(hjson.dumps(config_data)) + + flow_type = detect_flow_type(config_file) + + assert_that(flow_type, equal_to(None)) diff --git a/tests/flow/test_lint_parser.py b/tests/flow/test_lint_parser.py index 94c80f37..5cd42da5 100644 --- a/tests/flow/test_lint_parser.py +++ b/tests/flow/test_lint_parser.py @@ -164,20 +164,20 @@ def test_missing_required_field_raises() -> None: assert "name" in str(exc_info.value) -def test_extra_fields_allowed() -> None: - """Test that extra fields are allowed in the configuration.""" +def test_extra_fields_forbidden() -> None: + """Test that extra fields are forbidden in the configuration.""" config_dict = { "name": "test", "flow": "lint", "custom_field": "custom_value", - "another_field": 123, } - config = load_lint_config_from_dict(config_dict) + # Should raise ValidationError for unknown fields + with pytest.raises(ValidationError) as exc_info: + load_lint_config_from_dict(config_dict) - assert_that(config.name, equal_to("test")) - # Extra fields should be accessible via model_extra or __dict__ - assert hasattr(config, "custom_field") or "custom_field" in config.model_extra + # Verify the error mentions the unexpected field + assert "custom_field" in str(exc_info.value) def test_integration_with_example_hjson() -> None: