Skip to content

Commit 4c81113

Browse files
machshevclaude
andcommitted
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 <noreply@anthropic.com> Signed-off-by: James McCorrie <james.mccorrie@lowrisc.org>
1 parent 38c5fcf commit 4c81113

7 files changed

Lines changed: 234 additions & 9 deletions

File tree

src/dvsim/check/__init__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Copyright lowRISC contributors (OpenTitan project).
2+
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
"""Flow configuration checking."""

src/dvsim/check/flow.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Copyright lowRISC contributors (OpenTitan project).
2+
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
"""Check flow configuration files for validity."""
6+
7+
from pathlib import Path
8+
9+
import hjson
10+
11+
from dvsim.linting.parser import parse_lint_flow_config
12+
13+
14+
class FlowCheckError(Exception):
15+
"""Error checking flow configuration."""
16+
17+
18+
def detect_flow_type(hjson_path: Path) -> str | None:
19+
"""Detect the flow type from an hjson file.
20+
21+
Args:
22+
hjson_path: Path to the hjson configuration file
23+
24+
Returns:
25+
Flow type string (e.g., "lint", "sim") or None if not detected
26+
27+
Raises:
28+
FlowCheckError: If the file cannot be read or parsed
29+
30+
"""
31+
try:
32+
with hjson_path.open() as f:
33+
data = hjson.load(f, use_decimal=True)
34+
except hjson.HjsonDecodeError as e:
35+
msg = f"Failed to parse hjson: {e}"
36+
raise FlowCheckError(msg) from e
37+
except OSError as e:
38+
msg = f"Failed to read file: {e}"
39+
raise FlowCheckError(msg) from e
40+
41+
return data.get("flow")
42+
43+
44+
def check_flow_config(hjson_path: Path) -> tuple[bool, str, str | None]:
45+
"""Check a flow configuration file for validity.
46+
47+
Args:
48+
hjson_path: Path to the hjson configuration file
49+
50+
Returns:
51+
Tuple of (success, message, flow_type):
52+
- success: True if config is valid, False otherwise
53+
- message: Human-readable message describing the result
54+
- flow_type: The detected flow type or None
55+
56+
"""
57+
hjson_path = Path(hjson_path)
58+
59+
if not hjson_path.exists():
60+
return False, f"File not found: {hjson_path}", None
61+
62+
# Detect flow type
63+
try:
64+
flow_type = detect_flow_type(hjson_path)
65+
except FlowCheckError as e:
66+
return False, str(e), None
67+
68+
if flow_type is None:
69+
return False, "No 'flow' field found in configuration", None
70+
71+
# Check based on flow type
72+
if flow_type == "lint":
73+
try:
74+
config = parse_lint_flow_config(hjson_path)
75+
except (FileNotFoundError, RuntimeError) as e:
76+
return False, f"Invalid lint flow config: {e}", flow_type
77+
else:
78+
return True, f"Valid lint flow config: {config.name}", flow_type
79+
80+
# Other flow types not yet supported
81+
return False, f"Flow type '{flow_type}' checking not yet implemented", flow_type

src/dvsim/cli/admin.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,28 @@ def dashboard_gen(json_path: Path, output_dir: Path, base_url: str | None) -> No
6060
)
6161

6262

63+
@cli.command()
64+
@click.argument(
65+
"hjson_file",
66+
type=click.Path(exists=True, file_okay=True, dir_okay=False, path_type=Path),
67+
)
68+
def check(hjson_file: Path) -> None:
69+
"""Check a flow configuration file for validity."""
70+
from dvsim.check.flow import check_flow_config # noqa: PLC0415
71+
72+
success, message, flow_type = check_flow_config(hjson_file)
73+
74+
if flow_type:
75+
click.echo(f"Flow type: {flow_type}")
76+
77+
if success:
78+
click.secho(f"✓ {message}", fg="green")
79+
sys.exit(0)
80+
else:
81+
click.secho(f"✗ {message}", fg="red")
82+
sys.exit(1)
83+
84+
6385
@cli.group()
6486
def report() -> None:
6587
"""Reporting helper commands."""

src/dvsim/linting/config.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
"""Pydantic models for lint flow configuration."""
66

7-
87
from pydantic import BaseModel, ConfigDict, Field
98

109

@@ -75,5 +74,5 @@ class LintFlowConfig(BaseModel):
7574

7675
model_config = ConfigDict(
7776
frozen=True, # Make the model immutable
78-
extra="allow", # Allow extra fields for forwards compatibility
77+
extra="forbid", # Forbid extra fields to catch configuration errors
7978
)

tests/check/__init__.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Copyright lowRISC contributors (OpenTitan project).
2+
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
"""Check tests."""

