Skip to content

Commit ad9071c

Browse files
committed
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 ff15a06 commit ad9071c

7 files changed

Lines changed: 229 additions & 8 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: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
try:
31+
with hjson_path.open() as f:
32+
data = hjson.load(f, use_decimal=True)
33+
except hjson.HjsonDecodeError as e:
34+
raise FlowCheckError(f"Failed to parse hjson: {e}") from e
35+
except OSError as e:
36+
raise FlowCheckError(f"Failed to read file: {e}") from e
37+
38+
return data.get("flow")
39+
40+
41+
def check_flow_config(hjson_path: Path) -> tuple[bool, str, str | None]:
42+
"""Check a flow configuration file for validity.
43+
44+
Args:
45+
hjson_path: Path to the hjson configuration file
46+
47+
Returns:
48+
Tuple of (success, message, flow_type):
49+
- success: True if config is valid, False otherwise
50+
- message: Human-readable message describing the result
51+
- flow_type: The detected flow type or None
52+
"""
53+
hjson_path = Path(hjson_path)
54+
55+
if not hjson_path.exists():
56+
return False, f"File not found: {hjson_path}", None
57+
58+
# Detect flow type
59+
try:
60+
flow_type = detect_flow_type(hjson_path)
61+
except FlowCheckError as e:
62+
return False, str(e), None
63+
64+
if flow_type is None:
65+
return False, "No 'flow' field found in configuration", None
66+
67+
# Check based on flow type
68+
if flow_type == "lint":
69+
try:
70+
config = parse_lint_flow_config(hjson_path)
71+
return True, f"Valid lint flow config: {config.name}", flow_type
72+
except Exception as e:
73+
return False, f"Invalid lint flow config: {e}", flow_type
74+
75+
# Other flow types not yet supported
76+
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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,5 @@ class LintFlowConfig(BaseModel):
7676

7777
model_config = ConfigDict(
7878
frozen=True, # Make the model immutable
79-
extra="allow", # Allow extra fields for forwards compatibility
79+
extra="forbid", # Forbid extra fields to catch configuration errors
8080
)

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)