Add comprehensive test coverage (reorganized)

chipflow_lib/config_models.py

@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: BSD-2-Clause
+import enum
+import re
+from typing import Dict, List, Optional, Union, Literal, Any
+
+from pydantic import BaseModel, Field, model_validator, ValidationInfo, field_validator
+
+from .platforms.utils import Process
+
+
+class PadConfig(BaseModel):
+    """Configuration for a pad in chipflow.toml."""
+    type: Literal["io", "i", "o", "oe", "clock", "reset", "power", "ground"]
+    loc: str
+
+    @model_validator(mode="after")
+    def validate_loc_format(self):
+        """Validate that the location is in the correct format."""
+        if not re.match(r"^[NSWE]?[0-9]+$", self.loc):
+            raise ValueError(f"Invalid location format: {self.loc}, expected format: [NSWE]?[0-9]+")
+        return self
+
+    @classmethod
+    def validate_pad_dict(cls, v: dict, info: ValidationInfo):
+        """Custom validation for pad dicts from TOML that may not have all fields."""
+        if isinstance(v, dict):
+            # Handle legacy format - if 'type' is missing but should be inferred from context
+            if 'loc' in v and 'type' not in v:
+                if info.field_name == 'power':
+                    v['type'] = 'power'
+
+            # Map legacy 'clk' type to 'clock' to match our enum
+            if 'type' in v and v['type'] == 'clk':
+                v['type'] = 'clock'
+
+            return v
+        return v
+
+
+class SiliconConfig(BaseModel):
+    """Configuration for silicon in chipflow.toml."""
+    process: Process
+    package: Literal["caravel", "cf20", "pga144"]
+    pads: Dict[str, PadConfig] = {}
+    power: Dict[str, PadConfig] = {}
+    debug: Optional[Dict[str, bool]] = None
+
+    @field_validator('pads', 'power', mode='before')
+    @classmethod
+    def validate_pad_dicts(cls, v, info: ValidationInfo):
+        """Pre-process pad dictionaries to handle legacy format."""
+        if isinstance(v, dict):
+            result = {}
+            for key, pad_dict in v.items():
+                # Apply the pad validator with context about which field we're in
+                validated_pad = PadConfig.validate_pad_dict(pad_dict, info)
+                result[key] = validated_pad
+            return result
+        return v
+
+
+class StepsConfig(BaseModel):
+    """Configuration for steps in chipflow.toml."""
+    silicon: str
+
+
+class ChipFlowConfig(BaseModel):
+    """Root configuration for chipflow.toml."""
+    project_name: Optional[str] = None
+    top: Dict[str, Any] = {}
+    steps: StepsConfig
+    silicon: SiliconConfig
+    clocks: Optional[Dict[str, str]] = None
+    resets: Optional[Dict[str, str]] = None
+
+
+class Config(BaseModel):
+    """Root configuration model for chipflow.toml."""
+    chipflow: ChipFlowConfig

chipflow_lib/pin_lock.py

@@ -8,7 +8,8 @@
 
 from chipflow_lib import _parse_config, ChipFlowError
 from chipflow_lib.platforms import PACKAGE_DEFINITIONS, PIN_ANNOTATION_SCHEMA, top_interfaces
-from chipflow_lib.platforms.utils import LockFile, Package, PortMap, Port
+from chipflow_lib.platforms.utils import LockFile, Package, PortMap, Port, Process
+from chipflow_lib.config_models import Config
 
 # logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)
 logger = logging.getLogger(__name__)