tests/check/test_flow.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# Copyright lowRISC contributors (OpenTitan project).
2+
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
"""Test flow configuration checking."""
6+
7+
from pathlib import Path
8+
9+
import hjson
10+
from hamcrest import assert_that, equal_to
11+
12+
from dvsim.check.flow import check_flow_config, detect_flow_type
13+
14+
15+
def test_check_valid_lint_config() -> None:
16+
"""Test checking a valid lint flow configuration."""
17+
fixtures_dir = Path(__file__).parent.parent / "flow" / "fixtures"
18+
config_file = fixtures_dir / "example_lint.hjson"
19+
20+
success, message, flow_type = check_flow_config(config_file)
21+
22+
assert_that(success, equal_to(True))
23+
assert_that(flow_type, equal_to("lint"))
24+
assert_that("aes_dv_lint" in message, equal_to(True))
25+
26+
27+
def test_check_invalid_lint_config(tmp_path: Path) -> None:
28+
"""Test checking an invalid lint flow configuration."""
29+
config_file = tmp_path / "invalid_lint.hjson"
30+
config_data = {
31+
"flow": "lint",
32+
# Missing required 'name' field
33+
}
34+
config_file.write_text(hjson.dumps(config_data))
35+
36+
success, message, flow_type = check_flow_config(config_file)
37+
38+
assert_that(success, equal_to(False))
39+
assert_that(flow_type, equal_to("lint"))
40+
assert_that("Invalid" in message, equal_to(True))
41+
42+
43+
def test_check_missing_flow_field(tmp_path: Path) -> None:
44+
"""Test checking a config without a flow field."""
45+
config_file = tmp_path / "no_flow.hjson"
46+
config_data = {"name": "test"}
47+
config_file.write_text(hjson.dumps(config_data))
48+
49+
success, message, flow_type = check_flow_config(config_file)
50+
51+
assert_that(success, equal_to(False))
52+
assert_that(flow_type, equal_to(None))
53+
assert_that("No 'flow' field" in message, equal_to(True))
54+
55+
56+
def test_check_nonexistent_file(tmp_path: Path) -> None:
57+
"""Test checking a file that doesn't exist."""
58+
config_file = tmp_path / "nonexistent.hjson"
59+
60+
success, message, flow_type = check_flow_config(config_file)
61+
62+
assert_that(success, equal_to(False))
63+
assert_that(flow_type, equal_to(None))
64+
assert_that("not found" in message.lower(), equal_to(True))
65+
66+
67+
def test_check_invalid_hjson(tmp_path: Path) -> None:
68+
"""Test checking a file with invalid hjson syntax."""
69+
config_file = tmp_path / "invalid.hjson"
70+
config_file.write_text("{invalid hjson content")
71+
72+
success, message, flow_type = check_flow_config(config_file)
73+
74+
assert_that(success, equal_to(False))
75+
assert_that(flow_type, equal_to(None))
76+
assert_that("parse" in message.lower(), equal_to(True))
77+
78+
79+
def test_check_unsupported_flow_type(tmp_path: Path) -> None:
80+
"""Test checking a config with an unsupported flow type."""
81+
config_file = tmp_path / "sim.hjson"
82+
config_data = {
83+
"name": "test_sim",
84+
"flow": "sim",
85+
}
86+
config_file.write_text(hjson.dumps(config_data))
87+
88+
success, message, flow_type = check_flow_config(config_file)
89+
90+
assert_that(success, equal_to(False))
91+
assert_that(flow_type, equal_to("sim"))
92+
assert_that("not yet implemented" in message, equal_to(True))
93+
94+
95+
def test_detect_flow_type() -> None:
96+
"""Test detecting flow type from config file."""
97+
fixtures_dir = Path(__file__).parent.parent / "flow" / "fixtures"
98+
config_file = fixtures_dir / "example_lint.hjson"
99+
100+
flow_type = detect_flow_type(config_file)
101+
102+
assert_that(flow_type, equal_to("lint"))
103+
104+
105+
def test_detect_flow_type_missing(tmp_path: Path) -> None:
106+
"""Test detecting flow type when field is missing."""
107+
config_file = tmp_path / "no_flow.hjson"
108+
config_data = {"name": "test"}
109+
config_file.write_text(hjson.dumps(config_data))
110+
111+
flow_type = detect_flow_type(config_file)
112+
113+
assert_that(flow_type, equal_to(None))

tests/flow/test_lint_parser.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,20 +164,20 @@ def test_missing_required_field_raises() -> None:
164164
assert "name" in str(exc_info.value)
165165

166166

167-
def test_extra_fields_allowed() -> None:
168-
"""Test that extra fields are allowed in the configuration."""
167+
def test_extra_fields_forbidden() -> None:
168+
"""Test that extra fields are forbidden in the configuration."""
169169
config_dict = {
170170
"name": "test",
171171
"flow": "lint",
172172
"custom_field": "custom_value",
173-
"another_field": 123,
174173
}
175174

176-
config = load_lint_config_from_dict(config_dict)
175+
# Should raise ValidationError for unknown fields
176+
with pytest.raises(ValidationError) as exc_info:
177+
load_lint_config_from_dict(config_dict)
177178

178-
assert_that(config.name, equal_to("test"))
179-
# Extra fields should be accessible via model_extra or __dict__
180-
assert hasattr(config, "custom_field") or "custom_field" in config.model_extra
179+
# Verify the error mentions the unexpected field
180+
assert "custom_field" in str(exc_info.value)
181181

182182

183183
def test_integration_with_example_hjson() -> None:

0 commit comments

Comments
 (0)