Skip to content

Commit c6a4741

Browse files
robtaylorclaude
andcommitted
Add comprehensive tests for pinlock loading and fix error handling
Add three new tests to verify pinlock file loading behavior: - test_load_pinlock_file: validates successful loading of valid lockfile - test_load_pinlock_missing_file: ensures proper error when file missing - test_load_pinlock_malformed_file: ensures proper error for invalid data Fix bug in load_pinlock() where only pydantic.ValidationError was caught, but Pydantic 2.x also raises PydanticUserError for incomplete models. Now catches both exception types. Fix test isolation by clearing ensure_chipflow_root() cache between tests to prevent environment pollution across test runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 22c2fd1 commit c6a4741

File tree

2 files changed

+129
-2
lines changed

2 files changed

+129
-2
lines changed

chipflow/packaging/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def load_pinlock() -> LockFile:
3636
try:
3737
json = lockfile.read_text()
3838
return LockFile.model_validate_json(json)
39-
except pydantic.ValidationError:
39+
except (pydantic.ValidationError, pydantic.PydanticUserError):
4040
raise ChipFlowError(
4141
"Lockfile `pins.lock` is misformed. "
4242
"Please remove and rerun `chipflow pin lock`"

tests/test_pin_lock.py

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
# SPDX-License-Identifier: BSD-2-Clause
2+
import json
3+
import os
4+
import tempfile
25
import unittest
6+
from pathlib import Path
37

48
from chipflow.platforms import PACKAGE_DEFINITIONS
9+
from chipflow.packaging import load_pinlock
10+
from chipflow.packaging.lockfile import LockFile
11+
from chipflow import ChipFlowError
512

613

714
class TestPinLock(unittest.TestCase):
@@ -20,4 +27,124 @@ def test_package_definitions_structure(self):
2027
self.assertEqual(package_def.name.lower(), name.lower())
2128
# Package definitions should have allocation methods
2229
self.assertTrue(hasattr(package_def, 'allocate_pins'))
23-
self.assertTrue(callable(package_def.allocate_pins))
30+
self.assertTrue(callable(package_def.allocate_pins))
31+
32+
def test_load_pinlock_file(self):
33+
"""Test loading a valid pins.lock file"""
34+
# Create a temporary directory with a sample pins.lock file
35+
with tempfile.TemporaryDirectory() as tmpdir:
36+
# Set CHIPFLOW_ROOT to temporary directory
37+
old_chipflow_root = os.environ.get('CHIPFLOW_ROOT')
38+
os.environ['CHIPFLOW_ROOT'] = tmpdir
39+
40+
# Clear the cache on ensure_chipflow_root
41+
from chipflow.utils import ensure_chipflow_root
42+
if hasattr(ensure_chipflow_root, 'root'):
43+
delattr(ensure_chipflow_root, 'root')
44+
45+
try:
46+
# Create a minimal valid lockfile
47+
lockfile_data = {
48+
"process": "sky130",
49+
"package": {
50+
"package_type": {
51+
"package_type": "QuadPackageDef",
52+
"name": "test_package",
53+
"width": 10,
54+
"height": 10,
55+
"_power": []
56+
}
57+
},
58+
"port_map": {
59+
"ports": {}
60+
},
61+
"metadata": {}
62+
}
63+
64+
lockfile_path = Path(tmpdir) / 'pins.lock'
65+
with open(lockfile_path, 'w') as f:
66+
json.dump(lockfile_data, f, indent=2)
67+
68+
# Test loading the lockfile
69+
lockfile = load_pinlock()
70+
71+
# Validate structure
72+
self.assertIsInstance(lockfile, LockFile)
73+
self.assertEqual(str(lockfile.process), "sky130")
74+
self.assertIsNotNone(lockfile.package)
75+
self.assertIsNotNone(lockfile.port_map)
76+
self.assertIsInstance(lockfile.metadata, dict)
77+
78+
finally:
79+
# Restore original CHIPFLOW_ROOT
80+
if old_chipflow_root is not None:
81+
os.environ['CHIPFLOW_ROOT'] = old_chipflow_root
82+
elif 'CHIPFLOW_ROOT' in os.environ:
83+
del os.environ['CHIPFLOW_ROOT']
84+
85+
# Clear the cache again
86+
if hasattr(ensure_chipflow_root, 'root'):
87+
delattr(ensure_chipflow_root, 'root')
88+
89+
def test_load_pinlock_missing_file(self):
90+
"""Test that loading fails gracefully when pins.lock doesn't exist"""
91+
with tempfile.TemporaryDirectory() as tmpdir:
92+
old_chipflow_root = os.environ.get('CHIPFLOW_ROOT')
93+
os.environ['CHIPFLOW_ROOT'] = tmpdir
94+
95+
# Clear the cache on ensure_chipflow_root
96+
from chipflow.utils import ensure_chipflow_root
97+
if hasattr(ensure_chipflow_root, 'root'):
98+
delattr(ensure_chipflow_root, 'root')
99+
100+
try:
101+
# Should raise ChipFlowError when pins.lock doesn't exist
102+
with self.assertRaises(ChipFlowError) as cm:
103+
load_pinlock()
104+
105+
self.assertIn("not found", str(cm.exception))
106+
self.assertIn("pins.lock", str(cm.exception))
107+
108+
finally:
109+
if old_chipflow_root is not None:
110+
os.environ['CHIPFLOW_ROOT'] = old_chipflow_root
111+
elif 'CHIPFLOW_ROOT' in os.environ:
112+
del os.environ['CHIPFLOW_ROOT']
113+
114+
# Clear the cache again
115+
if hasattr(ensure_chipflow_root, 'root'):
116+
delattr(ensure_chipflow_root, 'root')
117+
118+
def test_load_pinlock_malformed_file(self):
119+
"""Test that loading fails gracefully with malformed pins.lock"""
120+
with tempfile.TemporaryDirectory() as tmpdir:
121+
old_chipflow_root = os.environ.get('CHIPFLOW_ROOT')
122+
os.environ['CHIPFLOW_ROOT'] = tmpdir
123+
124+
# Clear the cache on ensure_chipflow_root
125+
from chipflow.utils import ensure_chipflow_root
126+
if hasattr(ensure_chipflow_root, 'root'):
127+
delattr(ensure_chipflow_root, 'root')
128+
129+
try:
130+
# Create a malformed lockfile
131+
lockfile_path = Path(tmpdir) / 'pins.lock'
132+
with open(lockfile_path, 'w') as f:
133+
f.write('{"invalid": "json structure"}\n')
134+
135+
# Should raise ChipFlowError when pins.lock is malformed
136+
with self.assertRaises(ChipFlowError) as cm:
137+
load_pinlock()
138+
139+
self.assertIn("misformed", str(cm.exception))
140+
self.assertIn("pins.lock", str(cm.exception))
141+
142+
finally:
143+
if old_chipflow_root is not None:
144+
os.environ['CHIPFLOW_ROOT'] = old_chipflow_root
145+
elif 'CHIPFLOW_ROOT' in os.environ:
146+
del os.environ['CHIPFLOW_ROOT']
147+
148+
# Clear the cache again
149+
if hasattr(ensure_chipflow_root, 'root'):
150+
delattr(ensure_chipflow_root, 'root')

0 commit comments

Comments
 (0)