@@ -76,7 +77,13 @@ def allocate_pins(name: str, member: Dict[str, Any], pins: List[str], port_name:
 
 
 def lock_pins() -> None:
-    config = _parse_config()
+    # Get the config as dict for backward compatibility with top_interfaces
+    config_dict = _parse_config()
+
+    # Parse with Pydantic for type checking and strong typing
+    from chipflow_lib.config_models import Config
+    config_model = Config.model_validate(config_dict)
+
     used_pins = set()
     oldlock = None
 
@@ -86,35 +93,45 @@ def lock_pins() -> None:
         oldlock = LockFile.model_validate_json(json_string)
 
     print(f"Locking pins: {'using pins.lock' if lockfile.exists() else ''}")
-    process_name = config["chipflow"]["silicon"]["process"]
-    package_name = config["chipflow"]["silicon"]["package"]
+
+    process = config_model.chipflow.silicon.process
+    package_name = config_model.chipflow.silicon.package
 
     if package_name not in PACKAGE_DEFINITIONS:
         logger.debug(f"Package '{package_name} is unknown")
     package_type = PACKAGE_DEFINITIONS[package_name]
 
     package = Package(package_type=package_type)
+
+    # Process pads and power configurations using Pydantic models
     for d in ("pads", "power"):
         logger.debug(f"Checking [chipflow.silicon.{d}]:")
-        _map = {}
-        for k, v in config["chipflow"]["silicon"][d].items():
-            pin = str(v['loc'])
+        silicon_config = getattr(config_model.chipflow.silicon, d, {})
+        for k, v in silicon_config.items():
+            pin = str(v.loc)
             used_pins.add(pin)
-            port = oldlock.package.check_pad(k, v) if oldlock else None
+
+            # Convert Pydantic model to dict for backward compatibility
+            v_dict = {"type": v.type, "loc": v.loc}
+            port = oldlock.package.check_pad(k, v_dict) if oldlock else None
+
             if port and port.pins != [pin]:
                 raise ChipFlowError(
                     f"chipflow.toml conflicts with pins.lock: "
                     f"{k} had pin {port.pins}, now {[pin]}."
                 )
-            package.add_pad(k, v)
+
+            # Add pad to package
+            package.add_pad(k, v_dict)
 
     logger.debug(f'Pins in use: {package_type.sortpins(used_pins)}')
 
     unallocated = package_type.pins - used_pins
 
     logger.debug(f"unallocated pins = {package_type.sortpins(unallocated)}")
 
-    _, interfaces = top_interfaces(config)
+    # Use the raw dict for top_interfaces since it expects the legacy format
+    _, interfaces = top_interfaces(config_dict)
 
     logger.debug(f"All interfaces:\n{pformat(interfaces)}")
 
@@ -146,7 +163,7 @@ def lock_pins() -> None:
                 _map, _ = allocate_pins(k, v, pins)
                 port_map.add_ports(component, k, _map)
 
-    newlock = LockFile(process=process_name,
+    newlock = LockFile(process=process,
                        package=package,
                        port_map=port_map,
                        metadata=interfaces)

chipflow_lib/platforms/silicon.py

@@ -1,3 +1,5 @@
+# amaranth: UnusedElaboratable=no
+
 # SPDX-License-Identifier: BSD-2-Clause
 import logging
 import os
@@ -71,7 +73,14 @@ def __init__(self,
         self._direction = io.Direction(port.direction)
         self._invert = invert
         self._options = port.options
-
+        self._pins = port.pins
+
+        # Initialize signal attributes to None
+        self._i = None
+        self._o = None
+        self._oe = None
+
+        # Create signals based on direction
         if self._direction in (io.Direction.Input, io.Direction.Bidir):
             self._i = Signal(port.width, name=f"{component}_{name}__i")
         if self._direction in (io.Direction.Output, io.Direction.Bidir):
@@ -81,8 +90,10 @@ def __init__(self,
                 self._oe = Signal(port.width, name=f"{component}_{name}__oe", init=-1)
             else:
                 self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
+        elif self._direction is io.Direction.Output:
+            # Always create an _oe for output ports
+            self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
 
-        self._pins = port.pins
         logger.debug(f"Created SiliconPlatformPort {name}, width={len(port.pins)},dir{self._direction}")
 
     def wire(self, m: Module, interface: PureInterface):
@@ -148,6 +159,8 @@ def __getitem__(self, key):
         result._oe = None if self._oe is None else self._oe[key]
         result._invert = self._invert
         result._direction = self._direction
+        result._options = self._options
+        result._pins = self._pins
         return result
 
     def __invert__(self):
@@ -157,6 +170,8 @@ def __invert__(self):
         result._oe = self._oe
         result._invert = not self._invert
         result._direction = self._direction
+        result._options = self._options
+        result._pins = self._pins
         return result
 
     def __add__(self, other):
@@ -167,6 +182,8 @@ def __add__(self, other):
         result._oe = None if direction is io.Direction.Input else Cat(self._oe, other._oe)
         result._invert = self._invert
         result._direction = direction
+        result._options = self._options
+        result._pins = self._pins + other._pins
         return result
 
     def __repr__(self):

chipflow_lib/steps/silicon.py

@@ -9,12 +9,9 @@
 import requests
 import subprocess
 import sys
-import time
 
 import dotenv
-from pprint import pprint
 from amaranth import *
-from amaranth.lib.wiring import connect, flipped
 
 from .. import ChipFlowError
 from ..platforms import SiliconPlatform, top_interfaces
@@ -60,8 +57,12 @@ class SiliconStep:
     """Prepare and submit the design for an ASIC."""
     def __init__(self, config):
         self.config = config
-        self.project_name = config["chipflow"].get("project_name")
-        self.silicon_config = config["chipflow"]["silicon"]
+
+        # Also parse with Pydantic for type checking and better code structure
+        from chipflow_lib.config_models import Config
+        self.config_model = Config.model_validate(config)
+        self.project_name = self.config_model.chipflow.project_name
+        self.silicon_config = config["chipflow"]["silicon"]  # Keep for backward compatibility
         self.platform = SiliconPlatform(config)
 
     def build_cli_parser(self, parser):
@@ -96,7 +97,7 @@ def prepare(self):
 
         Returns the path to the RTLIL file.
         """
-        return self.platform.build(SiliconTop(self.config), name=self.config["chipflow"]["project_name"])
+        return self.platform.build(SiliconTop(self.config), name=self.config_model.chipflow.project_name)
 
     def submit(self, rtlil_path, *, dry_run=False):
         """Submit the design to the ChipFlow cloud builder.
@@ -111,7 +112,7 @@ def submit(self, rtlil_path, *, dry_run=False):
         submission_name = git_head
         if git_dirty:
             logging.warning("Git tree is dirty, submitting anyway!")
-            submission_name += f"-dirty"
+            submission_name += "-dirty"
         dep_versions = {
             "python": sys.version.split()[0]
         }
@@ -149,13 +150,15 @@ def submit(self, rtlil_path, *, dry_run=False):
                              f"dir={port.direction}, width={width}")
                 pads[padname] = {'loc': port.pins[0], 'type': port.direction.value}
 
+        # Use the Pydantic models to access configuration data
+        silicon_model = self.config_model.chipflow.silicon
         config = {
             "dependency_versions": dep_versions,
             "silicon": {
-                "process": self.silicon_config["process"],
-                "pad_ring": self.silicon_config["package"],
+                "process": str(silicon_model.process),
+                "pad_ring": silicon_model.package,
                 "pads": pads,
-                "power": self.silicon_config.get("power", {})
+                "power": {k: {"type": v.type, "loc": v.loc} for k, v in silicon_model.power.items()}
             }
         }
         if dry_run:
@@ -175,31 +178,30 @@ def submit(self, rtlil_path, *, dry_run=False):
                 "rtlil": open(rtlil_path, "rb"),
                 "config": json.dumps(config),
             })
-        
+
         # Parse response body
         try:
             resp_data = resp.json()
         except ValueError:
             resp_data = resp.text
-        
+
         # Handle response based on status code
         if resp.status_code == 200:
             logger.info(f"Submitted design: {resp_data}")
-            print(f"https://{host}/build/{resp_data["build_id"]}")
-
+            print(f"https://{host}/build/{resp_data['build_id']}")
         else:
             # Log detailed information about the failed request
             logger.error(f"Request failed with status code {resp.status_code}")
             logger.error(f"Request URL: {resp.request.url}")
-            
+
             # Log headers with auth information redacted
             headers = dict(resp.request.headers)
             if "Authorization" in headers:
                 headers["Authorization"] = "REDACTED"
             logger.error(f"Request headers: {headers}")
-            
+
             logger.error(f"Request data: {data}")
             logger.error(f"Response headers: {dict(resp.headers)}")
             logger.error(f"Response body: {resp_data}")
-            
+
             raise ChipFlowError(f"Failed to submit design: {resp_data}")

pyproject.toml

@@ -57,11 +57,13 @@ ignore = ['F403', 'F405']
 source = "scm"
 
 [tool.pdm.scripts]
-test-cov.cmd = "pytest --cov=chipflow_lib --cov-report=html"
 test.cmd = "pytest"
+test-cov.cmd = "pytest --cov=chipflow_lib --cov-report=term"
+test-cov-html.cmd = "pytest --cov=chipflow_lib --cov-report=html"
 test-docs.cmd = "sphinx-build -b doctest docs/ docs/_build"
 lint.cmd = "ruff check"
 docs.cmd = "sphinx-build docs/ docs/_build/ -W --keep-going"
+test-silicon.cmd = "pytest tests/test_silicon_platform.py tests/test_silicon_platform_additional.py tests/test_silicon_platform_amaranth.py tests/test_silicon_platform_build.py tests/test_silicon_platform_port.py --cov=chipflow_lib.platforms.silicon --cov-report=term"
 
 
 [dependency-groups]
@@ -78,3 +80,8 @@ doc = [
     "sphinx-autoapi>=3.5.0",
     "sphinx>=7.3.7",
 ]
+
+[tool.pytest.ini_options]
+testpaths = [
+    "tests",
+    ]

tests/fixtures/mock.toml

@@ -5,11 +5,7 @@ project_name = "proj-name"
 silicon = "chipflow_lib.steps.silicon:SiliconStep"
 
 [chipflow.silicon]
-processes = [
-  "ihp_sg13g2",
-  "helvellyn2",
-  "sky130"
-]
+process = "ihp_sg13g2"
 package = "pga144"
 
 [chipflow.clocks]